From bff823d336ce03dac0d247c2ec5ced05ed4343ba Mon Sep 17 00:00:00 2001 From: Stephan Herrmann Date: Fri, 30 Nov 2018 18:48:29 +0100 Subject: Bug 541759 - [resource] don't suggest using t-w-r for a foreach element variable Change-Id: I18f766ec36fa8e2c4bc25fafaa0e01bc3d40e388 --- .../internal/compiler/ast/FakedTrackingVariable.java | 20 ++++++++++++++------ .../jdt/internal/compiler/ast/ForeachStatement.java | 3 ++- 2 files changed, 16 insertions(+), 7 deletions(-) (limited to 'org.eclipse.jdt.core') 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); } -- cgit v1.2.3