diff options
author | Stephan Herrmann | 2018-11-30 17:48:29 +0000 |
---|---|---|
committer | Stephan Herrmann | 2018-11-30 17:48:29 +0000 |
commit | bff823d336ce03dac0d247c2ec5ced05ed4343ba (patch) | |
tree | dd5cee28ac9da73fb5122adf6457b80eb812acc3 | |
parent | 2ea4c6b7bac849abcf85bfee1ba560eb0656856e (diff) | |
download | eclipse.jdt.core-bff823d336ce03dac0d247c2ec5ced05ed4343ba.tar.gz eclipse.jdt.core-bff823d336ce03dac0d247c2ec5ced05ed4343ba.tar.xz eclipse.jdt.core-bff823d336ce03dac0d247c2ec5ced05ed4343ba.zip |
Bug 541759 - [resource] don't suggest using t-w-r for a foreach elementI20181205-0600I20181204-1800I20181204-0600I20181203-1800
variable
Change-Id: I18f766ec36fa8e2c4bc25fafaa0e01bc3d40e388
3 files changed, 75 insertions, 7 deletions
diff --git a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/ResourceLeakTests.java b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/ResourceLeakTests.java index 765e10dfac..0d2d7a5b82 100644 --- a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/ResourceLeakTests.java +++ b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/ResourceLeakTests.java @@ -5437,4 +5437,63 @@ public void testBug473317() { "----------\n", compilerOptions); } +public void testBug541705() { + if (this.complianceLevel < ClassFileConstants.JDK1_7) return; // uses diamond + Runner runner = new Runner(); + runner.customOptions = getCompilerOptions(); + runner.customOptions.put(CompilerOptions.OPTION_ReportExplicitlyClosedAutoCloseable, CompilerOptions.ERROR); + runner.testFiles = new String[] { + "Test.java", + "import java.util.*;\n" + + "import java.util.zip.*;\n" + + "import java.io.*;\n" + + "public class Test {\n" + + " private static HashMap<String, ZipFile> fgZipFileCache = new HashMap<>(5);\n" + + " public static void closeArchives() {\n" + + " synchronized (fgZipFileCache) {\n" + + " for (ZipFile file : fgZipFileCache.values()) {\n" + + " synchronized (file) {\n" + + " try {\n" + + " file.close();\n" + + " } catch (IOException e) {\n" + + " System.out.println(e);\n" + + " }\n" + + " }\n" + + " }\n" + + " fgZipFileCache.clear();\n" + + " }\n" + + " }\n" + + "}\n" + }; + runner.runConformTest(); +} +public void testBug541705b() { + if (this.complianceLevel < ClassFileConstants.JDK9) return; // variable used in t-w-r + Runner runner = new Runner(); + runner.customOptions = getCompilerOptions(); + runner.customOptions.put(CompilerOptions.OPTION_ReportExplicitlyClosedAutoCloseable, CompilerOptions.ERROR); + runner.testFiles = new String[] { + "Test.java", + "import java.util.*;\n" + + "import java.util.zip.*;\n" + + "import java.io.*;\n" + + "public class Test {\n" + + " private static HashMap<String, ZipFile> fgZipFileCache = new HashMap<>(5);\n" + + " public static void closeArchives() {\n" + + " synchronized (fgZipFileCache) {\n" + + " for (ZipFile file : fgZipFileCache.values()) {\n" + + " synchronized (file) {\n" + + " try (file) {\n" + + " } catch (IOException e) {\n" + + " System.out.println(e);\n" + + " }\n" + + " }\n" + + " }\n" + + " fgZipFileCache.clear();\n" + + " }\n" + + " }\n" + + "}\n" + }; + runner.runConformTest(); +} } diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/FakedTrackingVariable.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/FakedTrackingVariable.java index d09322585c..8498b31c09 100644 --- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/FakedTrackingVariable.java +++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/FakedTrackingVariable.java @@ -79,7 +79,9 @@ public class FakedTrackingVariable extends LocalDeclaration { private static final int REPORTED_POTENTIAL_LEAK = 32; // a location independent definitive problem has been reported against this resource: private static final int REPORTED_DEFINITIVE_LEAK = 64; - + // a local declarations that acts as the element variable of a foreach loop (should never suggest to use t-w-r): + private static final int FOREACH_ELEMENT_VAR = 128; + public static boolean TEST_372319 = false; // see https://bugs.eclipse.org/372319 /** @@ -449,9 +451,9 @@ public class FakedTrackingVariable extends LocalDeclaration { if (rhsTrackVar.originalBinding != null) local.closeTracker = rhsTrackVar; // a.: let fresh LHS share it if (rhsTrackVar.currentAssignment == location) { - // pre-set tracker from lhs - passed from outside? + // pre-set tracker from lhs - passed from outside (or foreach)? // now it's a fresh resource - rhsTrackVar.globalClosingState &= ~(SHARED_WITH_OUTSIDE|OWNED_BY_OUTSIDE); + rhsTrackVar.globalClosingState &= ~(SHARED_WITH_OUTSIDE|OWNED_BY_OUTSIDE|FOREACH_ELEMENT_VAR); } } else { if (rhs instanceof AllocationExpression || rhs instanceof ConditionalExpression) { @@ -484,7 +486,7 @@ public class FakedTrackingVariable extends LocalDeclaration { } } // re-assigning from a fresh value, mark as not-closed again: - if ((previousTracker.globalClosingState & (SHARED_WITH_OUTSIDE|OWNED_BY_OUTSIDE)) == 0 + if ((previousTracker.globalClosingState & (SHARED_WITH_OUTSIDE|OWNED_BY_OUTSIDE|FOREACH_ELEMENT_VAR)) == 0 && flowInfo.hasNullInfoFor(previousTracker.binding)) // avoid spilling info into a branch that doesn't see the corresponding resource flowInfo.markAsDefinitelyNull(previousTracker.binding); local.closeTracker = analyseCloseableExpression(flowInfo, flowContext, local, location, rhs, previousTracker); @@ -494,7 +496,7 @@ public class FakedTrackingVariable extends LocalDeclaration { if (rhsTrackVar != null) { local.closeTracker = rhsTrackVar; // a fresh resource, mark as not-closed: - if ((rhsTrackVar.globalClosingState & (SHARED_WITH_OUTSIDE|OWNED_BY_OUTSIDE)) == 0) + if ((rhsTrackVar.globalClosingState & (SHARED_WITH_OUTSIDE|OWNED_BY_OUTSIDE|FOREACH_ELEMENT_VAR)) == 0) flowInfo.markAsDefinitelyNull(rhsTrackVar.binding); // TODO(stephan): this might be useful, but I could not find a test case for it: // if (flowContext.initsOnFinally != null) @@ -753,6 +755,12 @@ public class FakedTrackingVariable extends LocalDeclaration { return flowInfo; } + public static void markForeachElementVar(LocalDeclaration local) { + if (local.binding != null && local.binding.closeTracker != null) { + local.binding.closeTracker.globalClosingState |= FOREACH_ELEMENT_VAR; + } + } + /** * Iterator for a set of FakedTrackingVariable, which dispenses the elements * according to the priorities defined by enum {@link Stage}. @@ -987,7 +995,7 @@ public class FakedTrackingVariable extends LocalDeclaration { } public void reportExplicitClosing(ProblemReporter problemReporter) { - if ((this.globalClosingState & (OWNED_BY_OUTSIDE|REPORTED_EXPLICIT_CLOSE)) == 0) { // can't use t-w-r for OWNED_BY_OUTSIDE + if ((this.globalClosingState & (OWNED_BY_OUTSIDE|REPORTED_EXPLICIT_CLOSE|FOREACH_ELEMENT_VAR)) == 0) { // can't use t-w-r for OWNED_BY_OUTSIDE this.globalClosingState |= REPORTED_EXPLICIT_CLOSE; problemReporter.explicitlyClosedAutoCloseable(this); } diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/ForeachStatement.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/ForeachStatement.java index 26e30f772b..fe438b0220 100644 --- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/ForeachStatement.java +++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/ForeachStatement.java @@ -99,7 +99,7 @@ public class ForeachStatement extends Statement { int initialComplaintLevel = (flowInfo.reachMode() & FlowInfo.UNREACHABLE) != 0 ? Statement.COMPLAINED_FAKE_REACHABLE : Statement.NOT_COMPLAINED; // process the element variable and collection - flowInfo = this.elementVariable.analyseCode(this.scope, flowContext, flowInfo); + flowInfo = this.elementVariable.analyseCode(this.scope, flowContext, flowInfo); FlowInfo condInfo = this.collection.analyseCode(this.scope, flowContext, flowInfo.copy()); this.collection.checkNPE(currentScope, flowContext, condInfo.copy(), 1); LocalVariableBinding elementVarBinding = this.elementVariable.binding; @@ -132,6 +132,7 @@ public class ForeachStatement extends Statement { if (this.action.complainIfUnreachable(actionInfo, this.scope, initialComplaintLevel, true) < Statement.COMPLAINED_UNREACHABLE) { actionInfo = this.action.analyseCode(this.scope, loopingContext, actionInfo).unconditionalCopy(); if (this.action instanceof Block) { + FakedTrackingVariable.markForeachElementVar(this.elementVariable); // action.analyseCode() missed the following check due to identical scopes of ForeachStatement and Block: this.scope.checkUnclosedCloseables(actionInfo, loopingContext, null, null); } |