comment 151 + 149

6 files changed, 90 insertions, 18 deletions

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 32bfe8e815..22562bfd67 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
@@ -49,7 +49,7 @@ 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[] { "testBug247564g" };
+//		TESTS_NAMES = new String[] { "testBug247564k_3" };
 //		TESTS_NUMBERS = new int[] { 561 };
 //		TESTS_RANGE = new int[] { 1, 2049 };
 }
@@ -15590,7 +15590,7 @@ public void testBug247564a_5() {
 }
 
 // null analysis -- simple case for static final field
-// once deferenced, treat as non null. Consistent with local variables.
+// once dereferenced, treat as non null. Consistent with local variables.
 public void testBug247564b() {
 	this.runNegativeTest(
 		new String[] {
@@ -16334,7 +16334,7 @@ public void testBug247564b_7() {
 			"  static final Object o1 = null;\n" +
 			"  static final Object o2 = new Object();\n" +
 			"  static {\n" +
-			"		if (o1.hashCode() == 2){\n" + // report NPE. But deferenced here, so later it should be treated as non null
+			"		if (o1.hashCode() == 2){\n" + // report NPE. But dereferenced here, so later it should be treated as non null
 			"			o = new Object();\n" +
 			"		} else {\n" +
 			"			o = null;\n" +
@@ -17589,6 +17589,40 @@ public void testBug247564k_2() {
 	);
 }
 
+// null analysis -- simple case for constant field in try-catch-finally
+// presence or absence of method call in finally should not affect the behaviour
+public void testBug247564k_3() {
+	this.runNegativeTest(
+		new String[] {
+			"X.java",
+			"public class X {\n" +
+			"  private static final Object f = null;\n" +
+			"	 void gooCalls() throws NumberFormatException{}\n" +
+			"	 void goo(Object var) throws Exception{\n" +
+			"		try {\n" +
+			"			gooCalls();\n" +
+			"		} catch(NumberFormatException e) {\n" +
+			"			if (f.hashCode() == 0){}\n" +
+			"		} finally {\n" +
+			"			gooCalls();\n" +
+			"			if (f.hashCode() == 0){}\n" +
+			"		}\n" +
+			"  }\n" +
+			"}\n"},
+		"----------\n" + 
+		"1. ERROR in X.java (at line 8)\n" + 
+		"	if (f.hashCode() == 0){}\n" + 
+		"	    ^\n" + 
+		"Null pointer access: The field f can only be null at this location\n" + 
+		"----------\n" + 
+		"2. ERROR in X.java (at line 11)\n" + 
+		"	if (f.hashCode() == 0){}\n" + 
+		"	    ^\n" + 
+		"Null pointer access: The field f can only be null at this location\n" + 
+		"----------\n"
+	);
+}
+
 // null analysis -- potentially redundant checks against the same field
 public void testBug247564l_1() {
 	this.runConformTest(
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 c4d4dbd601..6469df3400 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
@@ -118,7 +118,9 @@ public FlowInfo analyseCode(BlockScope currentScope, FlowContext flowContext, Fl
 	manageSyntheticAccessIfNecessary(currentScope, flowInfo);
 	// a method call can result in changed values for fields, 
 	// so wipe out null info for fields collected till now.
-	flowInfo.resetNullInfoForFields();
+	CompilerOptions options = currentScope.compilerOptions();
+	if(options.includeFieldsInNullAnalysis)
+		flowInfo.resetNullInfoForFields();
 	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/TypeDeclaration.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/TypeDeclaration.java
index e492447c3a..df1101b6ee 100644
--- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/TypeDeclaration.java
+++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/TypeDeclaration.java
@@ -650,9 +650,11 @@ private void internalAnalyseCode(FlowContext flowContext, FlowInfo flowInfo) {
 				staticInitializerContext.handledExceptions = Binding.ANY_EXCEPTION; // tolerate them all, and record them
 				/*}*/
 				staticFieldInfo = field.analyseCode(this.staticInitializerScope, staticInitializerContext, staticFieldInfo);
-				if (field.binding != null && ((field.binding.modifiers & ClassFileConstants.AccFinal) != 0))
+				if (field.binding != null && this.scope.compilerOptions().includeFieldsInNullAnalysis
+						&& ((field.binding.modifiers & ClassFileConstants.AccFinal) != 0)) {
 					// we won't reset null Info for constant fields
 					staticFieldInfo.updateConstantFieldsMask(field.binding);
+				}
 				// in case the initializer is not reachable, use a reinitialized flowInfo and enter a fake reachable
 				// branch, since the previous initializer already got the blame.
 				if (staticFieldInfo == FlowInfo.DEAD_END) {
@@ -690,12 +692,17 @@ private void internalAnalyseCode(FlowContext flowContext, FlowInfo flowInfo) {
 	if (this.methods != null) {
 		UnconditionalFlowInfo outerInfo = flowInfo.unconditionalFieldLessCopy();
 		UnconditionalFlowInfo staticFieldUnconditionalInfo = staticFieldInfo.unconditionalInits();
-		flowInfo.addNullInfoFrom(staticFieldUnconditionalInfo.discardNonFieldInitializations());
-		flowInfo.addConstantFieldsMask(staticFieldInfo);	// prevent resetting null info for constant fields inside methods
-		flowInfo.resetNullInfoForFields();	// only preserve null info for constant fields
-
-		FlowInfo constructorInfo = nonStaticFieldInfo.unconditionalInits().discardNonFieldInitializations().addInitializationsFrom(flowInfo);
-		constructorInfo.addConstantFieldsMask(staticFieldUnconditionalInfo); // prevent resetting null info for constant fields inside c'tor too
+		FlowInfo constructorInfo;
+		if (this.scope.compilerOptions().includeFieldsInNullAnalysis) {
+			flowInfo.addNullInfoFrom(staticFieldUnconditionalInfo.discardNonFieldInitializations());
+			flowInfo.addConstantFieldsMask(staticFieldInfo);	// prevent resetting null info for constant fields inside methods
+			flowInfo.resetNullInfoForFields();	// only preserve null info for constant fields
+			constructorInfo = nonStaticFieldInfo.unconditionalInits().discardNonFieldInitializations().addInitializationsFrom(flowInfo);
+			constructorInfo.addConstantFieldsMask(staticFieldUnconditionalInfo); // prevent resetting null info for constant fields inside c'tor too
+		} else {
+			constructorInfo = nonStaticFieldInfo.unconditionalInits().discardNonFieldInitializations().addInitializationsFrom(outerInfo);
+		}
+		
 		for (int i = 0, count = this.methods.length; i < count; i++) {
 			AbstractMethodDeclaration method = this.methods[i];
 			if (method.ignoreFurtherInvestigation)
diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/flow/FlowInfo.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/flow/FlowInfo.java
index 29b0abe816..4a80517797 100644
--- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/flow/FlowInfo.java
+++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/flow/FlowInfo.java
@@ -280,6 +280,8 @@ abstract public void markAsComparedEqualToNull(VariableBinding binding);
 
 	/**
 	 *  variant of {@link #resetNullInfo(VariableBinding)} for resetting null info for all fields
+	 *  Note that each fields status after the reset will become def. unknown i.e. 1001
+	 *  Also this method does not reset constant fields, which are identified by {@link #constantFieldsMask}
 	 */
 	abstract public void resetNullInfoForFields();
 	
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
index 09b442f78c..a70fc4d71b 100644
--- 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
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2006, 2011 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
@@ -11,6 +11,7 @@
  *******************************************************************************/
 package org.eclipse.jdt.internal.compiler.flow;
 
+import org.eclipse.jdt.internal.compiler.lookup.FieldBinding;
 import org.eclipse.jdt.internal.compiler.lookup.VariableBinding;
 
 /**
@@ -40,6 +41,8 @@ public class NullInfoRegistry extends UnconditionalFlowInfo {
  */
 public NullInfoRegistry(UnconditionalFlowInfo upstream) {
 	this.maxFieldCount = upstream.maxFieldCount;
+	this.constantFieldsMask = upstream.constantFieldsMask;
+	this.extraConstantFieldMask = upstream.extraConstantFieldMask;
 	if ((upstream.tagBits & NULL_FLAG_MASK) != 0) {
 		long u1, u2, u3, u4, nu2, nu3, nu4;
 		this.nullBit2 = (u1 = upstream.nullBit1)
@@ -87,6 +90,8 @@ public NullInfoRegistry add(NullInfoRegistry other) {
 	this.nullBit2 |= other.nullBit2;
 	this.nullBit3 |= other.nullBit3;
 	this.nullBit4 |= other.nullBit4;
+	this.maxFieldCount = other.maxFieldCount;
+	this.addConstantFieldsMask(other);
 	if (other.extra != null) {
 		if (this.extra == null) {
 			this.extra = new long[extraLength][];
@@ -120,7 +125,15 @@ public void markAsComparedEqualToNonNull(VariableBinding local) {
 	// protected from non-object locals in calling methods
 	if (this != DEAD_END) {
     	this.tagBits |= NULL_FLAG_MASK;
-    	int position = local.getAnalysisId(this.maxFieldCount);
+    	int position;
+    	if (local instanceof FieldBinding && ((local.modifiers & AccConstant) == AccConstant)) {
+	    	// non-final fields may be modified in separate threads and we cannot be sure about their
+	    	// definite nullness. Hence, marking as definitely unknown to avoid deferring null check for these fields.
+	    	this.markAsDefinitelyUnknown(local);
+	    	return;
+    	} else {
+    		position = local.getAnalysisId(this.maxFieldCount);
+    	}
     	// position is zero-based
     	if (position < BitCacheSize) { // use bits
     		// set protected non null
@@ -165,7 +178,15 @@ public void markAsDefinitelyNonNull(VariableBinding local) {
 	// protected from non-object locals in calling methods
 	if (this != DEAD_END) {
     	this.tagBits |= NULL_FLAG_MASK;
-    	int position = local.getAnalysisId(this.maxFieldCount);
+    	int position;
+    	if (local instanceof FieldBinding && ((local.modifiers & AccConstant) == AccConstant)) {
+	    	// non-final fields may be modified in separate threads and we cannot be sure about their
+	    	// definite nullness. Hence, marking as definitely unknown to avoid deferring null check for these fields.
+	    	this.markAsDefinitelyUnknown(local);
+	    	return;
+    	} else {
+    		position = local.getAnalysisId(this.maxFieldCount);
+    	}
     	if (position < BitCacheSize) { // use bits
     		// set assigned non null
     		this.nullBit3 |= (1L << position);
@@ -210,7 +231,15 @@ public void markAsDefinitelyNull(VariableBinding local) {
 	// protected from non-object locals in calling methods
 	if (this != DEAD_END) {
     	this.tagBits |= NULL_FLAG_MASK;
-    	int position = local.getAnalysisId(this.maxFieldCount);
+    	int position;
+    	if (local instanceof FieldBinding && ((local.modifiers & AccConstant) == AccConstant)) {
+    		// non-final fields may be modified in separate threads and we cannot be sure about their
+    		// definite nullness. Hence, marking as potential null.
+    		this.markNullStatus(local, FlowInfo.POTENTIALLY_NULL);
+	    	return;
+    	} else {
+    		position = local.getAnalysisId(this.maxFieldCount);
+    	}
     	// position is zero-based
     	if (position < BitCacheSize) { // use bits
     		// set assigned null
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 b1e824fc42..bfeeff2bd4 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
@@ -93,9 +93,7 @@ public class UnconditionalFlowInfo extends FlowInfo {
 	// Constants
 	public static final int BitCacheSize = 64; // 64 bits in a long.
 	public int[] nullStatusChangedInAssert; // https://bugs.eclipse.org/bugs/show_bug.cgi?id=303448
-	private static final int AccConstant = ClassFileConstants.AccStatic|ClassFileConstants.AccFinal;
-	
-	public static final int indexOfConstantFieldBitStream = 6; // the index just after nullBit4 i.e. extraLength
+	protected static final int AccConstant = ClassFileConstants.AccStatic|ClassFileConstants.AccFinal;
 
 public FlowInfo addInitializationsFrom(FlowInfo inits) {
 	return addInfoFrom(inits, true);