Attachment 178458 Details for Bug 292478 – Improved fix for regression

[patch] Improved fix for regression

Bug_292478_regression_fix2.patch (text/plain), 16.84 KB, created by Stephan Herrmann

on 2010-09-08 18:25:32 EDT

(hide)

Description:

Filename:

MIME Type:

Creator: Stephan Herrmann

Created: 2010-09-08 18:25:32 EDT

Size: 16.84 KB

patch

obsolete

>### Eclipse Workspace Patch 1.0
>#P org.eclipse.jdt.core
>Index: compiler/org/eclipse/jdt/internal/compiler/ast/Assignment.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/Assignment.java,v
>retrieving revision 1.91
>diff -u -r1.91 Assignment.java
>--- compiler/org/eclipse/jdt/internal/compiler/ast/Assignment.java	31 Aug 2010 14:58:05 -0000	1.91
>+++ compiler/org/eclipse/jdt/internal/compiler/ast/Assignment.java	8 Sep 2010 22:19:34 -0000
>@@ -53,34 +53,9 @@
> 		.analyseAssignment(currentScope, flowContext, flowInfo, this, false)
> 		.unconditionalInits();
> 	if (local != null && (local.type.tagBits & TagBits.IsBaseType) == 0) {
>-		switch(nullStatus) {
>-			case FlowInfo.NULL :
>-				flowInfo.markAsDefinitelyNull(local);
>-				break;
>-			case FlowInfo.NON_NULL :
>-				flowInfo.markAsDefinitelyNonNull(local);
>-				break;
>-			case FlowInfo.POTENTIALLY_NULL :
>-				flowInfo.markAsPotentiallyNull(local);
>-				break;
>-			default:
>-				flowInfo.markAsDefinitelyUnknown(local);
>-		}
>-		if (flowContext.initsOnFinally != null) {
>-			switch(nullStatus) {
>-				case FlowInfo.NULL :
>-					flowContext.initsOnFinally.markAsDefinitelyNull(local);
>-					break;
>-				case FlowInfo.NON_NULL :
>-					flowContext.initsOnFinally.markAsDefinitelyNonNull(local);
>-					break;
>-				case FlowInfo.POTENTIALLY_NULL :
>-					flowContext.initsOnFinally.markAsPotentiallyNull(local);
>-					break;
>-				default:
>-					flowContext.initsOnFinally.markAsDefinitelyUnknown(local);
>-			}
>-		}
>+		flowInfo.markNullStatus(local, nullStatus);
>+		if (flowContext.initsOnFinally != null)
>+			flowContext.initsOnFinally.markNullStatus(local, nullStatus);
> 	}
> 	return flowInfo;
> }
>Index: compiler/org/eclipse/jdt/internal/compiler/ast/ConditionalExpression.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/ConditionalExpression.java,v
>retrieving revision 1.96
>diff -u -r1.96 ConditionalExpression.java
>--- compiler/org/eclipse/jdt/internal/compiler/ast/ConditionalExpression.java	1 Sep 2010 15:43:21 -0000	1.96
>+++ compiler/org/eclipse/jdt/internal/compiler/ast/ConditionalExpression.java	8 Sep 2010 22:19:35 -0000
>@@ -319,17 +319,42 @@
> 	if (ifTrueNullStatus == ifFalseNullStatus) {
> 		return ifTrueNullStatus;
> 	}
>-	// is there a chance of null? -> potentially null
>+	// is there a chance of null (or non-null)? -> potentially null etc.
> 	// https://bugs.eclipse.org/bugs/show_bug.cgi?id=133125
>+	boolean potentiallyNull = false;
>+	boolean potentiallyNonNull = false;
> 	switch (ifTrueNullStatus) {
> 		case FlowInfo.NULL:
> 		case FlowInfo.POTENTIALLY_NULL:
>-			return FlowInfo.POTENTIALLY_NULL;
>+			potentiallyNull = true;
>+			break;
>+		case FlowInfo.NON_NULL:
>+		case FlowInfo.POTENTIALLY_NON_NULL:
>+			potentiallyNonNull = true;
>+			break;
>+		case FlowInfo.POTENTIALLY_NULL_OR_NON_NULL:
>+			potentiallyNonNull = true;
>+			potentiallyNull = true;
> 	}
> 	switch (ifFalseNullStatus) {
> 		case FlowInfo.NULL:
> 		case FlowInfo.POTENTIALLY_NULL:
>-			return FlowInfo.POTENTIALLY_NULL;
>+			potentiallyNull = true;
>+			break;
>+		case FlowInfo.NON_NULL:
>+		case FlowInfo.POTENTIALLY_NON_NULL:
>+			potentiallyNonNull = true;
>+			break;
>+		case FlowInfo.POTENTIALLY_NULL_OR_NON_NULL:
>+			potentiallyNonNull = true;
>+			potentiallyNull = true;
>+	}
>+	if (potentiallyNull) {
>+		if (potentiallyNonNull)
>+			return FlowInfo.POTENTIALLY_NULL_OR_NON_NULL;
>+		return FlowInfo.POTENTIALLY_NULL;
>+	} else if (potentiallyNonNull) {
>+		return FlowInfo.POTENTIALLY_NON_NULL;
> 	}
> 	return FlowInfo.UNKNOWN;
> 	// cannot decide which branch to take, and they disagree
>Index: compiler/org/eclipse/jdt/internal/compiler/ast/Expression.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/Expression.java,v
>retrieving revision 1.129
>diff -u -r1.129 Expression.java
>--- compiler/org/eclipse/jdt/internal/compiler/ast/Expression.java	31 Aug 2010 14:58:05 -0000	1.129
>+++ compiler/org/eclipse/jdt/internal/compiler/ast/Expression.java	8 Sep 2010 22:19:37 -0000
>@@ -837,15 +837,8 @@
> 	return FlowInfo.NON_NULL; // constant expression cannot be null
> 
> 	LocalVariableBinding local = localVariableBinding();
>-	if (local != null) {
>-		if (flowInfo.isDefinitelyNull(local))
>-			return FlowInfo.NULL;
>-		if (flowInfo.isDefinitelyNonNull(local))
>-			return FlowInfo.NON_NULL;
>-		if (flowInfo.isPotentiallyNull(local))
>-			return FlowInfo.POTENTIALLY_NULL;
>-		return FlowInfo.UNKNOWN;
>-	}
>+	if (local != null)
>+		return flowInfo.nullStatus(local);
> 	return FlowInfo.NON_NULL;
> }
> 
>Index: compiler/org/eclipse/jdt/internal/compiler/ast/LocalDeclaration.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/LocalDeclaration.java,v
>retrieving revision 1.73
>diff -u -r1.73 LocalDeclaration.java
>--- compiler/org/eclipse/jdt/internal/compiler/ast/LocalDeclaration.java	31 Aug 2010 14:58:05 -0000	1.73
>+++ compiler/org/eclipse/jdt/internal/compiler/ast/LocalDeclaration.java	8 Sep 2010 22:19:37 -0000
>@@ -57,19 +57,7 @@
> 	}
> 	flowInfo.markAsDefinitelyAssigned(this.binding);
> 	if ((this.binding.type.tagBits & TagBits.IsBaseType) == 0) {
>-		switch(nullStatus) {
>-			case FlowInfo.NULL :
>-				flowInfo.markAsDefinitelyNull(this.binding);
>-				break;
>-			case FlowInfo.NON_NULL :
>-				flowInfo.markAsDefinitelyNonNull(this.binding);
>-				break;
>-			case FlowInfo.POTENTIALLY_NULL :
>-				flowInfo.markAsPotentiallyNull(this.binding);
>-				break;
>-			default:
>-				flowInfo.markAsDefinitelyUnknown(this.binding);
>-		}
>+		flowInfo.markNullStatus(this.binding, nullStatus);
> 		// no need to inform enclosing try block since its locals won't get
> 		// known by the finally block
> 	}
>Index: compiler/org/eclipse/jdt/internal/compiler/ast/SingleNameReference.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/SingleNameReference.java,v
>retrieving revision 1.118
>diff -u -r1.118 SingleNameReference.java
>--- compiler/org/eclipse/jdt/internal/compiler/ast/SingleNameReference.java	31 Aug 2010 14:58:05 -0000	1.118
>+++ compiler/org/eclipse/jdt/internal/compiler/ast/SingleNameReference.java	8 Sep 2010 22:19:39 -0000
>@@ -770,15 +770,8 @@
> 			return FlowInfo.UNKNOWN;
> 		case Binding.LOCAL : // reading a local variable
> 			LocalVariableBinding local = (LocalVariableBinding) this.binding;
>-			if (local != null) {
>-				if (flowInfo.isDefinitelyNull(local))
>-					return FlowInfo.NULL;
>-				if (flowInfo.isDefinitelyNonNull(local))
>-					return FlowInfo.NON_NULL;
>-				if (flowInfo.isPotentiallyNull(local))
>-					return FlowInfo.POTENTIALLY_NULL;
>-				return FlowInfo.UNKNOWN;
>-			}
>+			if (local != null)
>+				return flowInfo.nullStatus(local);
> 	}
> 	return FlowInfo.NON_NULL; // never get there
> }
>Index: compiler/org/eclipse/jdt/internal/compiler/flow/ConditionalFlowInfo.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/flow/ConditionalFlowInfo.java,v
>retrieving revision 1.31
>diff -u -r1.31 ConditionalFlowInfo.java
>--- compiler/org/eclipse/jdt/internal/compiler/flow/ConditionalFlowInfo.java	31 Aug 2010 14:58:05 -0000	1.31
>+++ compiler/org/eclipse/jdt/internal/compiler/flow/ConditionalFlowInfo.java	8 Sep 2010 22:19:39 -0000
>@@ -162,6 +162,16 @@
> 	this.initsWhenFalse.markAsPotentiallyNull(local);
> }
> 
>+public void markAsPotentiallyNonNull(LocalVariableBinding local) {
>+	this.initsWhenTrue.markAsPotentiallyNonNull(local);
>+	this.initsWhenFalse.markAsPotentiallyNonNull(local);
>+}
>+
>+public void markAsPotentiallyNullOrNonNull(LocalVariableBinding local) {
>+	this.initsWhenTrue.markAsPotentiallyNullOrNonNull(local);
>+	this.initsWhenFalse.markAsPotentiallyNullOrNonNull(local);
>+}
>+
> public void markAsDefinitelyUnknown(LocalVariableBinding local) {
> 	this.initsWhenTrue.markAsDefinitelyUnknown(local);
> 	this.initsWhenFalse.markAsDefinitelyUnknown(local);
>Index: compiler/org/eclipse/jdt/internal/compiler/flow/FlowInfo.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/flow/FlowInfo.java,v
>retrieving revision 1.43
>diff -u -r1.43 FlowInfo.java
>--- compiler/org/eclipse/jdt/internal/compiler/flow/FlowInfo.java	31 Aug 2010 14:58:05 -0000	1.43
>+++ compiler/org/eclipse/jdt/internal/compiler/flow/FlowInfo.java	8 Sep 2010 22:19:40 -0000
>@@ -27,6 +27,8 @@
> 	public final static int NULL = 1;
> 	public final static int NON_NULL = -1;
> 	public final static int POTENTIALLY_NULL = 2;
>+	public final static int POTENTIALLY_NON_NULL = 3;
>+	public final static int POTENTIALLY_NULL_OR_NON_NULL = 4;
> 
> 	public static final UnconditionalFlowInfo DEAD_END; // Represents a dead branch status of initialization
> 	static {
>@@ -255,6 +257,16 @@
> 	abstract public void markAsPotentiallyNull(LocalVariableBinding local);
> 
> 	/**
>+	 * Record a local may have got assigned to non-null.
>+	 */
>+	abstract public void markAsPotentiallyNonNull(LocalVariableBinding local);
>+
>+	/**
>+	 * Record a local may have got assigned to null or to non-null.
>+	 */
>+	abstract public void markAsPotentiallyNullOrNonNull(LocalVariableBinding local);
>+	
>+	/**
> 	 * Record a local got definitely assigned.
> 	 */
> 	abstract public void markAsDefinitelyAssigned(LocalVariableBinding local);
>@@ -265,6 +277,53 @@
> abstract public void markAsDefinitelyUnknown(LocalVariableBinding local);
> 
> /**
>+ * Mark the null status of the given local according to the given status
>+ * @param local
>+ * @param nullStatus one of FLowInfo.NULL ... FlowInfo.POTENTIALLY_NULL_OR_NON_NULL
>+ */
>+public void markNullStatus(LocalVariableBinding local, int nullStatus) {
>+	switch(nullStatus) {
>+		case NULL :
>+			markAsDefinitelyNull(local);
>+			break;
>+		case NON_NULL :
>+			markAsDefinitelyNonNull(local);
>+			break;
>+		case POTENTIALLY_NULL :
>+			markAsPotentiallyNull(local);
>+			break;
>+		case POTENTIALLY_NON_NULL :
>+			markAsPotentiallyNonNull(local);
>+			break;
>+		case POTENTIALLY_NULL_OR_NON_NULL :
>+			markAsPotentiallyNullOrNonNull(local);
>+			break;
>+		default:
>+			markAsDefinitelyUnknown(local);
>+	}
>+}
>+
>+/**
>+ * Answer the null status of the given local
>+ * @param local
>+ * @return one of FLowInfo.NULL ... FlowInfo.POTENTIALLY_NULL_OR_NON_NULL
>+ */
>+public int nullStatus(LocalVariableBinding local) {
>+	if (isDefinitelyNull(local))
>+		return NULL;
>+	if (isDefinitelyNonNull(local))
>+		return NON_NULL;
>+	if (isPotentiallyNull(local)) {
>+		if (isPotentiallyNonNull(local))
>+			return POTENTIALLY_NULL_OR_NON_NULL;
>+		return POTENTIALLY_NULL;
>+	}
>+	if (isPotentiallyNonNull(local))
>+		return POTENTIALLY_NON_NULL;
>+	return UNKNOWN;
>+}
>+
>+/**
>  * Merge branches using optimized boolean conditions
>  */
> public static UnconditionalFlowInfo mergedOptimizedBranches(
>@@ -482,4 +541,5 @@
>  */
> //https://bugs.eclipse.org/bugs/show_bug.cgi?id=303448
> abstract public boolean isMarkedAsNullOrNonNullInAssertExpression(LocalVariableBinding local);
>+
> }
>Index: compiler/org/eclipse/jdt/internal/compiler/flow/UnconditionalFlowInfo.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/flow/UnconditionalFlowInfo.java,v
>retrieving revision 1.67
>diff -u -r1.67 UnconditionalFlowInfo.java
>--- compiler/org/eclipse/jdt/internal/compiler/flow/UnconditionalFlowInfo.java	31 Aug 2010 14:58:05 -0000	1.67
>+++ compiler/org/eclipse/jdt/internal/compiler/flow/UnconditionalFlowInfo.java	8 Sep 2010 22:19:43 -0000
>@@ -1363,6 +1363,72 @@
> 	}
> }
> 
>+public void markAsPotentiallyNonNull(LocalVariableBinding local) {
>+	if (this != DEAD_END) {
>+		this.tagBits |= NULL_FLAG_MASK;
>+        int position;
>+        long mask;
>+        if ((position = local.id + this.maxFieldCount) < BitCacheSize) {
>+            // use bits
>+            this.nullBit1 &= ~(mask = 1L << position);
>+            this.nullBit2 &= ~mask;
>+            this.nullBit3 |= mask;
>+            this.nullBit4 &= ~mask;
>+            if (COVERAGE_TEST_FLAG) {
>+				if(CoverageTestId == 42) {
>+				  	this.nullBit4 = ~0;
>+				}
>+			}
>+        } else {
>+    		// use extra vector
>+    		int vectorIndex ;
>+    		this.extra[2][vectorIndex = (position / BitCacheSize) - 1]
>+    		    &= ~(mask = 1L << (position % BitCacheSize));
>+    		this.extra[3][vectorIndex] &= ~mask;
>+    		this.extra[4][vectorIndex] |= mask;
>+    		this.extra[5][vectorIndex] &= ~mask;
>+    		if (COVERAGE_TEST_FLAG) {
>+				if(CoverageTestId == 43) {
>+					this.extra[5][vectorIndex] = ~0;
>+				}
>+			}
>+    	}
>+	}
>+}
>+
>+public void markAsPotentiallyNullOrNonNull(LocalVariableBinding local) {
>+	if (this != DEAD_END) {
>+		this.tagBits |= NULL_FLAG_MASK;
>+        int position;
>+        long mask;
>+        if ((position = local.id + this.maxFieldCount) < BitCacheSize) {
>+            // use bits
>+            this.nullBit1 &= ~(mask = 1L << position);
>+            this.nullBit2 |= mask;
>+            this.nullBit3 |= mask;
>+            this.nullBit4 &= ~mask;
>+            if (COVERAGE_TEST_FLAG) {
>+				if(CoverageTestId == 44) {
>+				  	this.nullBit4 = ~0;
>+				}
>+			}
>+        } else {
>+    		// use extra vector
>+    		int vectorIndex ;
>+    		this.extra[2][vectorIndex = (position / BitCacheSize) - 1]
>+    		    &= ~(mask = 1L << (position % BitCacheSize));
>+    		this.extra[3][vectorIndex] |= mask;
>+    		this.extra[4][vectorIndex] |= mask;
>+    		this.extra[5][vectorIndex] &= ~mask;
>+    		if (COVERAGE_TEST_FLAG) {
>+				if(CoverageTestId == 45) {
>+					this.extra[5][vectorIndex] = ~0;
>+				}
>+			}
>+    	}
>+	}
>+}
>+
> public UnconditionalFlowInfo mergedWith(UnconditionalFlowInfo otherInits) {
> 	if ((otherInits.tagBits & UNREACHABLE) != 0 && this != DEAD_END) {
> 		if (COVERAGE_TEST_FLAG) {
>#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.101
>diff -u -r1.101 NullReferenceTest.java
>--- src/org/eclipse/jdt/core/tests/compiler/regression/NullReferenceTest.java	1 Sep 2010 15:43:25 -0000	1.101
>+++ src/org/eclipse/jdt/core/tests/compiler/regression/NullReferenceTest.java	8 Sep 2010 22:20:03 -0000
>@@ -35,7 +35,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[] { "testBug320414" };
>+//		TESTS_NAMES = new String[] { "testBug324762" };
> //		TESTS_NUMBERS = new int[] { 561 };
> //		TESTS_RANGE = new int[] { 1, 2049 };
> }
>@@ -13222,4 +13222,77 @@
> 		"Potential null pointer access: The variable y may be null at this location\n" + 
> 		"----------\n");
> }
>+// https://bugs.eclipse.org/bugs/show_bug.cgi?id=292478 -  Report potentially null across variable assignment
>+// test regression reported in comment 8
>+public void testBug292478e() {
>+	this.runConformTest(
>+		new String[] {
>+			"Test.java",
>+			"public class Test {\n" +
>+			"	Object foo(int i, boolean b1, boolean b2) {\n" +
>+			"		Object o1 = null;\n" +
>+			"		done : while (true) { \n" +
>+			"			switch (i) {\n" +
>+			"				case 1 :\n" +
>+			"					Object o2 = null;\n" +
>+			"					if (b2)\n" +
>+			"						o2 = new Object();\n" +
>+			"					o1 = o2;\n" +
>+			"					break;\n" +
>+			"				case 2 :\n" +
>+			"					break done;\n" +
>+			"			}\n" +
>+			"		}		\n" +
>+			"		if (o1 != null)\n" +
>+			"			return o1;\n" +
>+			"		return null;\n" +
>+			"	}\n" +
>+			"}\n"
>+		});
>+}
>+// https://bugs.eclipse.org/bugs/show_bug.cgi?id=292478 -  Report potentially null across variable assignment
>+// variant where regression occurred inside the while-switch structure
>+public void testBug292478f() {
>+	this.runConformTest(
>+		new String[] {
>+			"Test.java",
>+			"public class Test {\n" +
>+			"	Object foo(int i, boolean b1, boolean b2) {\n" +
>+			"		Object o1 = null;\n" +
>+			"		done : while (true) { \n" +
>+			"			switch (i) {\n" +
>+			"				case 1 :\n" +
>+			"					Object o2 = null;\n" +
>+			"					if (b2)\n" +
>+			"						o2 = new Object();\n" +
>+			"					o1 = o2;\n" +
>+			"					if (o1 != null)\n" +
>+			"						return o1;\n" +
>+			"					break;\n" +
>+			"				case 2 :\n" +
>+			"					break done;\n" +
>+			"			}\n" +
>+			"		}		\n" +
>+			"		return null;\n" +
>+			"	}\n" +
>+			"}\n"
>+		});
>+}
>+
>+public void testBug324762() {
>+	this.runConformTest(
>+		new String[] {
>+			"Test.java",
>+			"public class Test {\n" +
>+			"	void zork(boolean b1) {\n" +
>+			"		Object satisfied = null;\n" +
>+			"		if (b1) {\n" +
>+			"			String[] s = new String[] { \"a\", \"b\" };\n" +
>+			"			for (int k = 0; k < s.length && satisfied == null; k++)\n" +
>+			"				satisfied = s.length > 1 ? new Object() : null;\n" +
>+			"		}\n" +
>+			"	}\n" +
>+			"}\n"
>+		});
>+}
> }
>\ No newline at end of file
>

Actions: View | Diff

Attachments on bug 292478: 174559 | 177820 | 178454 | 178458 | 178468 | 178507 | 178515 | 178528