Attachment 189794 Details for Bug 336428 – slightly improved patch with more tests

[patch] slightly improved patch with more tests

Bug_336428_v1.patch (text/plain), 8.87 KB, created by Stephan Herrmann

on 2011-02-25 08:19:03 EST

(hide)

Description:

Filename:

MIME Type:

Creator: Stephan Herrmann

Created: 2011-02-25 08:19:03 EST

Size: 8.87 KB

patch

obsolete

>### 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 <stephan@cs.tu-berlin.de> - 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.

Actions: View | Diff

Attachments on bug 336428: 188390 | 188391 | 189794