Attachment 209552 Details for Bug 247564 – alternative recording of field nullness

[patch] alternative recording of field nullness

Bug_247564_alternatives.patch (text/plain), 8.90 KB, created by Stephan Herrmann

on 2012-01-16 08:27:16 EST

(hide)

Description:

Filename:

MIME Type:

Creator: Stephan Herrmann

Created: 2012-01-16 08:27:16 EST

Size: 8.90 KB

patch

obsolete

>diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/EqualExpression.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/EqualExpression.java
>index 7f47925..6748863 100644
>--- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/EqualExpression.java
>+++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/EqualExpression.java
>@@ -49,16 +49,26 @@
> 		}
> 	}
> 	private void checkVariableComparison(BlockScope scope, FlowContext flowContext, FlowInfo flowInfo, FlowInfo initsWhenTrue, FlowInfo initsWhenFalse, VariableBinding local, int nullStatus, Expression reference) {
>+		boolean report = true;
>+		if (local instanceof FieldBinding) {
>+			FieldBinding field = (FieldBinding) local;
>+			report = field.isStatic() && field.isFinal();
>+			// or if field has null-annotation
>+		}
> 		switch (nullStatus) {
> 			case FlowInfo.NULL :
> 				if (((this.bits & OperatorMASK) >> OperatorSHIFT) == EQUAL_EQUAL) {
>-					flowContext.recordUsingNullReference(scope, local, reference,
>+					if (report) {
>+						flowContext.recordUsingNullReference(scope, local, reference,
> 							FlowContext.CAN_ONLY_NULL_NON_NULL | FlowContext.IN_COMPARISON_NULL, flowInfo);
>+					}
> 					initsWhenTrue.markAsComparedEqualToNull(local); // from thereon it is set
> 					initsWhenFalse.markAsComparedEqualToNonNull(local); // from thereon it is set
> 				} else {
>-					flowContext.recordUsingNullReference(scope, local, reference,
>+					if (report) {
>+						flowContext.recordUsingNullReference(scope, local, reference,
> 							FlowContext.CAN_ONLY_NULL_NON_NULL | FlowContext.IN_COMPARISON_NON_NULL, flowInfo);
>+					}
> 					initsWhenTrue.markAsComparedEqualToNonNull(local); // from thereon it is set
> 					initsWhenFalse.markAsComparedEqualToNull(local); // from thereon it is set
> 				}
>@@ -68,15 +78,19 @@
> 				break;
> 			case FlowInfo.NON_NULL :
> 				if (((this.bits & OperatorMASK) >> OperatorSHIFT) == EQUAL_EQUAL) {
>-					flowContext.recordUsingNullReference(scope, local, reference,
>+					if (report) {
>+						flowContext.recordUsingNullReference(scope, local, reference,
> 							FlowContext.CAN_ONLY_NULL | FlowContext.IN_COMPARISON_NON_NULL, flowInfo);
>+					}
> 					initsWhenTrue.markAsComparedEqualToNonNull(local); // from thereon it is set
> 					if ((flowContext.tagBits & FlowContext.HIDE_NULL_COMPARISON_WARNING) != 0) {
> 						initsWhenTrue.markedAsNullOrNonNullInAssertExpression(local);
> 					}
> 				} else {
>-					flowContext.recordUsingNullReference(scope, local, reference,
>+					if (report) {
>+						flowContext.recordUsingNullReference(scope, local, reference,
> 							FlowContext.CAN_ONLY_NULL | FlowContext.IN_COMPARISON_NULL, flowInfo);
>+					}
> 				}
> 				break;
> 		}
>diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/Expression.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/Expression.java
>index 87d886d..d18886e 100644
>--- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/Expression.java
>+++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/Expression.java
>@@ -529,9 +529,39 @@
> 	VariableBinding local = variableBinding(scope);
> 	if (local != null &&
> 			(local.type.tagBits & TagBits.IsBaseType) == 0) {
>-		if ((this.bits & ASTNode.IsNonNull) == 0) {
>+		boolean suppressReporting = false;
>+		FlowInfo infoForReporting = flowInfo;
>+		if (local instanceof FieldBinding) {
>+			FieldBinding field = (FieldBinding) local;
>+			if (!(field.isStatic() && field.isFinal())) {
>+				// non-constant
>+				int fieldStatus = flowInfo.nullStatus(local);
>+				if (fieldStatus == FlowInfo.NULL) {
>+					// weaken null to pot.null
>+// 					infoForReporting=flowInfo.copy();
>+					infoForReporting=FlowInfo.initial(flowInfo); // we're only interested in the bits for this one field
>+					infoForReporting.markNullStatus(local, FlowInfo.POTENTIALLY_NULL);
>+				} else if ((fieldStatus & FlowInfo.POTENTIALLY_NULL) != 0) {
>+					// report pot.null as-is
>+//				} else {
>+//					// don't report others
>+//					suppressReporting = true;
>+				}
>+			}
>+		}
>+// DOES NOT WORK
>+//		if (local instanceof FieldBinding) {
>+//			FieldBinding field = (FieldBinding) local;
>+//			if (!(field.isStatic() && field.isFinal())) {
>+//				// non-constant
>+//				FlowInfo doubtInfo = flowInfo.copy();
>+//				doubtInfo.markAsDefinitelyUnknown(field);
>+//				infoForReporting = FlowInfo.conditional(flowInfo, doubtInfo);
>+//			}
>+//		}
>+		if ((this.bits & ASTNode.IsNonNull) == 0 && !suppressReporting) {
> 			flowContext.recordUsingNullReference(scope, local, this,
>-					FlowContext.MAY_NULL, flowInfo);
>+					FlowContext.MAY_NULL, infoForReporting);
> 		}
> 		flowInfo.markAsComparedEqualToNonNull(local );
> 			// from thereon it is set
>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 3414264..f1c051c 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
>@@ -144,6 +144,15 @@
> 		return info;
> 	}
> 
>+	public static UnconditionalFlowInfo initial(FlowInfo other) {
>+		UnconditionalFlowInfo info = new UnconditionalFlowInfo();
>+		while(other instanceof ConditionalFlowInfo) {
>+			other = other.initsWhenTrue();
>+		}
>+		info.maxFieldCount = ((UnconditionalFlowInfo) other).maxFieldCount;
>+		return info;
>+	}
>+
> /**
>  * Return the flow info that would result from the path associated to the
>  * value false for the condition expression that generated this flow info.
>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 ca0ca21..1561c37 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
>@@ -1070,17 +1070,7 @@
> 	// protected from non-object locals in calling methods
> 	if (this != DEAD_END) {
> 		this.tagBits |= NULL_FLAG_MASK;
>-		int position;
>-		if (local instanceof FieldBinding) {
>-			// non-final fields may be modified in separate threads and we cannot be sure about their
>-			// definite nullness. Hence, marking as potential non null.
>-			// Also marking it as definitely unknown to avoid deferring null check for these fields
>-			this.markNullStatus(local, FlowInfo.POTENTIALLY_NON_NULL);
>-			this.markAsDefinitelyUnknown(local);
>-			return;
>-		} else {
>-			position = local.id + this.maxFieldCount;
>-		}
>+		int position = local.getAnalysisId(this.maxFieldCount);
> 		long mask;
> 		long a1, a2, a3, a4, na2;
> 		// position is zero-based
>@@ -1177,17 +1167,8 @@
> 	// protected from non-object locals in calling methods
> 	if (this != DEAD_END) {
> 		this.tagBits |= NULL_FLAG_MASK;
>-		int position;
> 		long mask;
>-		// position is zero-based
>-		if (local instanceof FieldBinding) {
>-			// 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.id + this.maxFieldCount;
>-		}
>+		int position = local.getAnalysisId(this.maxFieldCount);
> 		if (position < BitCacheSize) {
> 			// use bits
> 			if (((mask = 1L << position) & this.nullBit1) != 0) {
>@@ -1319,18 +1300,7 @@
> 	if (this != DEAD_END) {
>     	this.tagBits |= NULL_FLAG_MASK;
>     	long mask;
>-    	int position;
>-    	// position is zero-based
>-    	if (local instanceof FieldBinding) {
>-    		// non-final fields may be modified in separate threads and we cannot be sure about their
>-    		// definite nullness. Hence, marking as potential non null.
>-    		// Also marking it as definitely unknown to avoid deferring null check for these fields
>-    		this.markNullStatus(local, FlowInfo.POTENTIALLY_NON_NULL);
>-    		this.markAsDefinitelyUnknown(local);
>-    		return;
>-    	} else {
>-    		position = local.id + this.maxFieldCount;
>-    	}
>+    	int position = local.getAnalysisId(this.maxFieldCount);
>     	if (position < BitCacheSize) { // use bits
>     		// set assigned non null
>     		this.nullBit1 |= (mask = 1L << position);
>@@ -1383,16 +1353,7 @@
> 	if (this != DEAD_END) {
>     	this.tagBits |= NULL_FLAG_MASK;
>     	long mask;
>-    	int position;
>-    	// position is zero-based
>-    	if (local instanceof FieldBinding) {
>-    		// 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.id + this.maxFieldCount;
>-    	}
>+    	int position = local.getAnalysisId(this.maxFieldCount);
>     	if (position < BitCacheSize) { // use bits
>     		// mark assigned null
>     		this.nullBit1 |= (mask = 1L << position);

Actions: View | Diff

Attachments on bug 247564: 185536 | 185729 | 186938 | 187094 | 188023 | 206514 | 206570 | 206572 | 207019 | 209475 | 209486 | 209552 | 209734 | 209741 | 209759 | 209774 | 209775