Attachment 33699 Details for Bug 110030 – Patch to change the behavior on second error - applies to v_634a

[patch] Patch to change the behavior on second error - applies to v_634a

org.eclipse.jdt.core_146_634a_changed_second_diag_policy.txt (text/plain), 18.69 KB, created by Maxime Daniel

on 2006-01-27 07:58:05 EST

(hide)

Description:

Filename:

MIME Type:

Creator: Maxime Daniel

Created: 2006-01-27 07:58:05 EST

Size: 18.69 KB

patch

obsolete

>### Eclipse Workspace Patch 1.0
>#P org.eclipse.jdt.core.tests.compiler
>Index: src/org/eclipse/jdt/core/tests/compiler/regression/AssignmentTest.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/AssignmentTest.java,v
>retrieving revision 1.26
>diff -u -r1.26 AssignmentTest.java
>--- src/org/eclipse/jdt/core/tests/compiler/regression/AssignmentTest.java	24 Jan 2006 10:50:06 -0000	1.26
>+++ src/org/eclipse/jdt/core/tests/compiler/regression/AssignmentTest.java	27 Jan 2006 12:52:28 -0000
>@@ -188,7 +188,7 @@
> 		"2. ERROR in X.java (at line 9)\n" + 
> 		"	if(a!=null)\n" + 
> 		"	   ^\n" + 
>-		"The variable a can only be null; it was either set to null or checked for null when last used\n" + 
>+		"The variable a cannot be null; it was either set to a non-null value or assumed to be non-null when last used\n" + 
> 		"----------\n" + 
> 		"3. ERROR in X.java (at line 13)\n" + 
> 		"	System.out.println(a+b);\n" + 
>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.5
>diff -u -r1.5 NullReferenceTest.java
>--- src/org/eclipse/jdt/core/tests/compiler/regression/NullReferenceTest.java	24 Jan 2006 10:50:06 -0000	1.5
>+++ src/org/eclipse/jdt/core/tests/compiler/regression/NullReferenceTest.java	27 Jan 2006 12:52:30 -0000
>@@ -33,7 +33,7 @@
>   	// -Dcompliance=1.4 (for example) to lower it if needed
>   	static {
> //    	TESTS_NAMES = new String[] { "test011" };
>-//    	TESTS_NUMBERS = new int[] { 729 };   
>+//    	TESTS_NUMBERS = new int[] { 333 };   
> //    	TESTS_NUMBERS = new int[] { 2999 };   
> //    	TESTS_RANGE = new int[] { 2050, -1 }; 
> //  	TESTS_RANGE = new int[] { 1, 2049 }; 
>@@ -1223,11 +1223,6 @@
> 		"	if (o == null && o == null) {\n" + 
> 		"	                 ^\n" + 
> 		"The variable o can only be null; it was either set to null or checked for null when last used\n" + 
>-		"----------\n" + 
>-		"2. ERROR in X.java (at line 6)\n" + 
>-		"	if (o == null) { /* */ }\n" + 
>-		"	    ^\n" + 
>-		"The variable o cannot be null; it was either set to a non-null value or assumed to be non-null when last used\n" + 
> 		"----------\n");
> }
> 
>@@ -1601,15 +1596,7 @@
> }
> 
> // null analysis - if/else
>-// PMT: exactly the case we talked about; what happens is that the first
>-// if shade doubts upon o; what we could do is to avoid marking in case
>-// of error? not sure this is appropriate though, because of inner code
>-// into the if itself; I believe I somewhat did that on purpose: the latest
>-// wins; completed with o.toString()...
>-// basically, the choice is about what we should do in case of error:
>-// neglect the effect of the error, or propagate this effect; the second
>-//  tends to produce less repeated errors (I believe) than the first...
>-// PREMATURE could refine adding a null-dependent reachable mark... not urgent
>+// rationale: erroneous tests no more change null status
> public void test0312_if_else() {
> 	this.runNegativeTest(
> 		new String[] {
>@@ -1619,8 +1606,8 @@
> 			"  void foo() {\n" + 
> 			"    Object o = new Object();\n" + 
> 			"    if (o == null) { /* */ }\n" + // complain 
>-			"    if (o != null) { /* */ }\n" + // quiet
>-			"    o.toString();\n" + // complain
>+			"    if (o != null) { /* */ }\n" + // complain
>+			"    o.toString();\n" + // quiet
> 			"  }\n" + 
> 			"}\n"},
> 		"----------\n" + 
>@@ -1629,10 +1616,10 @@
> 		"	    ^\n" + 
> 		"The variable o cannot be null; it was either set to a non-null value or assumed to be non-null when last used\n" + 
> 		"----------\n" + 
>-		"2. ERROR in X.java (at line 7)\n" + 
>-		"	o.toString();\n" + 
>-		"	^\n" + 
>-		"The variable o may be null\n" + 
>+		"2. ERROR in X.java (at line 6)\n" + 
>+		"	if (o != null) { /* */ }\n" + 
>+		"	    ^\n" + 
>+		"The variable o cannot be null; it was either set to a non-null value or assumed to be non-null when last used\n" + 
> 		"----------\n");
> }
>  
>@@ -2062,6 +2049,30 @@
> 		"----------\n");
> }
> 
>+// null analysis - if/else
>+// avoid double diagnostic (Frédéric)
>+public void test0333_if_else() {
>+	this.runNegativeTest(
>+		new String[] {
>+			"X.java",
>+			"public class X {\n" + 
>+			"  void foo(Object[] o) {\n" + 
>+			"    if (o != null) {\n" +
>+			"      int length = o == null ? 0 : o.length;\n" + // complain
>+			"      for (int i = 0; i < length; i++) {\n" +
>+			"        o[i].toString();\n" + 	// quiet, protected by if above
>+			"      }\n" + 
>+			"    }\n" + 
>+			"  }\n" + 
>+			"}"},
>+		"----------\n" + 
>+		"1. ERROR in X.java (at line 4)\n" + 
>+		"	int length = o == null ? 0 : o.length;\n" + 
>+		"	             ^\n" + 
>+		"The variable o cannot be null; it was either set to a non-null value or assumed to be non-null when last used\n" + 
>+		"----------\n");
>+}
>+
> // null analysis -- while
> public void test0401_while() {
> 	this.runNegativeTest(
>@@ -2551,7 +2562,6 @@
> }
> 
> // null analysis -- while
>-// REVIEW we get one extraneous message that looks a bit strange
> public void test0423_while() {
> 	this.runNegativeTest(
> 		new String[] {
>@@ -2572,11 +2582,6 @@
> 		"	if (o == null) { /* */ }\n" + 
> 		"	    ^\n" + 
> 		"The variable o cannot be null; it was either set to a non-null value or assumed to be non-null when last used\n" + 
>-		"----------\n" + 
>-		"2. ERROR in X.java (at line 8)\n" + 
>-		"	o = null;\n" + 
>-		"	^\n" + 
>-		"The variable o can only be null; it was either set to null or checked for null when last used\n" + 
> 		"----------\n");
> }
> 
>@@ -5230,6 +5235,9 @@
> }
> 
> // moved from AssignmentTest
>+// new policy (do not modify null info in case of problem in comparison)
>+// removes errors on 8 - which is better, and 3 - which might be
>+// questionable
> public void test1004() {
> 	this.runNegativeTest(
> 		new String[] {
>@@ -5261,20 +5269,10 @@
> 		"	    ^\n" + 
> 		"The variable x cannot be null; it was either set to a non-null value or assumed to be non-null when last used\n" + 
> 		"----------\n" + 
>-		"3. ERROR in X.java (at line 6)\n" + 
>-		"	x.foo(null); // 3\n" + 
>-		"	^\n" + 
>-		"The variable x can only be null; it was either set to null or checked for null when last used\n" + 
>-		"----------\n" + 
>-		"4. ERROR in X.java (at line 9)\n" + 
>+		"3. ERROR in X.java (at line 9)\n" + 
> 		"	} else if (x != null) { // 6\n" + 
> 		"	           ^\n" + 
> 		"The variable x cannot be null; it was either set to a non-null value or assumed to be non-null when last used\n" + 
>-		"----------\n" + 
>-		"5. ERROR in X.java (at line 12)\n" + 
>-		"	x.foo(null); // 8\n" + 
>-		"	^\n" + 
>-		"The variable x may be null\n" + 
> 		"----------\n");
> }
> 
>@@ -5428,6 +5426,11 @@
> 		"	if (other != null) {\n" + 
> 		"	    ^^^^^\n" + 
> 		"The variable other cannot be null; it was either set to a non-null value or assumed to be non-null when last used\n" + 
>+		"----------\n" + 
>+		"2. ERROR in X.java (at line 11)\n" + 
>+		"	if (other == null) {\n" + 
>+		"	    ^^^^^\n" + 
>+		"The variable other cannot be null; it was either set to a non-null value or assumed to be non-null when last used\n" + 
> 		"----------\n");
> }
> 
>@@ -5459,7 +5462,6 @@
> 	);
> }
> 
>-// REVIEW here we do not catch the dead branch: x cannot equal this then null with no assignment in between
> public void test1013() {
> 	this.runNegativeTest(
> 		new String[] {
>@@ -5478,11 +5480,6 @@
> 		"	if (x == null) {\n" + 
> 		"	    ^\n" + 
> 		"The variable x cannot be null; it was either set to a non-null value or assumed to be non-null when last used\n" + 
>-		"----------\n" + 
>-		"2. ERROR in X.java (at line 5)\n" + 
>-		"	x.foo(this);\n" + 
>-		"	^\n" + 
>-		"The variable x can only be null; it was either set to null or checked for null when last used\n" + 
> 		"----------\n");
> }
> 
>@@ -5988,7 +5985,7 @@
> 		"2. ERROR in X.java (at line 8)\n" + 
> 		"	if(a!=null)\n" + 
> 		"	   ^\n" + 
>-		"The variable a can only be null; it was either set to null or checked for null when last used\n" + 
>+		"The variable a cannot be null; it was either set to a non-null value or assumed to be non-null when last used\n" + 
> 		"----------\n");
> }
> 
>@@ -6082,7 +6079,7 @@
> 			"      o60 = o0, o61 = o0, o62 = o0, o63 = o0, o64 = o0,\n" + 
> 			"      o65 = o0, o66 = o0, o67 = o0, o68 = o0, o69 = o0;\n" + 
> 			"    if (o65 == null) { /* */ }\n" + // complain 
>-			"    if (o65 != null) { /* */ }\n" + // quiet (already reported)
>+			"    if (o65 != null) { /* */ }\n" + // complain
> 			"  }\n" + 
> 			"}\n"},
> 		"----------\n" + 
>@@ -6090,6 +6087,11 @@
> 		"	if (o65 == null) { /* */ }\n" + 
> 		"	    ^^^\n" + 
> 		"The variable o65 cannot be null; it was either set to a non-null value or assumed to be non-null when last used\n" + 
>+		"----------\n" + 
>+		"2. ERROR in X.java (at line 19)\n" + 
>+		"	if (o65 != null) { /* */ }\n" + 
>+		"	    ^^^\n" + 
>+		"The variable o65 cannot be null; it was either set to a non-null value or assumed to be non-null when last used\n" + 
> 		"----------\n");
> }
> 
>#P org.eclipse.jdt.core
>Index: compiler/org/eclipse/jdt/internal/compiler/ast/EqualExpression.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/EqualExpression.java,v
>retrieving revision 1.60
>diff -u -r1.60 EqualExpression.java
>--- compiler/org/eclipse/jdt/internal/compiler/ast/EqualExpression.java	24 Jan 2006 10:50:38 -0000	1.60
>+++ compiler/org/eclipse/jdt/internal/compiler/ast/EqualExpression.java	27 Jan 2006 12:52:34 -0000
>@@ -36,20 +36,23 @@
> 	private void checkVariableComparison(BlockScope scope, FlowContext flowContext, FlowInfo flowInfo, FlowInfo initsWhenTrue, FlowInfo initsWhenFalse, LocalVariableBinding local, int nullStatus, Expression reference) {
> 		switch (nullStatus) {
> 			case FlowInfo.NULL :
>-				flowContext.recordUsingNullReference(scope, local, reference, 
>-					FlowContext.CAN_ONLY_NULL_NON_NULL, flowInfo);
>-				if (((this.bits & OperatorMASK) >> OperatorSHIFT) == EQUAL_EQUAL) {
>-					initsWhenTrue.markAsComparedEqualToNull(local); // from thereon it is set
>-					initsWhenFalse.markAsComparedEqualToNonNull(local); // from thereon it is set
>-				} else {
>-					initsWhenTrue.markAsComparedEqualToNonNull(local); // from thereon it is set
>-					initsWhenFalse.markAsComparedEqualToNull(local); // from thereon it is set
>+				if (!flowContext.recordUsingNullReference(scope, local, reference, 
>+						FlowContext.CAN_ONLY_NULL_NON_NULL, flowInfo)) {
>+					// no error reported (yet), mark downstream flow
>+					// WORK consider inverting the logic - saves a not operation
>+					if (((this.bits & OperatorMASK) >> OperatorSHIFT) == EQUAL_EQUAL) {
>+						initsWhenTrue.markAsComparedEqualToNull(local); // from thereon it is set
>+						initsWhenFalse.markAsComparedEqualToNonNull(local); // from thereon it is set
>+					} else {
>+						initsWhenTrue.markAsComparedEqualToNonNull(local); // from thereon it is set
>+						initsWhenFalse.markAsComparedEqualToNull(local); // from thereon it is set
>+					}
> 				}
> 				break;
> 			case FlowInfo.NON_NULL :
>-				flowContext.recordUsingNullReference(scope, local, reference, 
>-					FlowContext.CAN_ONLY_NULL, flowInfo);
>-				if (((this.bits & OperatorMASK) >> OperatorSHIFT) == EQUAL_EQUAL) {
>+				if (!flowContext.recordUsingNullReference(scope, local, reference, 
>+							FlowContext.CAN_ONLY_NULL, flowInfo)
>+						&& ((this.bits & OperatorMASK) >> OperatorSHIFT) == EQUAL_EQUAL) {
> 					initsWhenTrue.markAsComparedEqualToNonNull(local); // from thereon it is set
> 				}
> 				break;
>Index: compiler/org/eclipse/jdt/internal/compiler/flow/FinallyFlowContext.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/flow/FinallyFlowContext.java,v
>retrieving revision 1.18
>diff -u -r1.18 FinallyFlowContext.java
>--- compiler/org/eclipse/jdt/internal/compiler/flow/FinallyFlowContext.java	24 Jan 2006 10:50:38 -0000	1.18
>+++ compiler/org/eclipse/jdt/internal/compiler/flow/FinallyFlowContext.java	27 Jan 2006 12:52:34 -0000
>@@ -186,7 +186,7 @@
> 		return true;
> 	}
> 
>-	public void recordUsingNullReference(Scope scope, LocalVariableBinding local, 
>+	public boolean recordUsingNullReference(Scope scope, LocalVariableBinding local, 
> 			Expression reference, int checkType, FlowInfo flowInfo) {
> 		if ((flowInfo.tagBits & FlowInfo.UNREACHABLE) == 0)	{
> 		if (deferNullDiagnostic) { // within an enclosing loop, be conservative
>@@ -196,21 +196,22 @@
> 					if (flowInfo.isProtectedNonNull(local)) {
> 						if (checkType == CAN_ONLY_NULL_NON_NULL) {
> 							scope.problemReporter().localVariableCannotBeNull(local, reference);
>+							return true;
> 						}
>-						return;
>+						return false;
> 					}
> 					if (flowInfo.isProtectedNull(local)) {
> 						scope.problemReporter().localVariableCanOnlyBeNull(local, reference);
>-						return;
>+						return true;
> 					}
> 					break;
> 				case MAY_NULL :
> 					if (flowInfo.isProtectedNonNull(local)) {
>-						return;
>+						return false;
> 					}
> 					if (flowInfo.isProtectedNull(local)) {
> 						scope.problemReporter().localVariableCanOnlyBeNull(local, reference);
>-						return;
>+						return true;
> 					}
> 					break;
> 				default:
>@@ -222,25 +223,25 @@
> 				case CAN_ONLY_NULL_NON_NULL :
> 					if (flowInfo.isDefinitelyNonNull(local)) {
> 						scope.problemReporter().localVariableCannotBeNull(local, reference);				
>-						return;
>+						return true;
> 					}
> 				case CAN_ONLY_NULL:
> 					if (flowInfo.isDefinitelyNull(local)) {
> 						scope.problemReporter().localVariableCanOnlyBeNull(local, reference);
>-						return;
>+						return true;
> 					}
> 					break;
> 				case MAY_NULL :
> 					if (flowInfo.isDefinitelyNull(local)) {
> 						scope.problemReporter().localVariableCanOnlyBeNull(local, reference);
>-						return;
>+						return true;
> 					}
> 					if (flowInfo.isPotentiallyNull(local)) {
> 						scope.problemReporter().localVariableMayBeNull(local, reference);
>-						return;
>+						return true;
> 					}
> 					if (flowInfo.isDefinitelyNonNull(local)) {
>-						return; // shortcut: cannot be null
>+						return false; // shortcut: cannot be null
> 					}
> 					break;
> 				default:
>@@ -250,6 +251,7 @@
> 		recordNullReference(local, reference, checkType); 
> 		// prepare to re-check with try/catch flow info
> 		}
>+		return false;
> 	}
> 	
> 	void removeFinalAssignmentIfAny(Reference reference) {
>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.31
>diff -u -r1.31 LoopingFlowContext.java
>--- compiler/org/eclipse/jdt/internal/compiler/flow/LoopingFlowContext.java	24 Jan 2006 10:50:38 -0000	1.31
>+++ compiler/org/eclipse/jdt/internal/compiler/flow/LoopingFlowContext.java	27 Jan 2006 12:52:34 -0000
>@@ -303,11 +303,11 @@
> 	nullCheckTypes[nullCount++] = status;
> }
> 	
>-public void recordUsingNullReference(Scope scope, LocalVariableBinding local,
>+public boolean recordUsingNullReference(Scope scope, LocalVariableBinding local,
> 		Expression reference, int checkType, FlowInfo flowInfo) {
> 	if ((flowInfo.tagBits & FlowInfo.UNREACHABLE) != 0 || 
> 			flowInfo.isDefinitelyUnknown(local)) {
>-		return;
>+		return false;
> 	}
> 	switch (checkType) {
> 		case CAN_ONLY_NULL_NON_NULL :
>@@ -315,35 +315,37 @@
> 			if (flowInfo.isDefinitelyNonNull(local)) {
> 				if (checkType == CAN_ONLY_NULL_NON_NULL) {
> 					scope.problemReporter().localVariableCannotBeNull(local, reference);
>+					return true;
> 				}
>-				return;
>+				return false;
> 			}
> 			if (flowInfo.isDefinitelyNull(local)) {
> 				scope.problemReporter().localVariableCanOnlyBeNull(local, reference);
>-				return;
>+				return true;
> 			}
> 			if (flowInfo.isPotentiallyUnknown(local)) {
>-				return;
>+				return false;
> 			}
> 			recordNullReference(local, reference, checkType);
>-			return;
>+			return false;
> 		case MAY_NULL :
> 			if (flowInfo.isDefinitelyNonNull(local)) {
>-				return;
>+				return false;
> 			}
> 			if (flowInfo.isDefinitelyNull(local)) {
> 				scope.problemReporter().localVariableCanOnlyBeNull(local, reference);
>-				return;
>+				return true;
> 			}
> 			if (flowInfo.isPotentiallyNull(local)) {
> 				scope.problemReporter().localVariableMayBeNull(local, reference);
>-				return;
>+				return true;
> 			}
> 			recordNullReference(local, reference, checkType);
>-			return;
>+			return false;
> 		default:
> 			// never happens
> 	}
>+	return false;
> }
> 	
> 	void removeFinalAssignmentIfAny(Reference reference) {
>Index: compiler/org/eclipse/jdt/internal/compiler/flow/FlowContext.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/flow/FlowContext.java,v
>retrieving revision 1.47
>diff -u -r1.47 FlowContext.java
>--- compiler/org/eclipse/jdt/internal/compiler/flow/FlowContext.java	24 Jan 2006 10:50:38 -0000	1.47
>+++ compiler/org/eclipse/jdt/internal/compiler/flow/FlowContext.java	27 Jan 2006 12:52:34 -0000
>@@ -472,48 +472,50 @@
>  *  	perform supplementary checks against flow info instances that cannot
>  *  	be known at the time of calling this method (they are influenced by
>  * 		code that follows the current point)
>+ * @return true if a problem has been reported, false else
>  */
>-public void recordUsingNullReference(Scope scope, LocalVariableBinding local, 
>+public boolean recordUsingNullReference(Scope scope, LocalVariableBinding local, 
> 		Expression reference, int checkType, FlowInfo flowInfo) {
> 	if ((flowInfo.tagBits & FlowInfo.UNREACHABLE) != 0 || 
> 			flowInfo.isDefinitelyUnknown(local)) {
>-		return;
>+		return false;
> 	}
> 	switch (checkType) {
> 		case CAN_ONLY_NULL_NON_NULL :
> 			if (flowInfo.isDefinitelyNonNull(local)) {
> 				scope.problemReporter().localVariableCannotBeNull(local, reference);				
>-				return;
>+				return true;
> 			}
> 			else if (flowInfo.isPotentiallyUnknown(local)) {
>-				return;
>+				return false;
> 			}
> 		case CAN_ONLY_NULL:
> 			if (flowInfo.isDefinitelyNull(local)) {
> 				scope.problemReporter().localVariableCanOnlyBeNull(local, reference);
>-				return;
>+				return true;
> 			}
> 			else if (flowInfo.isPotentiallyUnknown(local)) {
>-				return;
>+				return false;
> 			}
> 			break;
> 		case MAY_NULL :
> 			if (flowInfo.isDefinitelyNull(local)) {
> 				scope.problemReporter().localVariableCanOnlyBeNull(local, reference);
>-				return;
>+				return true;
> 			}
> 			if (flowInfo.isPotentiallyNull(local)) {
> 				scope.problemReporter().localVariableMayBeNull(local, reference);
>-				return;
>+				return false;
> 			}
> 			break;
> 		default:
> 			// never happens
> 	}
> 	if (parent != null) {
>-		parent.recordUsingNullReference(scope, local, reference, checkType, 
>+		return parent.recordUsingNullReference(scope, local, reference, checkType, 
> 				flowInfo);
> 	}
>+	return false;
> }
> 
> void removeFinalAssignmentIfAny(Reference reference) {

Actions: View | Diff

Attachments on bug 110030: 30700 | 33507 | 33699