Thu, 03 Mar 2011 09:43:24 +0000
7023233: False positive for -Xlint:try with nested try with resources blocks
Summary: Wrong lint warning issued about unused resource when nested try-with-resource blocks are found
Reviewed-by: jjg
1.1 --- a/src/share/classes/com/sun/tools/javac/comp/Flow.java Wed Mar 02 21:13:55 2011 -0800 1.2 +++ b/src/share/classes/com/sun/tools/javac/comp/Flow.java Thu Mar 03 09:43:24 2011 +0000 1.3 @@ -272,7 +272,7 @@ 1.4 1.5 /** The list of unreferenced automatic resources. 1.6 */ 1.7 - Map<VarSymbol, JCVariableDecl> unrefdResources; 1.8 + Scope unrefdResources; 1.9 1.10 /** Set when processing a loop body the second time for DU analysis. */ 1.11 boolean loopPassTwo = false; 1.12 @@ -992,7 +992,6 @@ 1.13 public void visitTry(JCTry tree) { 1.14 List<Type> caughtPrev = caught; 1.15 List<Type> thrownPrev = thrown; 1.16 - Map<VarSymbol, JCVariableDecl> unrefdResourcesPrev = unrefdResources; 1.17 thrown = List.nil(); 1.18 for (List<JCCatch> l = tree.catchers; l.nonEmpty(); l = l.tail) { 1.19 List<JCExpression> subClauses = TreeInfo.isMultiCatch(l.head) ? 1.20 @@ -1002,17 +1001,18 @@ 1.21 caught = chk.incl(ct.type, caught); 1.22 } 1.23 } 1.24 + ListBuffer<JCVariableDecl> resourceVarDecls = ListBuffer.lb(); 1.25 Bits uninitsTryPrev = uninitsTry; 1.26 ListBuffer<PendingExit> prevPendingExits = pendingExits; 1.27 pendingExits = new ListBuffer<PendingExit>(); 1.28 Bits initsTry = inits.dup(); 1.29 uninitsTry = uninits.dup(); 1.30 - unrefdResources = new LinkedHashMap<VarSymbol, JCVariableDecl>(); 1.31 for (JCTree resource : tree.resources) { 1.32 if (resource instanceof JCVariableDecl) { 1.33 JCVariableDecl vdecl = (JCVariableDecl) resource; 1.34 visitVarDef(vdecl); 1.35 - unrefdResources.put(vdecl.sym, vdecl); 1.36 + unrefdResources.enter(vdecl.sym); 1.37 + resourceVarDecls.append(vdecl); 1.38 } else if (resource instanceof JCExpression) { 1.39 scanExpr((JCExpression) resource); 1.40 } else { 1.41 @@ -1049,11 +1049,14 @@ 1.42 Bits uninitsEnd = uninits; 1.43 int nextadrCatch = nextadr; 1.44 1.45 - if (!unrefdResources.isEmpty() && 1.46 + if (!resourceVarDecls.isEmpty() && 1.47 lint.isEnabled(Lint.LintCategory.TRY)) { 1.48 - for (Map.Entry<VarSymbol, JCVariableDecl> e : unrefdResources.entrySet()) { 1.49 - log.warning(Lint.LintCategory.TRY, e.getValue().pos(), 1.50 - "try.resource.not.referenced", e.getKey()); 1.51 + for (JCVariableDecl resVar : resourceVarDecls) { 1.52 + if (unrefdResources.includes(resVar.sym)) { 1.53 + log.warning(Lint.LintCategory.TRY, resVar.pos(), 1.54 + "try.resource.not.referenced", resVar.sym); 1.55 + unrefdResources.remove(resVar.sym); 1.56 + } 1.57 } 1.58 } 1.59 1.60 @@ -1143,7 +1146,6 @@ 1.61 while (exits.nonEmpty()) pendingExits.append(exits.next()); 1.62 } 1.63 uninitsTry.andSet(uninitsTryPrev).andSet(uninits); 1.64 - unrefdResources = unrefdResourcesPrev; 1.65 } 1.66 1.67 public void visitConditional(JCConditional tree) { 1.68 @@ -1369,9 +1371,7 @@ 1.69 } 1.70 1.71 void referenced(Symbol sym) { 1.72 - if (unrefdResources != null && unrefdResources.containsKey(sym)) { 1.73 - unrefdResources.remove(sym); 1.74 - } 1.75 + unrefdResources.remove(sym); 1.76 } 1.77 1.78 public void visitTypeCast(JCTypeCast tree) { 1.79 @@ -1430,6 +1430,7 @@ 1.80 alive = true; 1.81 this.thrown = this.caught = null; 1.82 this.classDef = null; 1.83 + unrefdResources = new Scope(env.enclClass.sym); 1.84 scan(tree); 1.85 } finally { 1.86 // note that recursive invocations of this method fail hard 1.87 @@ -1444,6 +1445,7 @@ 1.88 this.make = null; 1.89 this.thrown = this.caught = null; 1.90 this.classDef = null; 1.91 + unrefdResources = null; 1.92 } 1.93 } 1.94 }
2.1 --- /dev/null Thu Jan 01 00:00:00 1970 +0000 2.2 +++ b/test/tools/javac/TryWithResources/UnusedResourcesTest.java Thu Mar 03 09:43:24 2011 +0000 2.3 @@ -0,0 +1,250 @@ 2.4 +/* 2.5 + * Copyright (c) 2011, Oracle and/or its affiliates. All rights reserved. 2.6 + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. 2.7 + * 2.8 + * This code is free software; you can redistribute it and/or modify it 2.9 + * under the terms of the GNU General Public License version 2 only, as 2.10 + * published by the Free Software Foundation. 2.11 + * 2.12 + * This code is distributed in the hope that it will be useful, but WITHOUT 2.13 + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or 2.14 + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License 2.15 + * version 2 for more details (a copy is included in the LICENSE file that 2.16 + * accompanied this code). 2.17 + * 2.18 + * You should have received a copy of the GNU General Public License version 2.19 + * 2 along with this work; if not, write to the Free Software Foundation, 2.20 + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. 2.21 + * 2.22 + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA 2.23 + * or visit www.oracle.com if you need additional information or have any 2.24 + * questions. 2.25 + */ 2.26 + 2.27 +/* 2.28 + * @test 2.29 + * @bug 7023233 2.30 + * @summary False positive for -Xlint:try with nested try with resources blocks 2.31 + */ 2.32 + 2.33 +import com.sun.source.util.JavacTask; 2.34 +import com.sun.tools.javac.api.JavacTool; 2.35 +import com.sun.tools.javac.util.JCDiagnostic; 2.36 +import java.net.URI; 2.37 +import java.util.Arrays; 2.38 +import javax.tools.Diagnostic; 2.39 +import javax.tools.JavaCompiler; 2.40 +import javax.tools.JavaFileObject; 2.41 +import javax.tools.SimpleJavaFileObject; 2.42 +import javax.tools.StandardJavaFileManager; 2.43 +import javax.tools.ToolProvider; 2.44 + 2.45 +public class UnusedResourcesTest { 2.46 + 2.47 + enum XlintOption { 2.48 + NONE("none"), 2.49 + TRY("try"); 2.50 + 2.51 + String opt; 2.52 + 2.53 + XlintOption(String opt) { 2.54 + this.opt = opt; 2.55 + } 2.56 + 2.57 + String getXlintOption() { 2.58 + return "-Xlint:" + opt; 2.59 + } 2.60 + } 2.61 + 2.62 + enum TwrStmt { 2.63 + TWR1("res1"), 2.64 + TWR2("res2"), 2.65 + TWR3("res3"); 2.66 + 2.67 + final String resourceName; 2.68 + 2.69 + private TwrStmt(String resourceName) { 2.70 + this.resourceName = resourceName; 2.71 + } 2.72 + } 2.73 + 2.74 + enum SuppressLevel { 2.75 + NONE, 2.76 + SUPPRESS; 2.77 + 2.78 + String getSuppressAnno() { 2.79 + return this == SUPPRESS ? 2.80 + "@SuppressWarnings(\"try\")" : 2.81 + ""; 2.82 + } 2.83 + } 2.84 + 2.85 + enum ResourceUsage { 2.86 + NONE(null), 2.87 + USE_R1(TwrStmt.TWR1), 2.88 + USE_R2(TwrStmt.TWR2), 2.89 + USE_R3(TwrStmt.TWR3); 2.90 + 2.91 + TwrStmt stmt; 2.92 + 2.93 + private ResourceUsage(TwrStmt stmt) { 2.94 + this.stmt = stmt; 2.95 + } 2.96 + 2.97 + String usedResourceName() { 2.98 + return stmt != null ? stmt.resourceName : null; 2.99 + } 2.100 + 2.101 + boolean isUsedIn(TwrStmt res, TwrStmt stmt) { 2.102 + return this.stmt == res && 2.103 + stmt.ordinal() >= this.stmt.ordinal(); 2.104 + } 2.105 + 2.106 + String getUsage(TwrStmt stmt) { 2.107 + return this != NONE && stmt.ordinal() >= this.stmt.ordinal() ? 2.108 + "use(" + usedResourceName() + ");" : 2.109 + ""; 2.110 + } 2.111 + } 2.112 + 2.113 + static class JavaSource extends SimpleJavaFileObject { 2.114 + 2.115 + String template = "class Resource implements AutoCloseable {\n" + 2.116 + "public void close() {}\n" + 2.117 + "}\n" + 2.118 + "class Test {\n" + 2.119 + "void use(Resource r) {}\n" + 2.120 + "#S void test() {\n" + 2.121 + "try (Resource #R1 = new Resource()) {\n" + 2.122 + "#U1_R1\n" + 2.123 + "#U1_R2\n" + 2.124 + "#U1_R3\n" + 2.125 + "try (Resource #R2 = new Resource()) {\n" + 2.126 + "#U2_R1\n" + 2.127 + "#U2_R2\n" + 2.128 + "#U2_R3\n" + 2.129 + "try (Resource #R3 = new Resource()) {\n" + 2.130 + "#U3_R1\n" + 2.131 + "#U3_R2\n" + 2.132 + "#U3_R3\n" + 2.133 + "}\n" + 2.134 + "}\n" + 2.135 + "}\n" + 2.136 + "}\n" + 2.137 + "}\n"; 2.138 + 2.139 + String source; 2.140 + 2.141 + public JavaSource(SuppressLevel suppressLevel, ResourceUsage usage1, 2.142 + ResourceUsage usage2, ResourceUsage usage3) { 2.143 + super(URI.create("myfo:/Test.java"), JavaFileObject.Kind.SOURCE); 2.144 + source = template.replace("#S", suppressLevel.getSuppressAnno()). 2.145 + replace("#R1", TwrStmt.TWR1.resourceName). 2.146 + replace("#R2", TwrStmt.TWR2.resourceName). 2.147 + replace("#R3", TwrStmt.TWR3.resourceName). 2.148 + replace("#U1_R1", usage1.getUsage(TwrStmt.TWR1)). 2.149 + replace("#U1_R2", usage2.getUsage(TwrStmt.TWR1)). 2.150 + replace("#U1_R3", usage3.getUsage(TwrStmt.TWR1)). 2.151 + replace("#U2_R1", usage1.getUsage(TwrStmt.TWR2)). 2.152 + replace("#U2_R2", usage2.getUsage(TwrStmt.TWR2)). 2.153 + replace("#U2_R3", usage3.getUsage(TwrStmt.TWR2)). 2.154 + replace("#U3_R1", usage1.getUsage(TwrStmt.TWR3)). 2.155 + replace("#U3_R2", usage2.getUsage(TwrStmt.TWR3)). 2.156 + replace("#U3_R3", usage3.getUsage(TwrStmt.TWR3)); 2.157 + } 2.158 + 2.159 + @Override 2.160 + public CharSequence getCharContent(boolean ignoreEncodingErrors) { 2.161 + return source; 2.162 + } 2.163 + } 2.164 + 2.165 + public static void main(String... args) throws Exception { 2.166 + for (XlintOption xlint : XlintOption.values()) { 2.167 + for (SuppressLevel suppressLevel : SuppressLevel.values()) { 2.168 + for (ResourceUsage usage1 : ResourceUsage.values()) { 2.169 + for (ResourceUsage usage2 : ResourceUsage.values()) { 2.170 + for (ResourceUsage usage3 : ResourceUsage.values()) { 2.171 + test(xlint, 2.172 + suppressLevel, 2.173 + usage1, 2.174 + usage2, 2.175 + usage3); 2.176 + } 2.177 + } 2.178 + } 2.179 + } 2.180 + } 2.181 + } 2.182 + 2.183 + // Create a single file manager and reuse it for each compile to save time. 2.184 + static StandardJavaFileManager fm = JavacTool.create().getStandardFileManager(null, null, null); 2.185 + 2.186 + static void test(XlintOption xlint, SuppressLevel suppressLevel, ResourceUsage usage1, 2.187 + ResourceUsage usage2, ResourceUsage usage3) throws Exception { 2.188 + final JavaCompiler tool = ToolProvider.getSystemJavaCompiler(); 2.189 + JavaSource source = new JavaSource(suppressLevel, usage1, usage2, usage3); 2.190 + DiagnosticChecker dc = new DiagnosticChecker(); 2.191 + JavacTask ct = (JavacTask)tool.getTask(null, fm, dc, 2.192 + Arrays.asList(xlint.getXlintOption()), null, Arrays.asList(source)); 2.193 + ct.analyze(); 2.194 + check(source, xlint, suppressLevel, usage1, usage2, usage3, dc); 2.195 + } 2.196 + 2.197 + static void check(JavaSource source, XlintOption xlint, SuppressLevel suppressLevel, 2.198 + ResourceUsage usage1, ResourceUsage usage2, ResourceUsage usage3, DiagnosticChecker dc) { 2.199 + 2.200 + ResourceUsage[] usages = { usage1, usage2, usage3 }; 2.201 + boolean[] unusedFound = { dc.unused_r1, dc.unused_r2, dc.unused_r3 }; 2.202 + boolean[] usedResources = { false, false, false }; 2.203 + 2.204 + for (TwrStmt res : TwrStmt.values()) { 2.205 + outer: for (TwrStmt stmt : TwrStmt.values()) { 2.206 + for (ResourceUsage usage : usages) { 2.207 + if (usage.isUsedIn(res, stmt)) { 2.208 + usedResources[res.ordinal()] = true; 2.209 + break outer; 2.210 + } 2.211 + } 2.212 + } 2.213 + } 2.214 + 2.215 + for (TwrStmt stmt : TwrStmt.values()) { 2.216 + boolean unused = !usedResources[stmt.ordinal()] && 2.217 + xlint == XlintOption.TRY && 2.218 + suppressLevel != SuppressLevel.SUPPRESS; 2.219 + if (unused != unusedFound[stmt.ordinal()]) { 2.220 + throw new Error("invalid diagnostics for source:\n" + 2.221 + source.getCharContent(true) + 2.222 + "\nOptions: " + xlint.getXlintOption() + 2.223 + "\nFound unused res1: " + unusedFound[0] + 2.224 + "\nFound unused res2: " + unusedFound[1] + 2.225 + "\nFound unused res3: " + unusedFound[2] + 2.226 + "\nExpected unused res1: " + !usedResources[0] + 2.227 + "\nExpected unused res2: " + !usedResources[1] + 2.228 + "\nExpected unused res3: " + !usedResources[2]); 2.229 + } 2.230 + } 2.231 + } 2.232 + 2.233 + static class DiagnosticChecker implements javax.tools.DiagnosticListener<JavaFileObject> { 2.234 + 2.235 + boolean unused_r1 = false; 2.236 + boolean unused_r2 = false; 2.237 + boolean unused_r3 = false; 2.238 + 2.239 + public void report(Diagnostic<? extends JavaFileObject> diagnostic) { 2.240 + if (diagnostic.getKind() == Diagnostic.Kind.WARNING && 2.241 + diagnostic.getCode().contains("try.resource.not.referenced")) { 2.242 + String varName = ((JCDiagnostic)diagnostic).getArgs()[0].toString(); 2.243 + if (varName.equals(TwrStmt.TWR1.resourceName)) { 2.244 + unused_r1 = true; 2.245 + } else if (varName.equals(TwrStmt.TWR2.resourceName)) { 2.246 + unused_r2 = true; 2.247 + } else if (varName.equals(TwrStmt.TWR3.resourceName)) { 2.248 + unused_r3 = true; 2.249 + } 2.250 + } 2.251 + } 2.252 + } 2.253 +}