### Eclipse Workspace Patch 1.0 #P org.eclipse.jdt.core Index: compiler/org/eclipse/jdt/internal/compiler/flow/LoopingFlowContext.java =================================================================== RCS file: /cvsroot/eclipse/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/flow/LoopingFlowContext.java,v retrieving revision 1.52 diff -u -r1.52 LoopingFlowContext.java --- compiler/org/eclipse/jdt/internal/compiler/flow/LoopingFlowContext.java 16 Feb 2011 18:57:48 -0000 1.52 +++ compiler/org/eclipse/jdt/internal/compiler/flow/LoopingFlowContext.java 25 Feb 2011 13:11:06 -0000 @@ -7,6 +7,7 @@ * * Contributors: * IBM Corporation - initial API and implementation + * Stephan Herrmann - contribution for Bug 336428 - [compiler][null] bogus warning "redundant null check" in condition of do {} while() loop *******************************************************************************/ package org.eclipse.jdt.internal.compiler.flow; @@ -525,11 +526,18 @@ if ((this.tagBits & FlowContext.HIDE_NULL_COMPARISON_WARNING) == 0) { recordNullReference(local, reference, checkType); } - } else if (! flowInfo.cannotBeDefinitelyNullOrNonNull(local)) { - if (flowInfo.isPotentiallyNonNull(local)) { - recordNullReference(local, reference, CAN_ONLY_NON_NULL | checkType & CONTEXT_MASK); - } else { - if ((this.tagBits & FlowContext.HIDE_NULL_COMPARISON_WARNING) == 0) { + } else if (flowInfo.cannotBeDefinitelyNullOrNonNull(local)) { + return; // no reason to complain, since there is definitely some uncertainty making the comparison relevant. + } else { + if ((this.tagBits & FlowContext.HIDE_NULL_COMPARISON_WARNING) == 0) { + // note: pot non-null & pot null is already captured by cannotBeDefinitelyNullOrNonNull() + if (flowInfo.isPotentiallyNonNull(local)) { + // knowing 'local' can be non-null, we're only interested in seeing whether it can *only* be non-null + recordNullReference(local, reference, CAN_ONLY_NON_NULL | checkType & CONTEXT_MASK); + } else if (flowInfo.isPotentiallyNull(local)) { + // knowing 'local' can be null, we're only interested in seeing whether it can *only* be null + recordNullReference(local, reference, CAN_ONLY_NULL | checkType & CONTEXT_MASK); + } else { recordNullReference(local, reference, checkType); } } #P org.eclipse.jdt.core.tests.compiler Index: src/org/eclipse/jdt/core/tests/compiler/regression/NullReferenceTest.java =================================================================== RCS file: /cvsroot/eclipse/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/NullReferenceTest.java,v retrieving revision 1.112 diff -u -r1.112 NullReferenceTest.java --- src/org/eclipse/jdt/core/tests/compiler/regression/NullReferenceTest.java 16 Feb 2011 18:57:51 -0000 1.112 +++ src/org/eclipse/jdt/core/tests/compiler/regression/NullReferenceTest.java 25 Feb 2011 13:11:24 -0000 @@ -8,7 +8,13 @@ * Contributors: * IBM Corporation - initial API and implementation * Stephan Herrmann - Contributions for - * bugs 325755, 133125, 292478, 319201, 320170 and 332637 + * bug 325755 - [compiler] wrong initialization state after conditional expression + * bug 133125 - [compiler][null] need to report the null status of expressions and analyze them simultaneously + * bug 292478 - Report potentially null across variable assignment + * bug 319201 - [null] no warning when unboxing SingleNameReference causes NPE + * bug 320170 - [compiler] [null] Whitebox issues in null analysis + * bug 332637 - Dead Code detection removing code that isn't dead + * bug 336428 - [compiler][null] bogus warning "redundant null check" in condition of do {} while() loop *******************************************************************************/ package org.eclipse.jdt.core.tests.compiler.regression; @@ -36,7 +42,7 @@ // 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[] { "testBug325229" }; +// TESTS_NAMES = new String[] { "testBug336428e" }; // TESTS_NUMBERS = new int[] { 561 }; // TESTS_RANGE = new int[] { 1, 2049 }; } @@ -13833,6 +13839,149 @@ ""); } +// Bug 336428 - [compiler][null] bogus warning "redundant null check" in condition of do {} while() loop +// original issue +public void testBug336428() { + this.runConformTest( + new String[] { + "DoWhileBug.java", + "public class DoWhileBug {\n" + + " void test(boolean b1, Object o1) {\n" + + " Object o2 = new Object();\n" + + " do {\n" + + " if (b1)\n" + + " o1 = null;\n" + + " } while ((o2 = o1) != null);\n" + + " }\n" + + "}" + }, + ""); +} +// Bug 336428 - [compiler][null] bogus warning "redundant null check" in condition of do {} while() loop +// hitting the same implementation branch from within the loop +// information from unknown o1 is not propagated into the loop, analysis currently believes o2 is def null. +public void _testBug336428a() { + this.runConformTest( + new String[] { + "DoWhileBug.java", + "public class DoWhileBug {\n" + + " void test(boolean b1, Object o1) {\n" + + " Object o2 = null;\n" + + " do {\n" + + " if (b1)\n" + + " o1 = null;\n" + + " if ((o2 = o1) != null)\n" + + " break;\n" + + " } while (true);\n" + + " }\n" + + "}" + }, + ""); +} + +// Bug 336428 - [compiler][null] bogus warning "redundant null check" in condition of do {} while() loop +// in this variant the analysis believes o2 is def unknown and doesn't even consider raising a warning. +public void _testBug336428b() { + this.runNegativeTest( + new String[] { + "DoWhileBug.java", + "public class DoWhileBug {\n" + + " void test(boolean b1) {\n" + + " Object o1 = null;\n" + + " Object o2 = null;\n" + + " do {\n" + + " if ((o2 = o1) == null) break;\n" + + " } while (true);\n" + + " }\n" + + "}" + }, + "----------\n" + + "1. ERROR in DoWhileBug.java (at line 6)\n" + + " if ((o2 = o1) == null) break;\n" + + " ^^^^^^^^^\n" + + "Redundant null check: The variable o2 can only be null at this location\n" + + "----------\n"); +} + +// Bug 336428 - [compiler][null] bogus warning "redundant null check" in condition of do {} while() loop +// in this case considering o1 as unknown is correct +public void testBug336428c() { + if (this.complianceLevel >= ClassFileConstants.JDK1_5) { + this.runConformTest( + new String[] { + "DoWhileBug.java", + "public class DoWhileBug {\n" + + " void test(boolean b1, Object o1) {\n" + + " Object o2 = null;\n" + + " do {\n" + + " if ((o2 = o1) == null) break;\n" + + " } while (true);\n" + + " }\n" + + "}" + }, + ""); + } +} + +// Bug 336428 - [compiler][null] bogus warning "redundant null check" in condition of do {} while() loop +// one more if-statement triggers the expected warnings +public void testBug336428d() { + this.runNegativeTest( + new String[] { + "DoWhileBug.java", + "public class DoWhileBug {\n" + + " void test(boolean b1) {\n" + + " Object o1 = null;\n" + + " Object o2 = null;\n" + + " do {\n" + + " if (b1)\n" + + " o1 = null;\n" + + " if ((o2 = o1) == null) break;\n" + + " } while (true);\n" + + " }\n" + + "}" + }, + "----------\n" + + "1. ERROR in DoWhileBug.java (at line 7)\n" + + " o1 = null;\n" + + " ^^\n" + + "Redundant assignment: The variable o1 can only be null at this location\n" + + "----------\n" + + "2. ERROR in DoWhileBug.java (at line 8)\n" + + " if ((o2 = o1) == null) break;\n" + + " ^^^^^^^^^\n" + + "Redundant null check: The variable o2 can only be null at this location\n" + + "----------\n"); +} + +// Bug 336428 - [compiler][null] bogus warning "redundant null check" in condition of do {} while() loop +// same analysis, but assert instead of if suppresses the warning +public void testBug336428e() { + if (this.complianceLevel >= ClassFileConstants.JDK1_5) { + this.runNegativeTest( + new String[] { + "DoWhileBug.java", + "public class DoWhileBug {\n" + + " void test(boolean b1) {\n" + + " Object o1 = null;\n" + + " Object o2 = null;\n" + + " do {\n" + + " if (b1)\n" + + " o1 = null;\n" + + " assert (o2 = o1) == null : \"bug\";\n" + + " } while (true);\n" + + " }\n" + + "}" + }, + "----------\n" + + "1. ERROR in DoWhileBug.java (at line 7)\n" + + " o1 = null;\n" + + " ^^\n" + + "Redundant assignment: The variable o1 can only be null at this location\n" + + "----------\n"); + } +} + // https://bugs.eclipse.org/bugs/show_bug.cgi?id=332838 // Null info of assert statements should not affect flow info // when CompilerOptions.OPTION_IncludeNullInfoFromAsserts is disabled.