diff options
author | Stephan Herrmann | 2012-08-25 16:04:34 +0000 |
---|---|---|
committer | Stephan Herrmann | 2012-08-25 16:04:34 +0000 |
commit | 6d141275326cf4caf65ec5dca68b565e2e9b1360 (patch) | |
tree | 56484a0d3b4a6a02eaa8edbfc92df5e7656bacff | |
parent | 2eab698c606e3b17217602bbc99837242c743a5a (diff) | |
download | eclipse.jdt.core-6d141275326cf4caf65ec5dca68b565e2e9b1360.tar.gz eclipse.jdt.core-6d141275326cf4caf65ec5dca68b565e2e9b1360.tar.xz eclipse.jdt.core-6d141275326cf4caf65ec5dca68b565e2e9b1360.zip |
Fixed Bug 345305 - [compiler][null] Compiler misidentifies a case of
"variable can only be null"
Fixed by decoupling null analysis for try-finally from def.assign
analysis.
Change-Id: I17285d5034aa6cdcc390c41bbde121cb024b648c
38 files changed, 796 insertions, 921 deletions
diff --git a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/AssignmentTest.java b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/AssignmentTest.java index 8a0b1d2e29..fa1fba243a 100644 --- a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/AssignmentTest.java +++ b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/AssignmentTest.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2005, 2011 IBM Corporation and others. + * Copyright (c) 2005, 2012 IBM Corporation and others. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at @@ -7,12 +7,15 @@ * * Contributors: * IBM Corporation - initial API and implementation - * Stephan Herrmann <stephan@cs.tu-berlin.de> - Contribution for bug 185682 - Increment/decrement operators mark local variables as read + * Stephan Herrmann <stephan@cs.tu-berlin.de> - Contributions for + * bug 185682 - Increment/decrement operators mark local variables as read + * bug 345305 - [compiler][null] Compiler misidentifies a case of "variable can only be null" *******************************************************************************/ package org.eclipse.jdt.core.tests.compiler.regression; import java.util.Map; +import org.eclipse.jdt.core.JavaCore; import org.eclipse.jdt.internal.compiler.classfmt.ClassFileConstants; import org.eclipse.jdt.internal.compiler.impl.CompilerOptions; @@ -369,7 +372,9 @@ public void test033() { //https://bugs.eclipse.org/bugs/show_bug.cgi?id=84215 //TODO (philippe) should move to InitializationTest suite public void test034() { - this.runConformTest( + Map options = getCompilerOptions(); + options.put(JavaCore.COMPILER_PB_UNUSED_PRIVATE_MEMBER, JavaCore.IGNORE); + this.runNegativeTest( new String[] { "X.java", "public final class X \n" + @@ -748,7 +753,15 @@ public void test034() { " private String adsyasta;\n" + "}\n", }, - ""); + "----------\n" + + "1. ERROR in X.java (at line 356)\n" + + " if (rs != null)\n" + + " ^^\n" + + "Redundant null check: The variable rs cannot be null at this location\n" + + "----------\n", + null/*classLibs*/, + true/*shouldFlush*/, + options); } /* * Check scenario: i = i++ diff --git a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/NullReferenceImplTests.java b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/NullReferenceImplTests.java index ba863e5816..8f0a55ad96 100644 --- a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/NullReferenceImplTests.java +++ b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/NullReferenceImplTests.java @@ -7,7 +7,9 @@ * * Contributors: * IBM Corporation - initial API and implementation - * Stephan Herrmann <stephan@cs.tu-berlin.de> - Contribution for bug 320170 + * Stephan Herrmann <stephan@cs.tu-berlin.de> - Contributions for + * bug 320170 + * bug 345305 - [compiler][null] Compiler misidentifies a case of "variable can only be null" *******************************************************************************/ package org.eclipse.jdt.core.tests.compiler.regression; @@ -30,7 +32,6 @@ import junit.framework.Test; import junit.framework.TestSuite; import org.eclipse.jdt.internal.compiler.flow.FlowInfo; -import org.eclipse.jdt.internal.compiler.flow.NullInfoRegistry; import org.eclipse.jdt.internal.compiler.flow.UnconditionalFlowInfo; import org.eclipse.jdt.internal.compiler.flow.UnconditionalFlowInfo.AssertionFailedException; import org.eclipse.jdt.internal.compiler.impl.Constant; @@ -574,11 +575,6 @@ public void test2062_mergedWith() { assertTrue("nb of failures: " + failures, failures == 0); } -public void test2070_newNullInfoRegistry() { - int failures = NullReferenceImplTransformations.newNullInfoRegistry.test(); - assertTrue("nb of failures: " + failures, failures == 0); -} - // PREMATURE rewrite from scratch //public void _test2058_recode() { // long [][][] testData = transitionsTablesData[recode]; @@ -1463,135 +1459,6 @@ static String testString(UnconditionalFlowInfo zis, int position) { } } } -/** - * A class meant to augment - * @link{org.eclipse.jdt.internal.compiler.flow.NullInfoRegistry} with - * capabilities in the test domain. It especially provides factories to build - * fake flow info instances for use in state transitions validation. - */ -/* - * The reason why UnconditionalFlowInfoTestHarness and this class were - * separated is that NullInfoRegistry redefines part of the markAs* methods, - * in effect preventing a harness extending NullInfoRegistry to access - * UnconditionalFlowInfo implementations of the said methods. - */ -class NullInfoRegistryTestHarness extends NullInfoRegistry { - private int testPosition; - -private NullInfoRegistryTestHarness() { - super(FlowInfo.DEAD_END); -} - - // Interface -/** - * Return the state represented by this. - * @return the state represented by this - */ -NullReferenceImplTests.State asState() { - return UnconditionalFlowInfoTestHarness.asState(this, 0); -} - -/** - * Return the state represented by this for a variable encoded at a given position. - * @param position - int the position of the considered variable - * @return the state represented by this for a variable encoded at a given position - */ -NullReferenceImplTests.State asState(int position) { - return UnconditionalFlowInfoTestHarness.asState(this, position); -} - -public FlowInfo copy() { - NullInfoRegistryTestHarness copy = - new NullInfoRegistryTestHarness(); - copy.testPosition = this.testPosition; - UnconditionalFlowInfoTestHarness.copy(this, copy); - return copy; -} - -/** - * Return a fake null info registry derived from an unconditional flow - * info. - * @param upstream - UnconditionalFlowInfoTestHarness the upstream flow info - * @return a fake null info registry derived from upstream - */ -public static NullInfoRegistryTestHarness testNullInfoRegistry( - UnconditionalFlowInfoTestHarness upstream) { - NullInfoRegistry nullInfoRegistry = new NullInfoRegistry(upstream); - NullInfoRegistryTestHarness result = - new NullInfoRegistryTestHarness(); - result.testPosition = upstream.testPosition; - if (result.testPosition < BitCacheSize) { - result.nullBit1 = nullInfoRegistry.nullBit1; - result.nullBit2 = nullInfoRegistry.nullBit2; - result.nullBit3 = nullInfoRegistry.nullBit3; - result.nullBit4 = nullInfoRegistry.nullBit4; -// result.nullBit5 = nullInfoRegistry.nullBit5; -// result.nullBit6 = nullInfoRegistry.nullBit6; - } - else if ((nullInfoRegistry.tagBits & NULL_FLAG_MASK) != 0){ - int vectorIndex = (result.testPosition / BitCacheSize) - 1, - length = vectorIndex + 1; - result.extra = new long[extraLength][]; - result.extra[0] = new long[length]; - result.extra[1] = new long[length]; - for (int j = 2; j < extraLength; j++) { - result.extra[j] = new long[length]; - result.extra[j][vectorIndex] = nullInfoRegistry.extra[j][vectorIndex]; - } - } - if ((nullInfoRegistry.tagBits & NULL_FLAG_MASK) != 0) { - result.tagBits |= NULL_FLAG_MASK; - } - result.maxFieldCount = 0; - return result; -} - -/** - * Return true iff this flow info can be considered as equal to the one passed - * in parameter. - * @param other the flow info to compare to - * @return true iff this flow info compares equal to other - */ -public boolean testEquals(UnconditionalFlowInfo other) { - return UnconditionalFlowInfoTestHarness.testEquals(this, other); -} - -/** - * Return true iff this flow info can be considered as equal to the one passed - * in parameter in respect with a single local variable which id would be - * position in a class with no field. - * @param other the flow info to compare to - * @param position the position of the local to consider - * @return true iff this flow info compares equal to other for a given local - */ -public boolean testEquals(UnconditionalFlowInfo other, int position) { - return UnconditionalFlowInfoTestHarness.testEquals(this, other, position); -} - -/** - * Return a string suitable for use as a representation of this flow info - * within test series. - * @return a string suitable for use as a representation of this flow info - */ -public String testString() { - if (this == DEAD_END) { - return "FlowInfo.DEAD_END"; //$NON-NLS-1$ - } - return UnconditionalFlowInfoTestHarness.testString(this, this.testPosition); -} - -/** - * Return a string suitable for use as a representation of this flow info - * within test series. - * @param position a position to consider instead of this flow info default - * test position - * @return a string suitable for use as a representation of this flow info - */ -public String testString(int position) { - return UnconditionalFlowInfoTestHarness.testString(this, position); -} -} - interface CodeAnalysis { public static final String definitionStartMarker = "DEFINITION START", diff --git a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/NullReferenceImplTransformations.java b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/NullReferenceImplTransformations.java index be763c83c0..7162b30921 100644 --- a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/NullReferenceImplTransformations.java +++ b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/NullReferenceImplTransformations.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2005, 2010 IBM Corporation and others. + * Copyright (c) 2005, 2012 IBM Corporation and others. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at @@ -7,6 +7,8 @@ * * Contributors: * IBM Corporation - initial API and implementation + * Stephan Herrmann - Contribution for + * bug 345305 - [compiler][null] Compiler misidentifies a case of "variable can only be null" *******************************************************************************/ package org.eclipse.jdt.core.tests.compiler.regression; @@ -1276,47 +1278,6 @@ public class NullReferenceImplTransformations { UnconditionalFlowInfo input2) { return input1.copy().mergedWith(input2); } - }, - newNullInfoRegistry = - // newNullInfoRegistry DEFINITION START - // start => start - // prot. non null => start - // prot. null => start - // pot. unknown => start - // pot. non null => start - // pot. nn & prot. nn => start - // pot. nn & pot. un => start - // pot. null => start - // pot. n & prot. n => start - // pot. n & pot. un => start - // pot. n & pot. nn => start - // def. unknown => pot. unknown - // def. non null => pot. non null - // def. null => pot. null - // newNullInfoRegistry DEFINITION END - new CreationalTransformation("newNullInfoRegistry", - new byte[][] { - // newNullInfoRegistry INITIALIZER START - {0x00,0x00}, - {0x04,0x00}, - {0x08,0x00}, - {0x0C,0x00}, - {0x10,0x00}, - {0x14,0x00}, - {0x18,0x00}, - {0x24,0x04}, - {0x28,0x08}, - {0x2C,0x00}, - {0x30,0x10}, - {0x34,0x00}, - {0x38,0x00}, - {0x3C,0x00}, - // newNullInfoRegistry INITIALIZER END - }) { - UnconditionalFlowInfo output(UnconditionalFlowInfo input) { - return NullInfoRegistryTestHarness. - testNullInfoRegistry((UnconditionalFlowInfoTestHarness) input); - } }; public static final Transformation[] transformations = { markAsComparedEqualToNonNull, @@ -1326,8 +1287,7 @@ public class NullReferenceImplTransformations { markAsDefinitelyUnknown, addInitializationsFrom, addPotentialInitializationsFrom, - mergedWith, - newNullInfoRegistry + mergedWith }; public abstract static class Transformation { public String name; @@ -1984,69 +1944,6 @@ int test() { return this.failuresNb; } } -abstract static class CreationalTransformation extends TwoDimensionalTransformation { -CreationalTransformation(String name, byte[][] transitions) { - super(name, transitions); -} -void hydrate() { - if (this.computedTransitions == null) { - State input, output; - this.computedTransitions = new HashMap(State.stateMaxValue + 1); - for (int i = 0, length = State.states.length; i < length; i++) { - output = ((NullInfoRegistryTestHarness) - output(UnconditionalFlowInfoTestHarness. - testUnconditionalFlowInfo(input = State.states[i]), - TestLocalVariableBinding.local0)).asState(); - if (input.symbolic && !output.symbolic) { - System.err.println(this.name + " generates non-symbolic state " + - output + " upon entry: " + input); - } - this.computedTransitions.put(input, output); - } - } -} -UnconditionalFlowInfo output(UnconditionalFlowInfo input, TestLocalVariableBinding local) { - return output(input); -} -abstract UnconditionalFlowInfo output(UnconditionalFlowInfo input); -int test() { - Iterator transitions = this.initializedTransitions.entrySet().iterator(); - State input, expectedOutput, effectiveOutput; - Map.Entry transition; - this.failuresNb = 0; // reset - while (transitions.hasNext()) { - transition = (Map.Entry) transitions.next(); - input = (State) transition.getKey(); - expectedOutput = (State) transition.getValue(); - effectiveOutput = ((NullInfoRegistryTestHarness) - output(UnconditionalFlowInfoTestHarness. - testUnconditionalFlowInfo(input), - TestLocalVariableBinding.local0)).asState(); - if (effectiveOutput != expectedOutput) { - fail(); - System.out.println("\t\t" + input.printableBitsField + - " => " + effectiveOutput.printableBitsField + - " instead of: " + expectedOutput.printableBitsField); - } - } - transitions = this.initializedTransitions.entrySet().iterator(); - while (transitions.hasNext()) { - transition = (Map.Entry) transitions.next(); - input = (State) transition.getKey(); - expectedOutput = (State) transition.getValue(); - effectiveOutput = ((NullInfoRegistryTestHarness) - output(UnconditionalFlowInfoTestHarness. - testUnconditionalFlowInfo(input, 64))).asState(64); - if (effectiveOutput != expectedOutput) { - fail(); - System.out.println("\t\t" + input.printableBitsField + - " => " + effectiveOutput.printableBitsField + - " (64) instead of: " + expectedOutput.printableBitsField); - } - } - return this.failuresNb; -} -} public abstract static class ThreeDimensionalTransformation extends Transformation { private static final boolean CHECKING_ROW_NAMES = false; ThreeDimensionalTransformation(String name) { diff --git a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/NullReferenceTest.java b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/NullReferenceTest.java index 7d1b587f55..077a4468cd 100644 --- a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/NullReferenceTest.java +++ b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/NullReferenceTest.java @@ -23,6 +23,7 @@ * bug 360328 - [compiler][null] detect null problems in nested code (local class inside a loop) * bug 367879 - Incorrect "Potential null pointer access" warning on statement after try-with-resources within try-finally * bug 383690 - [compiler] location of error re uninitialized final field should be aligned + * bug 345305 - [compiler][null] Compiler misidentifies a case of "variable can only be null" *******************************************************************************/ package org.eclipse.jdt.core.tests.compiler.regression; @@ -50,7 +51,8 @@ public NullReferenceTest(String name) { // Only the highest compliance level is run; add the VM argument // -Dcompliance=1.4 (for example) to lower it if needed static { -// TESTS_NAMES = new String[] { "testBug336428f" }; +// TESTS_NAMES = new String[] { "testBug345305_14" }; +// TESTS_NAMES = new String[] { "test0515_try_finally" }; // TESTS_NUMBERS = new int[] { 561 }; // TESTS_RANGE = new int[] { 1, 2049 }; } @@ -4634,13 +4636,12 @@ public void test0504_try_finally() { // null analysis -- try/finally // origin: AssignmentTest#test017 -// The whole issue here is whether or not to detect premature exits. We -// follow JLS's conservative approach, which considers that the try -// block may exit before the assignment is completed. -// Note: conversely, without line 1, we would complain about x not being -// initialized (for sure) on line 2. +// The whole issue here is whether or not to detect premature exits. +// Previously, we followed JLS's conservative approach, which considers +// that the try block may exit before the assignment is completed. +// As of Bug 345305 this has been changed to a more accurate analysis. public void test0505_try_finally() { - this.runConformTest( + this.runNegativeTest( new String[] { "X.java", "public class X {\n" + @@ -4653,7 +4654,12 @@ public void test0505_try_finally() { " }\n" + " }\n" + "}\n"}, - ""); + "----------\n" + + "1. ERROR in X.java (at line 7)\n" + + " if (x == null) {/* */}\n" + + " ^\n" + + "Redundant null check: The variable x can only be null at this location\n" + + "----------\n"); } // null analysis -- try finally @@ -5819,7 +5825,7 @@ public void test0555_try_catch() { "1. ERROR in X.java (at line 9)\n" + " o.toString();\n" + " ^\n" + - "Potential null pointer access: The variable o may be null at this location\n" + + "Null pointer access: The variable o can only be null at this location\n" + "----------\n", JavacTestOptions.Excuse.EclipseWarningConfiguredAsError); } @@ -5848,7 +5854,7 @@ public void test0556_try_catch() { "1. ERROR in X.java (at line 9)\n" + " o.toString();\n" + " ^\n" + - "Potential null pointer access: The variable o may be null at this location\n" + + "Null pointer access: The variable o can only be null at this location\n" + "----------\n", JavacTestOptions.Excuse.EclipseWarningConfiguredAsError); } @@ -9404,7 +9410,7 @@ public void test1013() { } public void test1014() { - this.runNegativeTest( + this.runConformTest( new String[] { "X.java", "public class X {\n" + @@ -9417,13 +9423,7 @@ public void test1014() { " }\n" + " }\n" + "}\n"}, - "----------\n" + - "1. ERROR in X.java (at line 7)\n" + - " x.foo(null);\n" + - " ^\n" + - "Potential null pointer access: The variable x may be null at this location\n" + - "----------\n", - JavacTestOptions.Excuse.EclipseWarningConfiguredAsError); + ""); } public void test1015() { @@ -9493,7 +9493,12 @@ public void test1017() { " }\n" + "}\n"}, "----------\n" + - "1. ERROR in X.java (at line 8)\n" + + "1. ERROR in X.java (at line 7)\n" + + " if (x == null) {\n" + + " ^\n" + + "Redundant null check: The variable x can only be null at this location\n" + + "----------\n" + + "2. ERROR in X.java (at line 8)\n" + " x.foo(null);\n" + " ^\n" + "Null pointer access: The variable x can only be null at this location\n" + @@ -9741,7 +9746,7 @@ public void _test1026() { } public void test1027() { - this.runConformTest( + this.runNegativeTest( new String[] { "X.java", "public class X {\n" + @@ -9759,7 +9764,12 @@ public void test1027() { " if (o == null) return;\n" + " }\n" + "}\n"}, - ""); + "----------\n" + + "1. ERROR in X.java (at line 9)\n" + + " if (o == null) \n" + + " ^\n" + + "Redundant null check: The variable o can only be null at this location\n" + + "----------\n"); } // TODO (philippe) reenable once fixed @@ -9942,7 +9952,7 @@ public void test1033() { // from AssignmentTest#test034, simplified public void test1034() { - this.runConformTest( + this.runNegativeTest( new String[] { "X.java", "public final class X \n" + @@ -9975,7 +9985,12 @@ public void test1034() { " }\n" + "}\n", }, - ""); + "----------\n" + + "1. ERROR in X.java (at line 16)\n" + + " if (rs != null)\n" + + " ^^\n" + + "Redundant null check: The variable rs cannot be null at this location\n" + + "----------\n"); } public void test1036() { @@ -15608,4 +15623,402 @@ public void testBug360328d() { "",/* expected error */ JavacTestOptions.Excuse.EclipseWarningConfiguredAsError); } -}
\ No newline at end of file + +// Bug 345305 - [compiler][null] Compiler misidentifies a case of "variable can only be null" +// simplified: only try-finally involved +public void testBug345305_1() { + runConformTest( + new String[] { + "X.java", + "public class X {\n" + + " void foo() {\n" + + " String s = null;\n" + + " try {\n" + + " s = \"hi\";\n" + + " } finally {\n" + + " s.length();\n" + + " s = null;\n" + + " }\n" + + " }\n" + + "}\n" + }); +} + +// Bug 345305 - [compiler][null] Compiler misidentifies a case of "variable can only be null" +// original test case +public void testBug345305_2() { + runConformTest( + new String[] { + "X.java", + "public class X {\n" + + " void foo() {\n" + + " String s = null;\n" + + " while (true) {\n" + + " try {\n" + + " s = \"hi\";\n" + + " }\n" + + " finally {\n" + + " s.length();\n" + + " s = null;\n" + + " }\n" + + " }\n" + + " }\n" + + "}\n" + }); +} + +// Bug 345305 - [compiler][null] Compiler misidentifies a case of "variable can only be null" +// assignment in method argument position +public void testBug345305_3() { + runConformTest( + new String[] { + "X.java", + "public class X {\n" + + " void foo() {\n" + + " String s = null;\n" + + " while (true) {\n" + + " try {\n" + + " check(s = \"hi\");\n" + + " }\n" + + " finally {\n" + + " s.length();\n" + + " s = null;\n" + + " }\n" + + " }\n" + + " }\n" + + " void check(String s) {}\n" + + "}\n" + }); +} + +// Bug 345305 - [compiler][null] Compiler misidentifies a case of "variable can only be null" +// analysis of second local variable must not interfere +public void testBug345305_4() { + runNegativeTest( + new String[] { + "X.java", + "public class X {\n" + + " void foo() {\n" + + " String s = \"\";\n" + + " String s2 = null;\n" + + " while (true) {\n" + + " try {\n" + + " s = null;\n" + + " bar();\n" + + " s2 = \"world\";\n" + + " }\n" + + " finally {\n" + + " s.length();\n" + + " s = null;\n" + + " }\n" + + " }\n" + + " }\n" + + " void bar() {}\n" + + "}\n" + }, + "----------\n" + + "1. ERROR in X.java (at line 12)\n" + + " s.length();\n" + + " ^\n" + + "Null pointer access: The variable s can only be null at this location\n" + + "----------\n"); +} + +// Bug 345305 - [compiler][null] Compiler misidentifies a case of "variable can only be null" +// DISABLED: block-less if involved - info about pot.nn. is lost when checking against loop's info (deferred check) +public void _testBug345305_6() { + runNegativeTest( + new String[] { + "X.java", + "public class X {\n" + + " void foo(boolean b) {\n" + + " String s = null;\n" + + " while (true) {\n" + + " try {\n" + + " if (b)\n" + + " s = \"hi\";\n" + + " }\n" + + " finally {\n" + + " s.length();\n" + + " s = null;\n" + + " }\n" + + " }\n" + + " }\n" + + "}\n" + }, + "----------\n" + + "1. ERROR in X.java (at line 10)\n" + + " s.length();\n" + + " ^\n" + + "Potential null pointer access: The variable s may be null at this location\n" + + "----------\n"); +} + +// Bug 345305 - [compiler][null] Compiler misidentifies a case of "variable can only be null" +// block-less if involved +public void testBug345305_7() { + runNegativeTest( + new String[] { + "X.java", + "public class X {\n" + + " void foo(boolean b) {\n" + + " while (true) {\n" + + " String s = null;\n" + + " try {\n" + + " if (b)\n" + + " s = \"hi\";\n" + + " }\n" + + " finally {\n" + + " s.length();\n" + + " s = null;\n" + + " }\n" + + " }\n" + + " }\n" + + "}\n" + }, + "----------\n" + + "1. ERROR in X.java (at line 10)\n" + + " s.length();\n" + + " ^\n" + + "Potential null pointer access: The variable s may be null at this location\n" + + "----------\n"); +} + +// Bug 345305 - [compiler][null] Compiler misidentifies a case of "variable can only be null" +// consider exception thrown from cast expression +public void testBug345305_8() { + runNegativeTest( + new String[] { + "X.java", + "public class X {\n" + + " void foo(Object o) {\n" + + " while (true) {\n" + + " String s = null;\n" + + " try {\n" + + " s = (String) o;\n" + + " }\n" + + " finally {\n" + + " s.length();\n" + + " s = null;\n" + + " }\n" + + " }\n" + + " }\n" + + "}\n" + }, + "----------\n" + + "1. ERROR in X.java (at line 9)\n" + + " s.length();\n" + + " ^\n" + + "Potential null pointer access: The variable s may be null at this location\n" + + "----------\n"); +} + +// Bug 345305 - [compiler][null] Compiler misidentifies a case of "variable can only be null" +// consider exception thrown from binary expression +public void testBug345305_9() { + runNegativeTest( + new String[] { + "X.java", + "public class X {\n" + + " void foo(int i, int j) {\n" + + " while (true) {\n" + + " String s = null;\n" + + " try {\n" + + " s = ((i / j) == 3) ? \"3\" : \"not-3\";\n" + + " }\n" + + " finally {\n" + + " s.length();\n" + + " s = null;\n" + + " }\n" + + " }\n" + + " }\n" + + "}\n" + }, + "----------\n" + + "1. ERROR in X.java (at line 9)\n" + + " s.length();\n" + + " ^\n" + + "Potential null pointer access: The variable s may be null at this location\n" + + "----------\n"); +} + +// Bug 345305 - [compiler][null] Compiler misidentifies a case of "variable can only be null" +// inner labeled block with break +public void testBug345305_10() { + runNegativeTest( + new String[] { + "X.java", + "public class X {\n" + + " void foo(int j) {\n" + + " while (true) {\n" + + " String s = null;\n" + + " try {\n" + + " int i=0;\n" + + " block: {\n" + + " if (i++ == j)\n" + + " break block;\n" + + " s = \"\";\n" + + " return;\n" + + " }\n" + + " }\n" + + " finally {\n" + + " s.length();\n" + + " s = null;\n" + + " }\n" + + " }\n" + + " }\n" + + "}\n" + }, + "----------\n" + + "1. ERROR in X.java (at line 15)\n" + + " s.length();\n" + + " ^\n" + + "Potential null pointer access: The variable s may be null at this location\n" + + "----------\n"); +} + +// Bug 345305 - [compiler][null] Compiler misidentifies a case of "variable can only be null" +// switch statement +public void testBug345305_11() { + runNegativeTest( + new String[] { + "X.java", + "public class X {\n" + + " void foo(int j) {\n" + + " while (true) {\n" + + " String s = null;\n" + + " try {\n" + + " switch (j) {\n" + + " case 3:\n" + + " s = \"\";\n" + + " return;\n" + + " default: return;\n" + + " }\n" + + " }\n" + + " finally {\n" + + " s.length();\n" + + " s = null;\n" + + " }\n" + + " }\n" + + " }\n" + + "}\n" + }, + "----------\n" + + "1. ERROR in X.java (at line 14)\n" + + " s.length();\n" + + " ^\n" + + "Potential null pointer access: The variable s may be null at this location\n" + + "----------\n"); +} + +// Bug 345305 - [compiler][null] Compiler misidentifies a case of "variable can only be null" +// assignment inside conditional expression +public void testBug345305_12() { + runNegativeTest( + new String[] { + "X.java", + "public class X {\n" + + " String foo(boolean b) {\n" + + " while (true) {\n" + + " String s = null;\n" + + " try {\n" + + " return b ? (s = \"be\") : \"be not\";\n" + + " }\n" + + " finally {\n" + + " s.length();\n" + + " s = null;\n" + + " }\n" + + " }\n" + + " }\n" + + "}\n" + }, + "----------\n" + + "1. ERROR in X.java (at line 9)\n" + + " s.length();\n" + + " ^\n" + + "Potential null pointer access: The variable s may be null at this location\n" + + "----------\n"); +} + +// Bug 345305 - [compiler][null] Compiler misidentifies a case of "variable can only be null" +// explicit throw +public void testBug345305_13() { + runNegativeTest( + new String[] { + "X.java", + "public class X {\n" + + " String foo(boolean b) {\n" + + " while (true) {\n" + + " String s = null;\n" + + " RuntimeException ex = new RuntimeException();\n" + + " try {\n" + + " if (b)\n" + + " throw ex;\n" + + " s = \"be\";\n" + + " }\n" + + " finally {\n" + + " s.length();\n" + + " s = null;\n" + + " }\n" + + " }\n" + + " }\n" + + "}\n" + }, + "----------\n" + + "1. ERROR in X.java (at line 12)\n" + + " s.length();\n" + + " ^\n" + + "Potential null pointer access: The variable s may be null at this location\n" + + "----------\n"); +} + +// Bug 345305 - [compiler][null] Compiler misidentifies a case of "variable can only be null" +// do-while +public void testBug345305_14() { + runNegativeTest( + new String[] { + "X.java", + "public class X {\n" + + " void foo1(boolean b) {\n" + + " while (true) {\n" + + " String s = null;\n" + + " try {\n" + + " do {\n" + + " s = \"be\";\n" + + " if (b)\n" + + " return;\n" + + " } while (true);\n" + + " }\n" + + " finally {\n" + + " s.length(); // don't complain here\n" + + " s = null;\n" + + " }\n" + + " }\n" + + " }\n" + + " void foo2(boolean b) {\n" + + " while (true) {\n" + + " String s = null;\n" + + " try {\n" + + " do {\n" + + " if (b)\n" + + " continue;\n" + + " s = \"be\";\n" + + " b = !b;\n" + + " } while (b);\n" + + " }\n" + + " finally {\n" + + " s.length();\n" + + " s = null;\n" + + " }\n" + + " }\n" + + " }\n" + + "}\n" + }, + "----------\n" + + "1. ERROR in X.java (at line 30)\n" + + " s.length();\n" + + " ^\n" + + "Potential null pointer access: The variable s may be null at this location\n" + + "----------\n"); +} +} diff --git a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/TryStatementTest.java b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/TryStatementTest.java index 6f61716a12..3fff7df7db 100644 --- a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/TryStatementTest.java +++ b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/TryStatementTest.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2003, 2011 IBM Corporation and others. + * Copyright (c) 2003, 2012 IBM Corporation and others. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at @@ -7,6 +7,8 @@ * * Contributors: * IBM Corporation - initial API and implementation + * Stephan Herrmann - Contribution for + * bug 345305 - [compiler][null] Compiler misidentifies a case of "variable can only be null" *******************************************************************************/ package org.eclipse.jdt.core.tests.compiler.regression; @@ -23,7 +25,7 @@ import junit.framework.Test; public class TryStatementTest extends AbstractRegressionTest { static { -// TESTS_NAMES = new String[] { "test000" }; +// TESTS_NAMES = new String[] { "test074" }; // TESTS_NUMBERS = new int[] { 74, 75 }; // TESTS_RANGE = new int[] { 11, -1 }; } @@ -5961,6 +5963,33 @@ public void test073() { "No exception of type Exception[] can be thrown; an exception type must be a subclass of Throwable\n" + "----------\n"); } +// test for regression during work on bug 345305 +// saw "The local variable name may not have been initialized" against last code line +public void test074() { + runConformTest( + new String[] { + "X.java", + "public class X {\n" + + " Class test(String name) throws ClassNotFoundException {\n" + + " Class c= findClass(name);\n" + + " if (c != null)\n" + + " return c;\n" + + " if (isExcluded(name)) {\n" + + " try {\n" + + " c= findClass(name);\n" + + " return c;\n" + + " } catch (ClassNotFoundException e) {\n" + + " // keep searching\n" + + " }\n" + + " }\n" + + " return findClass(name);\n" + + " }\n" + + " boolean isExcluded(String name) { return false; }\n" + + " Class findClass(String name) throws ClassNotFoundException { return null; }\n" + + "}\n" + }); +} + public static Class testClass() { return TryStatementTest.class; } diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/AllocationExpression.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/AllocationExpression.java index 613003bf82..42030e04a8 100644 --- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/AllocationExpression.java +++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/AllocationExpression.java @@ -15,6 +15,7 @@ * bug 358903 - Filter practically unimportant resource leak warnings * bug 368546 - [compiler][resource] Avoid remaining false positives found when compiling the Eclipse SDK * bug 370639 - [compiler][resource] restore the default for resource leak warnings + * bug 345305 - [compiler][null] Compiler misidentifies a case of "variable can only be null" *******************************************************************************/ package org.eclipse.jdt.internal.compiler.ast; @@ -95,6 +96,9 @@ public FlowInfo analyseCode(BlockScope currentScope, FlowContext flowContext, Fl manageEnclosingInstanceAccessIfNecessary(currentScope, flowInfo); manageSyntheticAccessIfNecessary(currentScope, flowInfo); + // account for possible exceptions thrown by the constructor + flowContext.recordAbruptExit(); // TODO whitelist of ctors that cannot throw any exc.?? + return flowInfo; } diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/ArrayAllocationExpression.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/ArrayAllocationExpression.java index a7af1885f5..ab7b3bf436 100644 --- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/ArrayAllocationExpression.java +++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/ArrayAllocationExpression.java @@ -7,7 +7,9 @@ * * Contributors: * IBM Corporation - initial API and implementation - * Stephan Herrmann - Contribution for bug 319201 - [null] no warning when unboxing SingleNameReference causes NPE + * Stephan Herrmann - Contributions for + * bug 319201 - [null] no warning when unboxing SingleNameReference causes NPE + * bug 345305 - [compiler][null] Compiler misidentifies a case of "variable can only be null" *******************************************************************************/ package org.eclipse.jdt.internal.compiler.ast; @@ -36,6 +38,8 @@ public class ArrayAllocationExpression extends Expression { } } } + // account for potential OutOfMemoryError: + flowContext.recordAbruptExit(); if (this.initializer != null) { return this.initializer.analyseCode(currentScope, flowContext, flowInfo); } diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/ArrayReference.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/ArrayReference.java index 9b12104810..55d4cac4ea 100644 --- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/ArrayReference.java +++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/ArrayReference.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2000, 2011 IBM Corporation and others. + * Copyright (c) 2000, 2012 IBM Corporation and others. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at @@ -7,6 +7,8 @@ * * Contributors: * IBM Corporation - initial API and implementation + * Stephan Herrmann - Contribution for + * bug 345305 - [compiler][null] Compiler misidentifies a case of "variable can only be null" *******************************************************************************/ package org.eclipse.jdt.internal.compiler.ast; @@ -33,6 +35,8 @@ public ArrayReference(Expression rec, Expression pos) { public FlowInfo analyseAssignment(BlockScope currentScope, FlowContext flowContext, FlowInfo flowInfo, Assignment assignment, boolean compoundAssignment) { // TODO (maxime) optimization: unconditionalInits is applied to all existing calls + // account for potential ArrayIndexOutOfBoundsException: + flowContext.recordAbruptExit(); if (assignment.expression == null) { return analyseCode(currentScope, flowContext, flowInfo); } @@ -47,7 +51,10 @@ public FlowInfo analyseAssignment(BlockScope currentScope, FlowContext flowConte public FlowInfo analyseCode(BlockScope currentScope, FlowContext flowContext, FlowInfo flowInfo) { this.receiver.checkNPE(currentScope, flowContext, flowInfo); flowInfo = this.receiver.analyseCode(currentScope, flowContext, flowInfo); - return this.position.analyseCode(currentScope, flowContext, flowInfo); + flowInfo = this.position.analyseCode(currentScope, flowContext, flowInfo); + // account for potential ArrayIndexOutOfBoundsException: + flowContext.recordAbruptExit(); + return flowInfo; } public void generateAssignment(BlockScope currentScope, CodeStream codeStream, Assignment assignment, boolean valueRequired) { diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/AssertStatement.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/AssertStatement.java index efef33868f..71c86c259a 100644 --- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/AssertStatement.java +++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/AssertStatement.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2000, 2011 IBM Corporation and others. + * Copyright (c) 2000, 2012 IBM Corporation and others. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at @@ -7,7 +7,9 @@ * * Contributors: * IBM Corporation - initial API and implementation - * Stephan Herrmann - Contribution for bug 319201 - [null] no warning when unboxing SingleNameReference causes NPE + * Stephan Herrmann - Contributions for + * bug 319201 - [null] no warning when unboxing SingleNameReference causes NPE + * bug 345305 - [compiler][null] Compiler misidentifies a case of "variable can only be null" *******************************************************************************/ package org.eclipse.jdt.internal.compiler.ast; @@ -78,6 +80,8 @@ public FlowInfo analyseCode(BlockScope currentScope, FlowContext flowContext, Fl // add the assert support in the clinit manageSyntheticAccessIfNecessary(currentScope, flowInfo); } + // account for potential AssertionError: + flowContext.recordAbruptExit(); if (isOptimizedFalseAssertion) { return flowInfo; // if assertions are enabled, the following code will be unreachable // change this if we need to carry null analysis results of the assert diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/Assignment.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/Assignment.java index 4dfe8614f8..a69879cfa4 100644 --- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/Assignment.java +++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/Assignment.java @@ -17,6 +17,7 @@ * bug 358903 - Filter practically unimportant resource leak warnings * bug 370639 - [compiler][resource] restore the default for resource leak warnings * bug 365859 - [compiler][null] distinguish warnings based on flow analysis vs. null annotations + * bug 345305 - [compiler][null] Compiler misidentifies a case of "variable can only be null" *******************************************************************************/ package org.eclipse.jdt.internal.compiler.ast; @@ -82,7 +83,7 @@ public FlowInfo analyseCode(BlockScope currentScope, FlowContext flowContext, Fl if (local != null && (local.type.tagBits & TagBits.IsBaseType) == 0) { flowInfo.markNullStatus(local, nullStatus); if (flowContext.initsOnFinally != null) - flowContext.initsOnFinally.markNullStatus(local, nullStatus); + flowContext.markFinallyNullStatus(local, nullStatus); } return flowInfo; } diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/BinaryExpression.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/BinaryExpression.java index 92200beb3a..439bcf7d49 100644 --- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/BinaryExpression.java +++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/BinaryExpression.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2000, 2008 IBM Corporation and others. + * Copyright (c) 2000, 2012 IBM Corporation and others. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at @@ -7,6 +7,8 @@ * * Contributors: * IBM Corporation - initial API and implementation + * Stephan Herrmann - Contribution for + * bug 345305 - [compiler][null] Compiler misidentifies a case of "variable can only be null" *******************************************************************************/ package org.eclipse.jdt.internal.compiler.ast; @@ -58,16 +60,21 @@ public BinaryExpression(BinaryExpression expression) { } public FlowInfo analyseCode(BlockScope currentScope, FlowContext flowContext, FlowInfo flowInfo) { // keep implementation in sync with CombinedBinaryExpression#analyseCode - if (this.resolvedType.id == TypeIds.T_JavaLangString) { - return this.right.analyseCode( - currentScope, flowContext, - this.left.analyseCode(currentScope, flowContext, flowInfo).unconditionalInits()) - .unconditionalInits(); - } else { - this.left.checkNPE(currentScope, flowContext, flowInfo); - flowInfo = this.left.analyseCode(currentScope, flowContext, flowInfo).unconditionalInits(); - this.right.checkNPE(currentScope, flowContext, flowInfo); - return this.right.analyseCode(currentScope, flowContext, flowInfo).unconditionalInits(); + try { + if (this.resolvedType.id == TypeIds.T_JavaLangString) { + return this.right.analyseCode( + currentScope, flowContext, + this.left.analyseCode(currentScope, flowContext, flowInfo).unconditionalInits()) + .unconditionalInits(); + } else { + this.left.checkNPE(currentScope, flowContext, flowInfo); + flowInfo = this.left.analyseCode(currentScope, flowContext, flowInfo).unconditionalInits(); + this.right.checkNPE(currentScope, flowContext, flowInfo); + return this.right.analyseCode(currentScope, flowContext, flowInfo).unconditionalInits(); + } + } finally { + // account for exception possibly thrown by arithmetics + flowContext.recordAbruptExit(); } } diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/Block.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/Block.java index 5467d79265..a93b327652 100644 --- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/Block.java +++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/Block.java @@ -10,6 +10,7 @@ * Stephan Herrmann - Contributions for * bug 349326 - [1.7] new warning for missing try-with-resources * bug 368546 - [compiler][resource] Avoid remaining false positives found when compiling the Eclipse SDK + * bug 345305 - [compiler][null] Compiler misidentifies a case of "variable can only be null" *******************************************************************************/ package org.eclipse.jdt.internal.compiler.ast; @@ -38,6 +39,9 @@ public FlowInfo analyseCode(BlockScope currentScope, FlowContext flowContext, Fl if ((complaintLevel = stat.complainIfUnreachable(flowInfo, this.scope, complaintLevel, true)) < Statement.COMPLAINED_UNREACHABLE) { flowInfo = stat.analyseCode(this.scope, flowContext, flowInfo); } + // record the effect of stat on the finally block of an enclosing try-finally, if any: + if (flowContext.initsOnFinally != null) + flowContext.mergeFinallyNullInfo(flowInfo); } if (this.explicitDeclarations > 0) { // if block has its own scope analyze tracking vars now: diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/BreakStatement.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/BreakStatement.java index cc390d3335..79f2eb0563 100644 --- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/BreakStatement.java +++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/BreakStatement.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2000, 2011 IBM Corporation and others. + * Copyright (c) 2000, 2012 IBM Corporation and others. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at @@ -7,6 +7,8 @@ * * Contributors: * IBM Corporation - initial API and implementation + * Stephan Herrmann - Contribution for + * bug 345305 - [compiler][null] Compiler misidentifies a case of "variable can only be null" *******************************************************************************/ package org.eclipse.jdt.internal.compiler.ast; @@ -39,6 +41,8 @@ public FlowInfo analyseCode(BlockScope currentScope, FlowContext flowContext, Fl return flowInfo; // pretend it did not break since no actual target } + targetContext.recordAbruptExit(); + this.initStateIndex = currentScope.methodScope().recordInitializationStates(flowInfo); diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/CastExpression.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/CastExpression.java index 000caf61ee..a0fac81f94 100644 --- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/CastExpression.java +++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/CastExpression.java @@ -8,7 +8,9 @@ * Contributors: * IBM Corporation - initial API and implementation * Nick Teryaev - fix for bug (https://bugs.eclipse.org/bugs/show_bug.cgi?id=40752) - * Stephan Herrmann - Contribution for bug 319201 - [null] no warning when unboxing SingleNameReference causes NPE + * Stephan Herrmann - Contributions for + * bug 319201 - [null] no warning when unboxing SingleNameReference causes NPE + * bug 345305 - [compiler][null] Compiler misidentifies a case of "variable can only be null" *******************************************************************************/ package org.eclipse.jdt.internal.compiler.ast; @@ -55,6 +57,8 @@ public FlowInfo analyseCode(BlockScope currentScope, FlowContext flowContext, Fl if ((this.expression.implicitConversion & TypeIds.UNBOXING) != 0) { this.expression.checkNPE(currentScope, flowContext, flowInfo); } + // account for pot. CCE: + flowContext.recordAbruptExit(); return result; } diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/CombinedBinaryExpression.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/CombinedBinaryExpression.java index d49c2d6cf8..7ff2fec0fe 100644 --- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/CombinedBinaryExpression.java +++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/CombinedBinaryExpression.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2006, 2008 IBM Corporation and others. + * Copyright (c) 2006, 2012 IBM Corporation and others. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at @@ -7,6 +7,8 @@ * * Contributors: * IBM Corporation - initial API and implementation + * Stephan Herrmann - Contribution for + * bug 345305 - [compiler][null] Compiler misidentifies a case of "variable can only be null" *******************************************************************************/ package org.eclipse.jdt.internal.compiler.ast; @@ -121,27 +123,32 @@ public FlowInfo analyseCode(BlockScope currentScope, FlowContext flowContext, if (this.referencesTable == null) { return super.analyseCode(currentScope, flowContext, flowInfo); } - BinaryExpression cursor; - if ((cursor = this.referencesTable[0]).resolvedType.id != - TypeIds.T_JavaLangString) { - cursor.left.checkNPE(currentScope, flowContext, flowInfo); - } - flowInfo = cursor.left.analyseCode(currentScope, flowContext, flowInfo). - unconditionalInits(); - for (int i = 0, end = this.arity; i < end; i ++) { - if ((cursor = this.referencesTable[i]).resolvedType.id != + try { + BinaryExpression cursor; + if ((cursor = this.referencesTable[0]).resolvedType.id != TypeIds.T_JavaLangString) { - cursor.right.checkNPE(currentScope, flowContext, flowInfo); + cursor.left.checkNPE(currentScope, flowContext, flowInfo); } - flowInfo = cursor.right. - analyseCode(currentScope, flowContext, flowInfo). - unconditionalInits(); - } - if (this.resolvedType.id != TypeIds.T_JavaLangString) { - this.right.checkNPE(currentScope, flowContext, flowInfo); + flowInfo = cursor.left.analyseCode(currentScope, flowContext, flowInfo). + unconditionalInits(); + for (int i = 0, end = this.arity; i < end; i ++) { + if ((cursor = this.referencesTable[i]).resolvedType.id != + TypeIds.T_JavaLangString) { + cursor.right.checkNPE(currentScope, flowContext, flowInfo); + } + flowInfo = cursor.right. + analyseCode(currentScope, flowContext, flowInfo). + unconditionalInits(); + } + if (this.resolvedType.id != TypeIds.T_JavaLangString) { + this.right.checkNPE(currentScope, flowContext, flowInfo); + } + return this.right.analyseCode(currentScope, flowContext, flowInfo). + unconditionalInits(); + } finally { + // account for exception possibly thrown by arithmetics + flowContext.recordAbruptExit(); } - return this.right.analyseCode(currentScope, flowContext, flowInfo). - unconditionalInits(); } public void generateOptimizedStringConcatenation(BlockScope blockScope, diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/CompoundAssignment.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/CompoundAssignment.java index d4eab06076..11074ddfcc 100644 --- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/CompoundAssignment.java +++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/CompoundAssignment.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2000, 2011 IBM Corporation and others. + * Copyright (c) 2000, 2012 IBM Corporation and others. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at @@ -7,6 +7,8 @@ * * Contributors: * IBM Corporation - initial API and implementation + * Stephan Herrmann - Contribution for + * bug 345305 - [compiler][null] Compiler misidentifies a case of "variable can only be null" *******************************************************************************/ package org.eclipse.jdt.internal.compiler.ast; @@ -41,6 +43,8 @@ public FlowInfo analyseCode(BlockScope currentScope, FlowContext flowContext, // just a local variable. if (this.resolvedType.id != T_JavaLangString) { this.lhs.checkNPE(currentScope, flowContext, flowInfo); + // account for exceptions thrown by any arithmetics: + flowContext.recordAbruptExit(); } flowInfo = ((Reference) this.lhs).analyseAssignment(currentScope, flowContext, flowInfo, this, true).unconditionalInits(); if (this.resolvedType.id == T_JavaLangString) { @@ -50,7 +54,7 @@ public FlowInfo analyseCode(BlockScope currentScope, FlowContext flowContext, // compound assignment results in a definitely non null value for String flowInfo.markAsDefinitelyNonNull(local); if (flowContext.initsOnFinally != null) - flowContext.initsOnFinally.markAsDefinitelyNonNull(local); + flowContext.markFinallyNullStatus(local, FlowInfo.NON_NULL); } } return flowInfo; diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/ConditionalExpression.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/ConditionalExpression.java index 75dce2a414..aa36cb9e1f 100644 --- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/ConditionalExpression.java +++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/ConditionalExpression.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2000, 2011 IBM Corporation and others. + * Copyright (c) 2000, 2012 IBM Corporation and others. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at @@ -13,6 +13,7 @@ * bug 324178 - [null] ConditionalExpression.nullStatus(..) doesn't take into account the analysis of condition itself * bug 354554 - [null] conditional with redundant condition yields weak error message * bug 349326 - [1.7] new warning for missing try-with-resources + * bug 345305 - [compiler][null] Compiler misidentifies a case of "variable can only be null" *******************************************************************************/ package org.eclipse.jdt.internal.compiler.ast; @@ -59,6 +60,8 @@ public FlowInfo analyseCode(BlockScope currentScope, FlowContext flowContext, int mode = flowInfo.reachMode(); flowInfo = this.condition.analyseCode(currentScope, flowContext, flowInfo, cst == Constant.NotAConstant); + flowContext.conditionalLevel++; + // process the if-true part FlowInfo trueFlowInfo = flowInfo.initsWhenTrue().copy(); if (isConditionOptimizedFalse) { @@ -85,6 +88,8 @@ public FlowInfo analyseCode(BlockScope currentScope, FlowContext flowContext, this.falseInitStateIndex = currentScope.methodScope().recordInitializationStates(falseFlowInfo); falseFlowInfo = this.valueIfFalse.analyseCode(currentScope, flowContext, falseFlowInfo); + flowContext.conditionalLevel--; + // merge if-true & if-false initializations FlowInfo mergedInfo; if (isConditionOptimizedTrue){ diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/ContinueStatement.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/ContinueStatement.java index 475fef21f3..1337cdd29b 100644 --- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/ContinueStatement.java +++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/ContinueStatement.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2000, 2011 IBM Corporation and others. + * Copyright (c) 2000, 2012 IBM Corporation and others. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at @@ -7,6 +7,8 @@ * * Contributors: * IBM Corporation - initial API and implementation + * Stephan Herrmann - Contribution for + * bug 345305 - [compiler][null] Compiler misidentifies a case of "variable can only be null" *******************************************************************************/ package org.eclipse.jdt.internal.compiler.ast; @@ -39,6 +41,8 @@ public FlowInfo analyseCode(BlockScope currentScope, FlowContext flowContext, Fl return flowInfo; // pretend it did not continue since no actual target } + targetContext.recordAbruptExit(); + if (targetContext == FlowContext.NotContinuableContext) { currentScope.problemReporter().invalidContinue(this); return flowInfo; // pretend it did not continue since no actual target diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/DoStatement.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/DoStatement.java index 1fd8f3e594..6a76c3e8fd 100644 --- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/DoStatement.java +++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/DoStatement.java @@ -7,7 +7,9 @@ * * Contributors: * IBM Corporation - initial API and implementation - * Stephan Herrmann - Contribution for bug 319201 - [null] no warning when unboxing SingleNameReference causes NPE + * Stephan Herrmann - Contributions for + * bug 319201 - [null] no warning when unboxing SingleNameReference causes NPE + * bug 345305 - [compiler][null] Compiler misidentifies a case of "variable can only be null" *******************************************************************************/ package org.eclipse.jdt.internal.compiler.ast; @@ -48,7 +50,8 @@ public FlowInfo analyseCode(BlockScope currentScope, FlowContext flowContext, Fl this, this.breakLabel, this.continueLabel, - currentScope); + currentScope, + false); Constant cst = this.condition.constant; boolean isConditionTrue = cst != Constant.NotAConstant && cst.booleanValue() == true; @@ -59,7 +62,6 @@ public FlowInfo analyseCode(BlockScope currentScope, FlowContext flowContext, Fl int previousMode = flowInfo.reachMode(); FlowInfo initsOnCondition = flowInfo; - UnconditionalFlowInfo actionInfo = flowInfo.nullInfoLessUnconditionalCopy(); // we need to collect the contribution to nulls of the coming paths through the // loop, be they falling through normally or branched to break, continue labels @@ -97,7 +99,7 @@ public FlowInfo analyseCode(BlockScope currentScope, FlowContext flowContext, Fl currentScope, (condLoopContext = new LoopingFlowContext(flowContext, flowInfo, this, null, - null, currentScope)), + null, currentScope, true)), (this.action == null ? actionInfo : (actionInfo.mergedWith(loopingContext.initsOnContinue))).copy()); diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/Expression.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/Expression.java index 33bd8de8e9..41f3b9fc09 100644 --- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/Expression.java +++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/Expression.java @@ -7,7 +7,9 @@ * * Contributors: * IBM Corporation - initial API and implementation - * Stephan Herrmann <stephan@cs.tu-berlin.de> - Contribution for bug 292478 - Report potentially null across variable assignment + * Stephan Herrmann <stephan@cs.tu-berlin.de> - Contributions for + * bug 292478 - Report potentially null across variable assignment + * bug 345305 - [compiler][null] Compiler misidentifies a case of "variable can only be null" *******************************************************************************/ package org.eclipse.jdt.internal.compiler.ast; @@ -531,11 +533,15 @@ public void checkNPE(BlockScope scope, FlowContext flowContext, FlowInfo flowInf if ((this.bits & ASTNode.IsNonNull) == 0) { flowContext.recordUsingNullReference(scope, local, this, FlowContext.MAY_NULL, flowInfo); + // account for possible NPE: + if (!flowInfo.isDefinitelyNonNull(local)) { + flowContext.recordAbruptExit(); + } } flowInfo.markAsComparedEqualToNonNull(local); // from thereon it is set if (flowContext.initsOnFinally != null) { - flowContext.initsOnFinally.markAsComparedEqualToNonNull(local); + flowContext.markFinallyNullStatus(local, FlowInfo.NON_NULL); } } } diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/ForStatement.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/ForStatement.java index 498a81408c..88948bb859 100644 --- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/ForStatement.java +++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/ForStatement.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2000, 2011 IBM Corporation and others. + * Copyright (c) 2000, 2012 IBM Corporation and others. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at @@ -10,6 +10,7 @@ * Stephan Herrmann - Contributions for * bug 319201 - [null] no warning when unboxing SingleNameReference causes NPE * bug 349326 - [1.7] new warning for missing try-with-resources + * bug 345305 - [compiler][null] Compiler misidentifies a case of "variable can only be null" *******************************************************************************/ package org.eclipse.jdt.internal.compiler.ast; @@ -93,7 +94,7 @@ public class ForStatement extends Statement { this.scope, (condLoopContext = new LoopingFlowContext(flowContext, flowInfo, this, null, - null, this.scope)), + null, this.scope, true)), condInfo); if ((this.condition.implicitConversion & TypeIds.UNBOXING) != 0) { this.condition.checkNPE(currentScope, flowContext, flowInfo); @@ -121,13 +122,15 @@ public class ForStatement extends Statement { actionInfo = condInfo.initsWhenTrue().unconditionalCopy(); loopingContext = new LoopingFlowContext(flowContext, flowInfo, this, - this.breakLabel, this.continueLabel, this.scope); + this.breakLabel, this.continueLabel, this.scope, false); + // there is no action guarded by a preTest, so we use preTest=false + // to avoid pointless burdens of updating FlowContext.conditionalLevel } } else { loopingContext = new LoopingFlowContext(flowContext, flowInfo, this, this.breakLabel, - this.continueLabel, this.scope); + this.continueLabel, this.scope, true); FlowInfo initsWhenTrue = condInfo.initsWhenTrue(); this.condIfTrueInitStateIndex = currentScope.methodScope().recordInitializationStates(initsWhenTrue); @@ -168,7 +171,7 @@ public class ForStatement extends Statement { if (this.increments != null) { incrementContext = new LoopingFlowContext(flowContext, flowInfo, this, null, - null, this.scope); + null, this.scope, true); FlowInfo incrementInfo = actionInfo; this.preIncrementsInitStateIndex = currentScope.methodScope().recordInitializationStates(incrementInfo); 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 4456f6eff6..3c9ee10dd0 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 @@ -11,6 +11,7 @@ * bug 349326 - [1.7] new warning for missing try-with-resources * bug 370930 - NonNull annotation not considered for enhanced for loops * bug 365859 - [compiler][null] distinguish warnings based on flow analysis vs. null annotations + * bug 345305 - [compiler][null] Compiler misidentifies a case of "variable can only be null" *******************************************************************************/ package org.eclipse.jdt.internal.compiler.ast; @@ -94,10 +95,11 @@ public class ForeachStatement extends Statement { this.postCollectionInitStateIndex = currentScope.methodScope().recordInitializationStates(condInfo); + // process the action LoopingFlowContext loopingContext = new LoopingFlowContext(flowContext, flowInfo, this, this.breakLabel, - this.continueLabel, this.scope); + this.continueLabel, this.scope, true); UnconditionalFlowInfo actionInfo = condInfo.nullInfoLessUnconditionalCopy(); actionInfo.markAsDefinitelyUnknown(elementVarBinding); diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/IfStatement.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/IfStatement.java index 8fc60941f4..4cfba98000 100644 --- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/IfStatement.java +++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/IfStatement.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2000, 2011 IBM Corporation and others. + * Copyright (c) 2000, 2012 IBM Corporation and others. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at @@ -10,6 +10,7 @@ * Stephan Herrmann - Contributions for * bug 319201 - [null] no warning when unboxing SingleNameReference causes NPE * bug 349326 - [1.7] new warning for missing try-with-resources + * bug 345305 - [compiler][null] Compiler misidentifies a case of "variable can only be null" *******************************************************************************/ package org.eclipse.jdt.internal.compiler.ast; @@ -66,6 +67,8 @@ public FlowInfo analyseCode(BlockScope currentScope, FlowContext flowContext, Fl boolean isConditionOptimizedTrue = cst != Constant.NotAConstant && cst.booleanValue() == true; boolean isConditionOptimizedFalse = cst != Constant.NotAConstant && cst.booleanValue() == false; + flowContext.conditionalLevel++; + // process the THEN part FlowInfo thenFlowInfo = conditionFlowInfo.safeInitsWhenTrue(); if (isConditionOptimizedFalse) { @@ -140,6 +143,7 @@ public FlowInfo analyseCode(BlockScope currentScope, FlowContext flowContext, Fl this, reportDeadCodeForKnownPattern); this.mergedInitStateIndex = currentScope.methodScope().recordInitializationStates(mergedInfo); + flowContext.conditionalLevel--; return mergedInfo; } diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/MessageSend.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/MessageSend.java index 44f5bc4b13..dddb1a001d 100644 --- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/MessageSend.java +++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/MessageSend.java @@ -14,6 +14,7 @@ * bug 186342 - [compiler][null] Using annotations for null checking * bug 358903 - Filter practically unimportant resource leak warnings * bug 370639 - [compiler][resource] restore the default for resource leak warnings + * bug 345305 - [compiler][null] Compiler misidentifies a case of "variable can only be null" *******************************************************************************/ package org.eclipse.jdt.internal.compiler.ast; @@ -154,6 +155,8 @@ public FlowInfo analyseCode(BlockScope currentScope, FlowContext flowContext, Fl // NullReferenceTest#test0510 } manageSyntheticAccessIfNecessary(currentScope, flowInfo); + // account for pot. exceptions thrown by method execution + flowContext.recordAbruptExit(); return flowInfo; } public void checkNPE(BlockScope scope, FlowContext flowContext, FlowInfo flowInfo) { diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/QualifiedAllocationExpression.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/QualifiedAllocationExpression.java index e593ad035a..fdbf4d0b2d 100644 --- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/QualifiedAllocationExpression.java +++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/QualifiedAllocationExpression.java @@ -13,6 +13,7 @@ * bug 186342 - [compiler][null] Using annotations for null checking * bug 368546 - [compiler][resource] Avoid remaining false positives found when compiling the Eclipse SDK * bug 370639 - [compiler][resource] restore the default for resource leak warnings + * bug 345305 - [compiler][null] Compiler misidentifies a case of "variable can only be null" *******************************************************************************/ package org.eclipse.jdt.internal.compiler.ast; @@ -126,6 +127,10 @@ public class QualifiedAllocationExpression extends AllocationExpression { manageEnclosingInstanceAccessIfNecessary(currentScope, flowInfo); manageSyntheticAccessIfNecessary(currentScope, flowInfo); + + // account for possible exceptions thrown by constructor execution: + flowContext.recordAbruptExit(); + return flowInfo; } diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/QualifiedNameReference.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/QualifiedNameReference.java index da5acaedd4..9045fe4c06 100644 --- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/QualifiedNameReference.java +++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/QualifiedNameReference.java @@ -12,6 +12,7 @@ * bug 186342 - [compiler][null] Using annotations for null checking * bug 365519 - editorial cleanup after bug 186342 and bug 365387 * bug 368546 - [compiler][resource] Avoid remaining false positives found when compiling the Eclipse SDK + * bug 345305 - [compiler][null] Compiler misidentifies a case of "variable can only be null" *******************************************************************************/ package org.eclipse.jdt.internal.compiler.ast; @@ -242,7 +243,7 @@ public void checkNPE(BlockScope scope, FlowContext flowContext, FlowInfo flowInf flowInfo.markAsComparedEqualToNonNull(local); // from thereon it is set if (flowContext.initsOnFinally != null) { - flowContext.initsOnFinally.markAsComparedEqualToNonNull(local); + flowContext.markFinallyNullStatus(local, FlowInfo.NON_NULL); } } } diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/ReturnStatement.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/ReturnStatement.java index 5cd11cf3c5..760d30cdd0 100644 --- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/ReturnStatement.java +++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/ReturnStatement.java @@ -18,6 +18,7 @@ * bug 368546 - [compiler][resource] Avoid remaining false positives found when compiling the Eclipse SDK * bug 370639 - [compiler][resource] restore the default for resource leak warnings * bug 365859 - [compiler][null] distinguish warnings based on flow analysis vs. null annotations + * bug 345305 - [compiler][null] Compiler misidentifies a case of "variable can only be null" *******************************************************************************/ package org.eclipse.jdt.internal.compiler.ast; @@ -135,6 +136,8 @@ public FlowInfo analyseCode(BlockScope currentScope, FlowContext flowContext, Fl } } currentScope.checkUnclosedCloseables(flowInfo, flowContext, this, currentScope); + // inside conditional structure respect that a finally-block may conditionally be entered directly from here + flowContext.recordAbruptExit(); return FlowInfo.DEAD_END; } void checkAgainstNullAnnotation(BlockScope scope, FlowContext flowContext, int nullStatus) { diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/SwitchStatement.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/SwitchStatement.java index eaede3c8b2..7a71bd1a81 100644 --- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/SwitchStatement.java +++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/SwitchStatement.java @@ -12,6 +12,7 @@ * bug 349326 - [1.7] new warning for missing try-with-resources * bug 265744 - Enum switch should warn about missing default * bug 374605 - Unreasonable warning for enum-based switch statements + * bug 345305 - [compiler][null] Compiler misidentifies a case of "variable can only be null" *******************************************************************************/ package org.eclipse.jdt.internal.compiler.ast; @@ -67,7 +68,7 @@ public class SwitchStatement extends Statement { this.expression.checkNPE(currentScope, flowContext, flowInfo); } SwitchFlowContext switchContext = - new SwitchFlowContext(flowContext, this, (this.breakLabel = new BranchLabel())); + new SwitchFlowContext(flowContext, this, (this.breakLabel = new BranchLabel()), true); // analyse the block by considering specially the case/default statements (need to bind them // to the entry point) diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/ThrowStatement.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/ThrowStatement.java index e7dc25353b..df15cc1979 100644 --- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/ThrowStatement.java +++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/ThrowStatement.java @@ -10,6 +10,7 @@ * Stephan Herrmann - Contributions for * bug 359334 - Analysis for resource leak warnings does not consider exceptions as method exit points * bug 368546 - [compiler][resource] Avoid remaining false positives found when compiling the Eclipse SDK + * bug 345305 - [compiler][null] Compiler misidentifies a case of "variable can only be null" *******************************************************************************/ package org.eclipse.jdt.internal.compiler.ast; @@ -39,6 +40,7 @@ public FlowInfo analyseCode(BlockScope currentScope, FlowContext flowContext, Fl // need to check that exception thrown is actually caught somewhere flowContext.checkExceptionHandlers(this.exceptionType, this, flowInfo, currentScope); currentScope.checkUnclosedCloseables(flowInfo, flowContext, this, currentScope); + flowContext.recordAbruptExit(); return FlowInfo.DEAD_END; } diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/TryStatement.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/TryStatement.java index 0648e5bf9a..be0922ff28 100644 --- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/TryStatement.java +++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/TryStatement.java @@ -13,6 +13,7 @@ * bug 349326 - [1.7] new warning for missing try-with-resources * bug 359334 - Analysis for resource leak warnings does not consider exceptions as method exit points * bug 358903 - Filter practically unimportant resource leak warnings + * bug 345305 - [compiler][null] Compiler misidentifies a case of "variable can only be null" *******************************************************************************/ package org.eclipse.jdt.internal.compiler.ast; @@ -118,12 +119,9 @@ public FlowInfo analyseCode(BlockScope currentScope, FlowContext flowContext, Fl this, this.caughtExceptionTypes, this.caughtExceptionsCatchBlocks, - this.catchArguments, null, this.scope, - flowInfo.unconditionalInits()); - handlingContext.initsOnFinally = - new NullInfoRegistry(flowInfo.unconditionalInits()); + flowInfo); // only try blocks initialize that member - may consider creating a // separate class if needed @@ -195,13 +193,13 @@ public FlowInfo analyseCode(BlockScope currentScope, FlowContext flowContext, Fl FlowInfo catchInfo; if (isUncheckedCatchBlock(i)) { catchInfo = - handlingContext.initsOnFinally.mitigateNullInfoOf( - flowInfo.unconditionalCopy(). - addPotentialInitializationsFrom( - handlingContext.initsOnException(i)). - addPotentialInitializationsFrom(tryInfo). - addPotentialInitializationsFrom( - handlingContext.initsOnReturn)); + flowInfo.unconditionalCopy(). + addPotentialInitializationsFrom( + handlingContext.initsOnException(i)). + addPotentialInitializationsFrom(tryInfo). + addPotentialInitializationsFrom( + handlingContext.initsOnReturn). + addNullInfoFrom(handlingContext.initsOnFinally); } else { FlowInfo initsOnException = handlingContext.initsOnException(i); catchInfo = @@ -244,7 +242,7 @@ public FlowInfo analyseCode(BlockScope currentScope, FlowContext flowContext, Fl // chain up null info registry if (flowContext.initsOnFinally != null) { - flowContext.initsOnFinally.add(handlingContext.initsOnFinally); + flowContext.initsOnFinally.addNullInfoFrom(handlingContext.initsOnFinally); } return tryInfo; @@ -282,12 +280,9 @@ public FlowInfo analyseCode(BlockScope currentScope, FlowContext flowContext, Fl this, this.caughtExceptionTypes, this.caughtExceptionsCatchBlocks, - this.catchArguments, null, this.scope, - flowInfo.unconditionalInits()); - handlingContext.initsOnFinally = - new NullInfoRegistry(flowInfo.unconditionalInits()); + flowInfo); // only try blocks initialize that member - may consider creating a // separate class if needed @@ -359,13 +354,13 @@ public FlowInfo analyseCode(BlockScope currentScope, FlowContext flowContext, Fl FlowInfo catchInfo; if (isUncheckedCatchBlock(i)) { catchInfo = - handlingContext.initsOnFinally.mitigateNullInfoOf( - flowInfo.unconditionalCopy(). - addPotentialInitializationsFrom( - handlingContext.initsOnException(i)). - addPotentialInitializationsFrom(tryInfo). - addPotentialInitializationsFrom( - handlingContext.initsOnReturn)); + flowInfo.unconditionalCopy(). + addPotentialInitializationsFrom( + handlingContext.initsOnException(i)). + addPotentialInitializationsFrom(tryInfo). + addPotentialInitializationsFrom( + handlingContext.initsOnReturn). + addNullInfoFrom(handlingContext.initsOnFinally); }else { FlowInfo initsOnException = handlingContext.initsOnException(i); catchInfo = @@ -406,19 +401,20 @@ public FlowInfo analyseCode(BlockScope currentScope, FlowContext flowContext, Fl // we also need to check potential multiple assignments of final variables inside the finally block // need to include potential inits from returns inside the try/catch parts - 1GK2AOF finallyContext.complainOnDeferredChecks( - handlingContext.initsOnFinally.mitigateNullInfoOf( - (tryInfo.tagBits & FlowInfo.UNREACHABLE) == 0 ? - flowInfo.unconditionalCopy(). + ((tryInfo.tagBits & FlowInfo.UNREACHABLE) == 0 ? + flowInfo.unconditionalCopy(). addPotentialInitializationsFrom(tryInfo). - // lighten the influence of the try block, which may have - // exited at any point + // lighten the influence of the try block, which may have + // exited at any point addPotentialInitializationsFrom(insideSubContext.initsOnReturn) : - insideSubContext.initsOnReturn), + insideSubContext.initsOnReturn). + addNullInfoFrom( + handlingContext.initsOnFinally), currentScope); // chain up null info registry if (flowContext.initsOnFinally != null) { - flowContext.initsOnFinally.add(handlingContext.initsOnFinally); + flowContext.initsOnFinally.addNullInfoFrom(handlingContext.initsOnFinally); } this.naturalExitMergeInitStateIndex = diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/WhileStatement.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/WhileStatement.java index e35ea9c0be..54b3a16fc9 100644 --- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/WhileStatement.java +++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/WhileStatement.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2000, 2011 IBM Corporation and others. + * Copyright (c) 2000, 2012 IBM Corporation and others. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at @@ -10,6 +10,7 @@ * Stephan Herrmann - Contributions for * bug 319201 - [null] no warning when unboxing SingleNameReference causes NPE * bug 349326 - [1.7] new warning for missing try-with-resources + * bug 345305 - [compiler][null] Compiler misidentifies a case of "variable can only be null" *******************************************************************************/ package org.eclipse.jdt.internal.compiler.ast; @@ -64,7 +65,7 @@ public class WhileStatement extends Statement { currentScope, (condLoopContext = new LoopingFlowContext(flowContext, flowInfo, this, null, - null, currentScope)), + null, currentScope, true)), condInfo); if ((this.condition.implicitConversion & TypeIds.UNBOXING) != 0) { this.condition.checkNPE(currentScope, flowContext, flowInfo); @@ -100,7 +101,8 @@ public class WhileStatement extends Statement { this, this.breakLabel, this.continueLabel, - currentScope); + currentScope, + true); if (isConditionFalse) { actionInfo = FlowInfo.DEAD_END; } else { diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/flow/ExceptionHandlingFlowContext.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/flow/ExceptionHandlingFlowContext.java index c98aa6a328..4beaa90f95 100644 --- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/flow/ExceptionHandlingFlowContext.java +++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/flow/ExceptionHandlingFlowContext.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2000, 2011 IBM Corporation and others. + * Copyright (c) 2000, 2012 IBM Corporation and others. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at @@ -7,6 +7,8 @@ * * Contributors: * IBM Corporation - initial API and implementation + * Stephan Herrmann - Contribution for + * bug 345305 - [compiler][null] Compiler misidentifies a case of "variable can only be null" *******************************************************************************/ package org.eclipse.jdt.internal.compiler.flow; @@ -67,6 +69,19 @@ public ExceptionHandlingFlowContext( } public ExceptionHandlingFlowContext( FlowContext parent, + TryStatement tryStatement, + ReferenceBinding[] handledExceptions, + int [] exceptionToCatchBlockMap, + FlowContext initializationParent, + BlockScope scope, + FlowInfo flowInfo) { + this(parent, tryStatement, handledExceptions, exceptionToCatchBlockMap, + tryStatement.catchArguments, initializationParent, scope, flowInfo.unconditionalInits()); + this.initsOnFinally = flowInfo.unconditionalCopy(); + this.conditionalLevel = 0; +} +ExceptionHandlingFlowContext( + FlowContext parent, ASTNode associatedNode, ReferenceBinding[] handledExceptions, int [] exceptionToCatchBlockMap, diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/flow/FlowContext.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/flow/FlowContext.java index b7031df574..2998097193 100644 --- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/flow/FlowContext.java +++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/flow/FlowContext.java @@ -12,6 +12,7 @@ * bug 186342 - [compiler][null] Using annotations for null checking * bug 368546 - [compiler][resource] Avoid remaining false positives found when compiling the Eclipse SDK * bug 365859 - [compiler][null] distinguish warnings based on flow analysis vs. null annotations + * bug 345305 - [compiler][null] Compiler misidentifies a case of "variable can only be null" *******************************************************************************/ package org.eclipse.jdt.internal.compiler.flow; @@ -52,9 +53,17 @@ public class FlowContext implements TypeConstants { public final static FlowContext NotContinuableContext = new FlowContext(null, null); public ASTNode associatedNode; public FlowContext parent; - public NullInfoRegistry initsOnFinally; + public FlowInfo initsOnFinally; // only used within try blocks; remembers upstream flow info mergedWith // any null related operation happening within the try block + /** + * Used to record whether effects in a try block affect the finally-block + * conditionally or unconditionally. + * -1 means: no effect, + * 0 means: unconditional effect, + * > 0 means levels of nested conditional structures. + */ + public int conditionalLevel = -1; public int tagBits; @@ -99,6 +108,7 @@ public FlowContext(FlowContext parent, ASTNode associatedNode) { this.tagBits |= FlowContext.DEFER_NULL_DIAGNOSTIC; } this.initsOnFinally = parent.initsOnFinally; + this.conditionalLevel = parent.conditionalLevel; } } @@ -550,6 +560,59 @@ public char[] labelName() { return null; } +/** + * Record a given null status of a given local variable as it will be seen in the finally block. + * Precondition: caller has checked that initsOnFinally != null. + * @param local the local variable being observed + * @param nullStatus the null status of local at the current point in the flow + */ +public void markFinallyNullStatus(LocalVariableBinding local, int nullStatus) { + if (this.conditionalLevel == -1) return; + if (this.conditionalLevel == 0) { + // node is unconditionally reached, take nullStatus as is: + this.initsOnFinally.markNullStatus(local, nullStatus); + return; + } + // node is reached only conditionally, weaken status to potentially_ and merge with previous + UnconditionalFlowInfo newInfo = this.initsOnFinally.unconditionalCopy(); + newInfo.markNullStatus(local, nullStatus); + this.initsOnFinally = this.initsOnFinally.mergedWith(newInfo); +} + +/** + * Merge the effect of a statement presumably contained in a try-block, + * i.e., record how the collected info will affect the corresponding finally-block. + * Precondition: caller has checked that initsOnFinally != null. + * @param flowInfo info after executing a statement of the try-block. + */ +public void mergeFinallyNullInfo(FlowInfo flowInfo) { + if (this.conditionalLevel == -1) return; + if (this.conditionalLevel == 0) { + // node is unconditionally reached, take null info as is: + this.initsOnFinally.addNullInfoFrom(flowInfo); + return; + } + // node is reached only conditionally: merge flowInfo with existing since both paths are possible + this.initsOnFinally = this.initsOnFinally.mergedWith(flowInfo.unconditionalCopy()); +} + +/** + * Record the fact that an abrupt exit has been observed, one of: + * - potential exception (incl. unchecked exceptions) + * - break + * - continue + * - return + */ +public void recordAbruptExit() { + if (this.conditionalLevel > -1) { + this.conditionalLevel++; + // delegate up up-to the enclosing try-finally: + if (!(this instanceof ExceptionHandlingFlowContext) && this.parent != null) { + this.parent.recordAbruptExit(); + } + } +} + public void recordBreakFrom(FlowInfo flowInfo) { // default implementation: do nothing } diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/flow/LabelFlowContext.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/flow/LabelFlowContext.java index 88ca4e7664..309eefc3a1 100644 --- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/flow/LabelFlowContext.java +++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/flow/LabelFlowContext.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2000, 2011 IBM Corporation and others. + * Copyright (c) 2000, 2012 IBM Corporation and others. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at @@ -7,6 +7,8 @@ * * Contributors: * IBM Corporation - initial API and implementation + * Stephan Herrmann - Contribution for + * bug 345305 - [compiler][null] Compiler misidentifies a case of "variable can only be null" *******************************************************************************/ package org.eclipse.jdt.internal.compiler.flow; @@ -24,7 +26,7 @@ public class LabelFlowContext extends SwitchFlowContext { public char[] labelName; public LabelFlowContext(FlowContext parent, ASTNode associatedNode, char[] labelName, BranchLabel breakLabel, BlockScope scope) { - super(parent, associatedNode, breakLabel); + super(parent, associatedNode, breakLabel, false); this.labelName = labelName; checkLabelValidity(scope); } diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/flow/LoopingFlowContext.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/flow/LoopingFlowContext.java index 2ac8981e1c..5020e8aea0 100644 --- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/flow/LoopingFlowContext.java +++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/flow/LoopingFlowContext.java @@ -14,6 +14,7 @@ * bug 368546 - [compiler][resource] Avoid remaining false positives found when compiling the Eclipse SDK * bug 365859 - [compiler][null] distinguish warnings based on flow analysis vs. null annotations * bug 385626 - @NonNull fails across loop boundaries + * bug 345305 - [compiler][null] Compiler misidentifies a case of "variable can only be null" *******************************************************************************/ package org.eclipse.jdt.internal.compiler.flow; @@ -85,8 +86,9 @@ public class LoopingFlowContext extends SwitchFlowContext { ASTNode associatedNode, BranchLabel breakLabel, BranchLabel continueLabel, - Scope associatedScope) { - super(parent, associatedNode, breakLabel); + Scope associatedScope, + boolean isPreTest) { + super(parent, associatedNode, breakLabel, isPreTest); this.tagBits |= FlowContext.PREEMPT_NULL_DIAGNOSTIC; // children will defer to this, which may defer to its own parent this.continueLabel = continueLabel; diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/flow/NullInfoRegistry.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/flow/NullInfoRegistry.java deleted file mode 100644 index 4005f54c8b..0000000000 --- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/flow/NullInfoRegistry.java +++ /dev/null @@ -1,549 +0,0 @@ -/******************************************************************************* - * Copyright (c) 2006, 2012 IBM Corporation and others. - * All rights reserved. This program and the accompanying materials - * are made available under the terms of the Eclipse Public License v1.0 - * which accompanies this distribution, and is available at - * http://www.eclipse.org/legal/epl-v10.html - * - * Contributors: - * IBM Corporation - initial API and implementation - * Stephan Herrmann <stephan@cs.tu-berlin.de> - Contribution for bug 320170 - *******************************************************************************/ -package org.eclipse.jdt.internal.compiler.flow; - -import org.eclipse.jdt.internal.compiler.lookup.LocalVariableBinding; - -/** - * A degenerate form of UnconditionalFlowInfo explicitly meant to capture - * the effects of null related operations within try blocks. Given the fact - * that a try block might exit at any time, a null related operation that - * occurs within such a block mitigates whatever we know about the previous - * null status of involved variables. NullInfoRegistry handles that - * by negating upstream definite information that clashes with what a given - * statement contends about the same variable. It also implements - * {@link #mitigateNullInfoOf(FlowInfo) mitigateNullInfo} so as to elaborate the - * flow info presented in input of finally blocks. - */ -public class NullInfoRegistry extends UnconditionalFlowInfo { - // significant states at this level: - // def. non null, def. null, def. unknown, prot. non null - -// PREMATURE implement coverage and low level tests - -/** - * Make a new null info registry, using an upstream flow info. All definite - * assignments of the upstream are carried forward, since a try block may - * exit before its first statement. - * @param upstream - UnconditionalFlowInfo: the flow info before we enter the - * try block; only definite assignments are considered; this parameter is - * not modified by this constructor - */ -public NullInfoRegistry(UnconditionalFlowInfo upstream) { - this.maxFieldCount = upstream.maxFieldCount; - if ((upstream.tagBits & NULL_FLAG_MASK) != 0) { - long u1, u2, u3, u4, nu2, nu3, nu4; - this.nullBit2 = (u1 = upstream.nullBit1) - & (u2 = upstream.nullBit2) - & (nu3 = ~(u3 = upstream.nullBit3)) - & (nu4 = ~(u4 = upstream.nullBit4)); - this.nullBit3 = u1 & (nu2 = ~u2) & u3 & nu4; - this.nullBit4 = u1 & nu2 &nu3 & u4; - if ((this.nullBit2 | this.nullBit3 | this.nullBit4) != 0) { - this.tagBits |= NULL_FLAG_MASK; - } - if (upstream.extra != null) { - this.extra = new long[extraLength][]; - int length = upstream.extra[2].length; - for (int i = 2; i < extraLength; i++) { - this.extra[i] = new long[length]; - } - for (int i = 0; i < length; i++) { - this.extra[2 + 1][i] = (u1 = upstream.extra[1 + 1][i]) - & (u2 = upstream.extra[2 + 1][i]) - & (nu3 = ~(u3 = upstream.extra[3 + 1][i])) - & (nu4 = ~(u4 = upstream.extra[4 + 1][i])); - this.extra[3 + 1][i] = u1 & (nu2 = ~u2) & u3 & nu4; - this.extra[4 + 1][i] = u1 & nu2 &nu3 & u4; - if ((this.extra[2 + 1][i] | this.extra[3 + 1][i] | this.extra[4 + 1][i]) != 0) { - this.tagBits |= NULL_FLAG_MASK; - } - } - } - } -} - -/** - * Add the information held by another NullInfoRegistry instance to this, - * then return this. - * @param other - NullInfoRegistry: the information to add to this - * @return this, modified to carry the information held by other - */ -public NullInfoRegistry add(NullInfoRegistry other) { - if ((other.tagBits & NULL_FLAG_MASK) == 0) { - return this; - } - this.tagBits |= NULL_FLAG_MASK; - this.nullBit1 |= other.nullBit1; - this.nullBit2 |= other.nullBit2; - this.nullBit3 |= other.nullBit3; - this.nullBit4 |= other.nullBit4; - if (other.extra != null) { - if (this.extra == null) { - this.extra = new long[extraLength][]; - for (int i = 2, length = other.extra[2].length; i < extraLength; i++) { - System.arraycopy(other.extra[i], 0, - (this.extra[i] = new long[length]), 0, length); - } - } else { - int length = this.extra[2].length, otherLength = other.extra[2].length; - if (otherLength > length) { - for (int i = 2; i < extraLength; i++) { - System.arraycopy(this.extra[i], 0, - (this.extra[i] = new long[otherLength]), 0, length); - System.arraycopy(other.extra[i], length, - this.extra[i], length, otherLength - length); - } - } else if (otherLength < length) { - length = otherLength; - } - for (int i = 2; i < extraLength; i++) { - for (int j = 0; j < length; j++) { - this.extra[i][j] |= other.extra[i][j]; - } - } - } - } - return this; -} - -public void markAsComparedEqualToNonNull(LocalVariableBinding local) { - // protected from non-object locals in calling methods - if (this != DEAD_END) { - this.tagBits |= NULL_FLAG_MASK; - int position; - // position is zero-based - if ((position = local.id + this.maxFieldCount) < BitCacheSize) { // use bits - // set protected non null - this.nullBit1 |= (1L << position); - if (COVERAGE_TEST_FLAG) { - if (CoverageTestId == 290) { - this.nullBit1 = 0; - } - } - } - else { - // use extra vector - int vectorIndex = (position / BitCacheSize) - 1; - if (this.extra == null) { - int length = vectorIndex + 1; - this.extra = new long[extraLength][]; - for (int j = 2; j < extraLength; j++) { - this.extra[j] = new long[length]; - } - } - else { - int oldLength; // might need to grow the arrays - if (vectorIndex >= (oldLength = this.extra[2].length)) { - for (int j = 2; j < extraLength; j++) { - System.arraycopy(this.extra[j], 0, - (this.extra[j] = new long[vectorIndex + 1]), 0, - oldLength); - } - } - } - this.extra[2][vectorIndex] |= (1L << (position % BitCacheSize)); - if (COVERAGE_TEST_FLAG) { - if (CoverageTestId == 300) { - this.extra[5][vectorIndex] = ~0; - } - } - } - } -} - -public void markAsDefinitelyNonNull(LocalVariableBinding local) { - // protected from non-object locals in calling methods - if (this != DEAD_END) { - this.tagBits |= NULL_FLAG_MASK; - int position; - // position is zero-based - if ((position = local.id + this.maxFieldCount) < BitCacheSize) { // use bits - // set assigned non null - this.nullBit3 |= (1L << position); - if (COVERAGE_TEST_FLAG) { - if (CoverageTestId == 290) { - this.nullBit1 = 0; - } - } - } - else { - // use extra vector - int vectorIndex = (position / BitCacheSize) - 1; - if (this.extra == null) { - int length = vectorIndex + 1; - this.extra = new long[extraLength][]; - for (int j = 2; j < extraLength; j++) { - this.extra[j] = new long[length]; - } - } - else { - int oldLength; // might need to grow the arrays - if (vectorIndex >= (oldLength = this.extra[2].length)) { - for (int j = 2; j < extraLength; j++) { - System.arraycopy(this.extra[j], 0, - (this.extra[j] = new long[vectorIndex + 1]), 0, - oldLength); - } - } - } - this.extra[4][vectorIndex] |= (1L << (position % BitCacheSize)); - if (COVERAGE_TEST_FLAG) { - if (CoverageTestId == 300) { - this.extra[5][vectorIndex] = ~0; - } - } - } - } -} -// PREMATURE consider ignoring extra 0 to 2 included - means a1 should not be used either -// PREMATURE project protected non null onto something else -public void markAsDefinitelyNull(LocalVariableBinding local) { - // protected from non-object locals in calling methods - if (this != DEAD_END) { - this.tagBits |= NULL_FLAG_MASK; - int position; - // position is zero-based - if ((position = local.id + this.maxFieldCount) < BitCacheSize) { // use bits - // set assigned null - this.nullBit2 |= (1L << position); - if (COVERAGE_TEST_FLAG) { - if (CoverageTestId == 290) { - this.nullBit1 = 0; - } - } - } - else { - // use extra vector - int vectorIndex = (position / BitCacheSize) - 1; - if (this.extra == null) { - int length = vectorIndex + 1; - this.extra = new long[extraLength][]; - for (int j = 2; j < extraLength; j++) { - this.extra[j] = new long[length]; - } - } - else { - int oldLength; // might need to grow the arrays - if (vectorIndex >= (oldLength = this.extra[2].length)) { - for (int j = 2; j < extraLength; j++) { - System.arraycopy(this.extra[j], 0, - (this.extra[j] = new long[vectorIndex + 1]), 0, - oldLength); - } - } - } - this.extra[3][vectorIndex] |= (1L << (position % BitCacheSize)); - if (COVERAGE_TEST_FLAG) { - if (CoverageTestId == 300) { - this.extra[5][vectorIndex] = ~0; - } - } - } - } -} - -public void markAsDefinitelyUnknown(LocalVariableBinding local) { - // protected from non-object locals in calling methods - if (this != DEAD_END) { - this.tagBits |= NULL_FLAG_MASK; - int position; - // position is zero-based - if ((position = local.id + this.maxFieldCount) < BitCacheSize) { // use bits - // set assigned unknown - this.nullBit4 |= (1L << position); - if (COVERAGE_TEST_FLAG) { - if (CoverageTestId == 290) { - this.nullBit1 = 0; - } - } - } - else { - // use extra vector - int vectorIndex = (position / BitCacheSize) - 1; - if (this.extra == null) { - int length = vectorIndex + 1; - this.extra = new long[extraLength][]; - for (int j = 2; j < extraLength; j++) { - this.extra[j] = new long[length]; - } - } - else { - int oldLength; // might need to grow the arrays - if (vectorIndex >= (oldLength = this.extra[2].length)) { - for (int j = 2; j < extraLength; j++) { - System.arraycopy(this.extra[j], 0, - (this.extra[j] = new long[vectorIndex + 1]), 0, - oldLength); - } - } - } - this.extra[5][vectorIndex] |= (1L << (position % BitCacheSize)); - if (COVERAGE_TEST_FLAG) { - if (CoverageTestId == 300) { - this.extra[5][vectorIndex] = ~0; - } - } - } - } -} - -/** - * Mitigate the definite and protected info of flowInfo, depending on what - * this null info registry knows about potential assignments and messages - * sends involving locals. May return flowInfo unchanged, or a modified, - * fresh copy of flowInfo. - * @param flowInfo - FlowInfo: the flow information that this null info - * registry may mitigate - * @return a copy of flowInfo carrying mitigated information, or else - * flowInfo unchanged - */ -public UnconditionalFlowInfo mitigateNullInfoOf(FlowInfo flowInfo) { - if ((this.tagBits & NULL_FLAG_MASK) == 0) { - return flowInfo.unconditionalInits(); - } - long m, m1, nm1, m2, nm2, m3, a2, a3, a4, s1, s2, ns2, s3, ns3, s4, ns4; - boolean newCopy = false; - UnconditionalFlowInfo source = flowInfo.unconditionalInits(); - // clear incompatible protections - m1 = (s1 = source.nullBit1) & (s3 = source.nullBit3) - & (s4 = source.nullBit4) - // prot. non null - & ((a2 = this.nullBit2) | (a4 = this.nullBit4)); - // null or unknown - m2 = s1 & (s2 = this.nullBit2) & (s3 ^ s4) // TODO(stephan): potential typo: should this be "s2 = source.nullBit2"??? - // prot. null - & ((a3 = this.nullBit3) | a4); - // non null or unknown - // clear incompatible assignments - // PREMATURE check effect of protected non null (no NPE on call) - // TODO (maxime) code extensive implementation tests - m3 = s1 & (s2 & (ns3 = ~s3) & (ns4 = ~s4) & (a3 | a4) - | (ns2 = ~s2) & s3 & ns4 & (a2 | a4) - | ns2 & ns3 & s4 & (a2 | a3)); - if ((m = (m1 | m2 | m3)) != 0) { - newCopy = true; - source = source.unconditionalCopy(); - source.nullBit1 &= ~m; - source.nullBit2 &= (nm1 = ~m1) & ((nm2 = ~m2) | a4); - source.nullBit3 &= (nm1 | a2) & nm2; - source.nullBit4 &= nm1 & nm2; - // any variable that is (pot n, pot nn, pot un) at end of try (as captured by *this* NullInfoRegistry) - // has the same uncertainty also for the mitigated case (function result) - // see https://bugs.eclipse.org/bugs/show_bug.cgi?id=320170 - [compiler] [null] Whitebox issues in null analysis - // and org.eclipse.jdt.core.tests.compiler.regression.NullReferenceTest.test0536_try_finally() - long x = ~this.nullBit1 & a2 & a3 & a4; // x is set for all variable ids that have state 0111 (pot n, pot nn, pot un) - if (x != 0) { - // restore state 0111 for all variable ids in x: - source.nullBit1 &= ~x; - source.nullBit2 |= x; - source.nullBit3 |= x; - source.nullBit4 |= x; - } - } - if (this.extra != null && source.extra != null) { - int length = this.extra[2].length, sourceLength = source.extra[0].length; - if (sourceLength < length) { - length = sourceLength; - } - for (int i = 0; i < length; i++) { - m1 = (s1 = source.extra[1 + 1][i]) & (s3 = source.extra[3 + 1][i]) - & (s4 = source.extra[4 + 1][i]) - & ((a2 = this.extra[2 + 1][i]) | (a4 = this.extra[4 + 1][i])); - m2 = s1 & (s2 = this.extra[2 + 1][i]) & (s3 ^ s4) - & ((a3 = this.extra[3 + 1][i]) | a4); - m3 = s1 & (s2 & (ns3 = ~s3) & (ns4 = ~s4) & (a3 | a4) - | (ns2 = ~s2) & s3 & ns4 & (a2 | a4) - | ns2 & ns3 & s4 & (a2 | a3)); - if ((m = (m1 | m2 | m3)) != 0) { - if (! newCopy) { - newCopy = true; - source = source.unconditionalCopy(); - } - source.extra[1 + 1][i] &= ~m; - source.extra[2 + 1][i] &= (nm1 = ~m1) & ((nm2 = ~m2) | a4); - source.extra[3 + 1][i] &= (nm1 | a2) & nm2; - source.extra[4 + 1][i] &= nm1 & nm2; - } - } - } - return source; -} - -public String toString(){ - if (this.extra == null) { - return "NullInfoRegistry<" + this.nullBit1 //$NON-NLS-1$ - + this.nullBit2 + this.nullBit3 + this.nullBit4 - + ">"; //$NON-NLS-1$ - } - else { - String nullS = "NullInfoRegistry<[" + this.nullBit1 //$NON-NLS-1$ - + this.nullBit2 + this.nullBit3 + this.nullBit4; - int i, ceil; - for (i = 0, ceil = this.extra[0].length > 3 ? - 3 : - this.extra[0].length; - i < ceil; i++) { - nullS += "," + this.extra[2][i] //$NON-NLS-1$ - + this.extra[3][i] + this.extra[4][i] + this.extra[5][i]; - } - if (ceil < this.extra[0].length) { - nullS += ",..."; //$NON-NLS-1$ - } - return nullS + "]>"; //$NON-NLS-1$ - } -} - -/** - * Mark a local as potentially having been assigned to an unknown value. - * @param local the local to mark - */ -public void markPotentiallyUnknownBit(LocalVariableBinding local) { - // protected from non-object locals in calling methods - if (this != DEAD_END) { - this.tagBits |= NULL_FLAG_MASK; - int position; - long mask; - if ((position = local.id + this.maxFieldCount) < BitCacheSize) { - // use bits - mask = 1L << position; - isTrue((this.nullBit1 & mask) == 0, "Adding 'unknown' mark in unexpected state"); //$NON-NLS-1$ - this.nullBit4 |= mask; - if (COVERAGE_TEST_FLAG) { - if(CoverageTestId == 46) { - this.nullBit4 = ~0; - } - } - } else { - // use extra vector - int vectorIndex = (position / BitCacheSize) - 1; - if (this.extra == null) { - int length = vectorIndex + 1; - this.extra = new long[extraLength][]; - for (int j = 2; j < extraLength; j++) { - this.extra[j] = new long[length]; - } - } else { - int oldLength; // might need to grow the arrays - if (vectorIndex >= (oldLength = this.extra[2].length)) { - for (int j = 2; j < extraLength; j++) { - System.arraycopy(this.extra[j], 0, - (this.extra[j] = new long[vectorIndex + 1]), 0, - oldLength); - } - } - } - mask = 1L << (position % BitCacheSize); - isTrue((this.extra[2][vectorIndex] & mask) == 0, "Adding 'unknown' mark in unexpected state"); //$NON-NLS-1$ - this.extra[5][vectorIndex] |= mask; - if (COVERAGE_TEST_FLAG) { - if(CoverageTestId == 47) { - this.extra[5][vectorIndex] = ~0; - } - } - } - } -} - -public void markPotentiallyNullBit(LocalVariableBinding local) { - if (this != DEAD_END) { - this.tagBits |= NULL_FLAG_MASK; - int position; - long mask; - if ((position = local.id + this.maxFieldCount) < BitCacheSize) { - // use bits - mask = 1L << position; - isTrue((this.nullBit1 & mask) == 0, "Adding 'potentially null' mark in unexpected state"); //$NON-NLS-1$ - this.nullBit2 |= mask; - if (COVERAGE_TEST_FLAG) { - if(CoverageTestId == 40) { - this.nullBit4 = ~0; - } - } - } else { - // use extra vector - int vectorIndex = (position / BitCacheSize) - 1; - if (this.extra == null) { - int length = vectorIndex + 1; - this.extra = new long[extraLength][]; - for (int j = 2; j < extraLength; j++) { - this.extra[j] = new long[length]; - } - } else { - int oldLength; // might need to grow the arrays - if (vectorIndex >= (oldLength = this.extra[2].length)) { - for (int j = 2; j < extraLength; j++) { - System.arraycopy(this.extra[j], 0, - (this.extra[j] = new long[vectorIndex + 1]), 0, - oldLength); - } - } - } - mask = 1L << (position % BitCacheSize); - this.extra[3][vectorIndex] |= mask; - isTrue((this.extra[2][vectorIndex] & mask) == 0, "Adding 'potentially null' mark in unexpected state"); //$NON-NLS-1$ - if (COVERAGE_TEST_FLAG) { - if(CoverageTestId == 41) { - this.extra[5][vectorIndex] = ~0; - } - } - } - } -} - -public void markPotentiallyNonNullBit(LocalVariableBinding local) { - if (this != DEAD_END) { - this.tagBits |= NULL_FLAG_MASK; - int position; - long mask; - if ((position = local.id + this.maxFieldCount) < BitCacheSize) { - // use bits - mask = 1L << position; - isTrue((this.nullBit1 & mask) == 0, "Adding 'potentially non-null' mark in unexpected state"); //$NON-NLS-1$ - this.nullBit3 |= mask; - if (COVERAGE_TEST_FLAG) { - if(CoverageTestId == 42) { - this.nullBit4 = ~0; - } - } - } else { - // use extra vector - int vectorIndex = (position / BitCacheSize) - 1; - if (this.extra == null) { - int length = vectorIndex + 1; - this.extra = new long[extraLength][]; - for (int j = 2; j < extraLength; j++) { - this.extra[j] = new long[length]; - } - } else { - int oldLength; // might need to grow the arrays - if (vectorIndex >= (oldLength = this.extra[2].length)) { - for (int j = 2; j < extraLength; j++) { - System.arraycopy(this.extra[j], 0, - (this.extra[j] = new long[vectorIndex + 1]), 0, - oldLength); - } - } - } - mask = 1L << (position % BitCacheSize); - isTrue((this.extra[2][vectorIndex] & mask) == 0, "Adding 'potentially non-null' mark in unexpected state"); //$NON-NLS-1$ - this.extra[4][vectorIndex] |= mask; - if (COVERAGE_TEST_FLAG) { - if(CoverageTestId == 43) { - this.extra[5][vectorIndex] = ~0; - } - } - } - } -} -} - diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/flow/SwitchFlowContext.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/flow/SwitchFlowContext.java index c17fa4e2e8..279fff0a36 100644 --- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/flow/SwitchFlowContext.java +++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/flow/SwitchFlowContext.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2000, 2011 IBM Corporation and others. + * Copyright (c) 2000, 2012 IBM Corporation and others. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at @@ -7,6 +7,8 @@ * * Contributors: * IBM Corporation - initial API and implementation + * Stephan Herrmann - Contribution for + * bug 345305 - [compiler][null] Compiler misidentifies a case of "variable can only be null" *******************************************************************************/ package org.eclipse.jdt.internal.compiler.flow; @@ -22,9 +24,12 @@ public class SwitchFlowContext extends FlowContext { public BranchLabel breakLabel; public UnconditionalFlowInfo initsOnBreak = FlowInfo.DEAD_END; -public SwitchFlowContext(FlowContext parent, ASTNode associatedNode, BranchLabel breakLabel) { +public SwitchFlowContext(FlowContext parent, ASTNode associatedNode, BranchLabel breakLabel, boolean isPreTest) { super(parent, associatedNode); this.breakLabel = breakLabel; + if (isPreTest && parent.conditionalLevel > -1) { + this.conditionalLevel++; + } } public BranchLabel breakLabel() { diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/flow/UnconditionalFlowInfo.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/flow/UnconditionalFlowInfo.java index b86fddf4d5..87c84a69d2 100644 --- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/flow/UnconditionalFlowInfo.java +++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/flow/UnconditionalFlowInfo.java @@ -14,6 +14,7 @@ * bug 332637 - Dead Code detection removing code that isn't dead * bug 341499 - [compiler][null] allocate extra bits in all methods of UnconditionalFlowInfo * bug 349326 - [1.7] new warning for missing try-with-resources + * bug 345305 - [compiler][null] Compiler misidentifies a case of "variable can only be null" *******************************************************************************/ package org.eclipse.jdt.internal.compiler.flow; @@ -1664,7 +1665,21 @@ public UnconditionalFlowInfo mergedWith(UnconditionalFlowInfo otherInits) { this.nullBit3 |= x; this.nullBit4 |= x; } - + // workaround for Bug 386181 - [compiler][null] wrong transition in UnconditionalFlowInfo.mergedWith() + ax = a1 & ~a2 & a3 & ~a4; + bx = b1 & ~b2 & ~b3 & b4; + x = ax&bx; + ax = a1 & ~a2 & ~a3 & a4; + bx = b1 & ~b2 & b3 & ~b4; + x |= (ax&bx); + if (x != 0) { + // establish state 0011 for all variable ids in x: + this.nullBit1 &= ~x; + this.nullBit2 &= ~x; + this.nullBit3 |= x; + this.nullBit4 |= x; + } + if (COVERAGE_TEST_FLAG) { if(CoverageTestId == 30) { this.nullBit4 = ~0; @@ -1820,6 +1835,20 @@ public UnconditionalFlowInfo mergedWith(UnconditionalFlowInfo otherInits) { this.extra[4][i] |= x; this.extra[5][i] |= x; } + // workaround for Bug 386181 - [compiler][null] wrong transition in UnconditionalFlowInfo.mergedWith() + ax = a1 & ~a2 & a3 & ~a4; + bx = b1 & ~b2 & ~b3 & b4; + x = ax&bx; + ax = a1 & ~a2 & ~a3 & a4; + bx = b1 & ~b2 & b3 & ~b4; + x |= (ax&bx); + if (x != 0) { + // establish state 0011 for all variable ids in x: + this.extra[2][i] &= ~x; + this.extra[3][i] &= ~x; + this.extra[4][i] |= x; + this.extra[5][i] |= x; + } thisHasNulls = thisHasNulls || this.extra[3][i] != 0 || this.extra[4][i] != 0 || |