Attachment 185280 Details for Bug 332637 – proposed patch

[patch] proposed patch

Bug_332637_v2.patch (text/plain), 10.81 KB, created by Stephan Herrmann

on 2010-12-15 18:24:15 EST

(hide)

Description:

Filename:

MIME Type:

Creator: Stephan Herrmann

Created: 2010-12-15 18:24:15 EST

Size: 10.81 KB

patch

obsolete

>### Eclipse Workspace Patch 1.0
>#P org.eclipse.jdt.core
>Index: compiler/org/eclipse/jdt/internal/compiler/ast/TryStatement.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/TryStatement.java,v
>retrieving revision 1.114
>diff -u -r1.114 TryStatement.java
>--- compiler/org/eclipse/jdt/internal/compiler/ast/TryStatement.java	19 Nov 2009 15:57:21 -0000	1.114
>+++ compiler/org/eclipse/jdt/internal/compiler/ast/TryStatement.java	15 Dec 2010 23:05:16 -0000
>@@ -126,15 +126,15 @@
> 								addPotentialInitializationsFrom(
> 									handlingContext.initsOnReturn));
> 				} else {
>+					FlowInfo initsOnException = handlingContext.initsOnException(this.caughtExceptionTypes[i]);
> 					catchInfo =
>-						flowInfo.unconditionalCopy().
>-							addPotentialInitializationsFrom(
>-								handlingContext.initsOnException(
>-									this.caughtExceptionTypes[i]))
>-							.addPotentialInitializationsFrom(tryInfo.unconditionalCopy())
>+						flowInfo.nullInfoLessUnconditionalCopy()
>+							.addPotentialInitializationsFrom(initsOnException)
>+							.addNullInfoFrom(initsOnException)	// null info only from here, this is the only way to enter the catch block
> 							.addPotentialInitializationsFrom(
>-								handlingContext.initsOnReturn.
>-									nullInfoLessUnconditionalCopy());
>+									tryInfo.nullInfoLessUnconditionalCopy())
>+							.addPotentialInitializationsFrom(
>+									handlingContext.initsOnReturn.nullInfoLessUnconditionalCopy());
> 				}
> 
> 				// catch var is always set
>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.32
>diff -u -r1.32 ConditionalFlowInfo.java
>--- compiler/org/eclipse/jdt/internal/compiler/flow/ConditionalFlowInfo.java	9 Sep 2010 17:36:21 -0000	1.32
>+++ compiler/org/eclipse/jdt/internal/compiler/flow/ConditionalFlowInfo.java	15 Dec 2010 23:05:17 -0000
>@@ -35,6 +35,13 @@
> 	return this;
> }
> 
>+public FlowInfo addNullInfoFrom(FlowInfo otherInits) {
>+
>+	this.initsWhenTrue.addNullInfoFrom(otherInits);
>+	this.initsWhenFalse.addNullInfoFrom(otherInits);
>+	return this;
>+}
>+
> public FlowInfo addPotentialInitializationsFrom(FlowInfo otherInits) {
> 
> 	this.initsWhenTrue.addPotentialInitializationsFrom(otherInits);
>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.44
>diff -u -r1.44 FlowInfo.java
>--- compiler/org/eclipse/jdt/internal/compiler/flow/FlowInfo.java	9 Sep 2010 17:36:21 -0000	1.44
>+++ compiler/org/eclipse/jdt/internal/compiler/flow/FlowInfo.java	15 Dec 2010 23:05:17 -0000
>@@ -45,6 +45,13 @@
>  */
> abstract public FlowInfo addInitializationsFrom(FlowInfo otherInits);
> 
>+/**
>+ * Add all null information from otherInits to this flow info and return this.
>+ * The operation models the effect of an unconditional sequence of this flow info
>+ * and otherInits.
>+ */
>+abstract public FlowInfo addNullInfoFrom(FlowInfo otherInits);
>+
> 
> /**
>  * Compose other inits over this flow info, then return this. The operation
>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.70
>diff -u -r1.70 UnconditionalFlowInfo.java
>--- compiler/org/eclipse/jdt/internal/compiler/flow/UnconditionalFlowInfo.java	29 Sep 2010 16:43:28 -0000	1.70
>+++ compiler/org/eclipse/jdt/internal/compiler/flow/UnconditionalFlowInfo.java	15 Dec 2010 23:05:19 -0000
>@@ -87,16 +87,25 @@
> 	public int[] nullStatusChangedInAssert; // https://bugs.eclipse.org/bugs/show_bug.cgi?id=303448
> 
> public FlowInfo addInitializationsFrom(FlowInfo inits) {
>+	return addInfoFrom(inits, true);
>+}
>+public FlowInfo addNullInfoFrom(FlowInfo inits) {
>+	return addInfoFrom(inits, false);
>+}
>+private FlowInfo addInfoFrom(FlowInfo inits, boolean handleInits) {
> 	if (this == DEAD_END)
> 		return this;
> 	if (inits == DEAD_END)
> 		return this;
> 	UnconditionalFlowInfo otherInits = inits.unconditionalInits();
> 
>-	// union of definitely assigned variables,
>-	this.definiteInits |= otherInits.definiteInits;
>-	// union of potentially set ones
>-	this.potentialInits |= otherInits.potentialInits;
>+	if (handleInits) {
>+		// union of definitely assigned variables,
>+		this.definiteInits |= otherInits.definiteInits;
>+		// union of potentially set ones
>+		this.potentialInits |= otherInits.potentialInits;
>+	}
>+	
> 	// combine null information
> 	boolean thisHadNulls = (this.tagBits & NULL_FLAG_MASK) != 0,
> 		otherHasNulls = (otherInits.tagBits & NULL_FLAG_MASK) != 0;
>@@ -218,14 +227,17 @@
> 			}
> 		}
> 		int i;
>-		// manage definite assignment info
>-		for (i = 0; i < mergeLimit; i++) {
>-			this.extra[0][i] |= otherInits.extra[0][i];
>-			this.extra[1][i] |= otherInits.extra[1][i];
>-		}
>-		for (; i < copyLimit; i++) {
>-			this.extra[0][i] = otherInits.extra[0][i];
>-			this.extra[1][i] = otherInits.extra[1][i];
>+		if (handleInits) {
>+			// manage definite assignment info
>+			for (i = 0; i < mergeLimit; i++) {
>+				this.extra[0][i] |= otherInits.extra[0][i];
>+				this.extra[1][i] |= otherInits.extra[1][i];
>+			}
>+			for (; i < copyLimit; i++) {
>+				this.extra[0][i] = otherInits.extra[0][i];
>+				this.extra[1][i] = otherInits.extra[1][i];
>+			
>+			}
> 		}
> 		// tweak limits for nulls
> 		if (!thisHadNulls) {
>#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.106
>diff -u -r1.106 NullReferenceTest.java
>--- src/org/eclipse/jdt/core/tests/compiler/regression/NullReferenceTest.java	29 Sep 2010 16:43:19 -0000	1.106
>+++ src/org/eclipse/jdt/core/tests/compiler/regression/NullReferenceTest.java	15 Dec 2010 23:05:46 -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[] { "testBug325229" };
>+//		TESTS_NAMES = new String[] { "test0560_try_catch" };
> //		TESTS_NUMBERS = new int[] { 561 };
> //		TESTS_RANGE = new int[] { 1, 2049 };
> }
>@@ -5741,7 +5741,7 @@
> 
> // null analysis - try/catch
> public void test0554_try_catch() {
>-	this.runConformTest(
>+	this.runNegativeTest(
> 		new String[] {
> 			"X.java",
> 			"public class X {\n" +
>@@ -5765,14 +5765,18 @@
> 			"    throw new LocalException();\n" +
> 			"  }\n" +
> 			"}\n"},
>-		""
>-		// conservative flow analysis suppresses the warning
>-//		"----------\n" +
>-//		"1. ERROR in X.java (at line 10)\n" +
>-//		"	if (o != null) {\n" +
>-//		"	    ^\n" +
>-//		"Redundant null check: The variable o can only be null at this location\n" +
>-//		"----------\n"
>+		"----------\n" + 
>+		"1. ERROR in X.java (at line 10)\n" + 
>+		"	if (o != null) {\n" + 
>+		"	    ^\n" + 
>+		"Null comparison always yields false: The variable o can only be null at this location\n" + 
>+		"----------\n" + 
>+		"2. WARNING in X.java (at line 10)\n" + 
>+		"	if (o != null) {\n" + 
>+		"    }\n" + 
>+		"	               ^^^^^^^\n" + 
>+		"Dead code\n" + 
>+		"----------\n"
> 	);
> }
> 
>@@ -5887,13 +5891,11 @@
> 			"    private static final long serialVersionUID = 1L;\n" +
> 			"  }\n" +
> 			"}\n"},
>-		"----------\n" +
>-		"1. ERROR in X.java (at line 13)\n" +
>-		"	o.toString();\n" +
>-		"	^\n" +
>-//		"Null pointer access: The variable o can only be null at this location\n" +
>-		"Potential null pointer access: The variable o may be null at this location\n" +
>-		// conservative flow analysis softens the error
>+		"----------\n" + 
>+		"1. ERROR in X.java (at line 13)\n" + 
>+		"	o.toString();\n" + 
>+		"	^\n" + 
>+		"Null pointer access: The variable o can only be null at this location\n" + 
> 		"----------\n",
> 	    JavacTestOptions.Excuse.EclipseWarningConfiguredAsError);
> }
>@@ -5924,13 +5926,11 @@
> 			"    private static final long serialVersionUID = 1L;\n" +
> 			"  }\n" +
> 			"}\n"},
>-		"----------\n" +
>-		"1. ERROR in X.java (at line 12)\n" +
>-		"	o.toString();\n" +
>-		"	^\n" +
>-//		"Null pointer access: The variable o can only be null at this location\n" +
>-		"Potential null pointer access: The variable o may be null at this location\n" +
>-		// conservative flow analysis softens the error
>+		"----------\n" + 
>+		"1. ERROR in X.java (at line 12)\n" + 
>+		"	o.toString();\n" + 
>+		"	^\n" + 
>+		"Null pointer access: The variable o can only be null at this location\n" + 
> 		"----------\n",
> 	    JavacTestOptions.Excuse.EclipseWarningConfiguredAsError);
> }
>@@ -13672,4 +13672,66 @@
> 			"}"},
> 		"null220");
> }
>+// https://bugs.eclipse.org/bugs/show_bug.cgi?id=332637 
>+// Dead Code detection removing code that isn't dead
>+public void testBug332637() {
>+	if (this.complianceLevel < ClassFileConstants.JDK1_5)
>+		return;
>+	this.runConformTest(
>+		new String[] {
>+			"DeadCodeExample.java",
>+			"public class DeadCodeExample {\n" + 
>+			"\n" + 
>+			"	private class CanceledException extends Exception {\n" + 
>+			"	}\n" + 
>+			"\n" + 
>+			"	private interface ProgressMonitor {\n" + 
>+			"		boolean isCanceled();\n" + 
>+			"	}\n" + 
>+			"\n" + 
>+			"	private void checkForCancellation(ProgressMonitor monitor)\n" + 
>+			"			throws CanceledException {\n" + 
>+			"		if (monitor.isCanceled()) {\n" + 
>+			"			throw new CanceledException();\n" + 
>+			"		}\n" + 
>+			"	}\n" + 
>+			"\n" + 
>+			"	private int run() {\n" + 
>+			"\n" + 
>+			"		ProgressMonitor monitor = new ProgressMonitor() {\n" + 
>+			"			private int i = 0;\n" + 
>+			"\n" + 
>+			"			public boolean isCanceled() {\n" + 
>+			"				return (++i == 5);\n" + 
>+			"			}\n" + 
>+			"		};\n" + 
>+			"\n" + 
>+			"		Integer number = null;\n" + 
>+			"\n" + 
>+			"		try {\n" + 
>+			"			checkForCancellation(monitor);\n" + 
>+			"\n" + 
>+			"			number = Integer.valueOf(0);\n" + 
>+			"\n" + 
>+			"			for (String s : new String[10]) {\n" + 
>+			"				checkForCancellation(monitor);\n" + 
>+			"				number++;\n" + 
>+			"			}\n" + 
>+			"			return 0;\n" + 
>+			"		} catch (CanceledException e) {\n" + 
>+			"			System.out.println(\"Canceled after \" + number\n" + 
>+			"				+ \" times through the loop\");\n" + 
>+			"			if (number != null) {\n" + 
>+			"				System.out.println(\"number = \" + number);\n" + 
>+			"			}\n" + 
>+			"			return -1;\n" + 
>+			"		}\n" + 
>+			"	}\n" + 
>+			"\n" + 
>+			"	public static void main(String[] args) {\n" + 
>+			"		System.out.println(new DeadCodeExample().run());\n" + 
>+			"	}\n" + 
>+			"}\n"				
>+		});
>+}
> }
>\ No newline at end of file

Actions: View | Diff

Attachments on bug 332637: 185225 | 185260 | 185280 | 185336 | 185401 | 185402