Attachment 178716 Details for Bug 185682 – improved fix + regression tests

[patch] improved fix + regression tests

patch185682.txt (text/plain), 17.27 KB, created by Ayushman Jain

on 2010-09-13 02:42:50 EDT

(hide)

Description:

Filename:

MIME Type:

Creator: Ayushman Jain

Created: 2010-09-13 02:42:50 EDT

Size: 17.27 KB

patch

obsolete

>### Eclipse Workspace Patch 1.0
>#P org.eclipse.jdt.core
>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.119
>diff -u -r1.119 SingleNameReference.java
>--- compiler/org/eclipse/jdt/internal/compiler/ast/SingleNameReference.java	9 Sep 2010 17:36:21 -0000	1.119
>+++ compiler/org/eclipse/jdt/internal/compiler/ast/SingleNameReference.java	13 Sep 2010 06:17:14 -0000
>@@ -7,7 +7,8 @@
>  *
>  * Contributors:
>  *     IBM Corporation - initial API and implementation
>- *     Stephan Herrmann <stephan@cs.tu-berlin.de> - Contribution for bug 292478 - Report potentially null across variable assignment
>+ *     Stephan Herrmann <stephan@cs.tu-berlin.de> - Contribution for bug 292478 - Report potentially null across variable assignment,
>+ *     											    Contribution for bug 185682 - Increment/decrement operators mark local variables as read
>  *******************************************************************************/
> package org.eclipse.jdt.internal.compiler.ast;
> 
>@@ -78,10 +79,14 @@
> 					currentScope.problemReporter().uninitializedLocalVariable(localBinding, this);
> 					// we could improve error msg here telling "cannot use compound assignment on final local variable"
> 				}
>-				if (isReachable) {
>-					localBinding.useFlag = LocalVariableBinding.USED;
>-				} else if (localBinding.useFlag == LocalVariableBinding.UNUSED) {
>-					localBinding.useFlag = LocalVariableBinding.FAKE_USED;
>+				if (localBinding.useFlag != LocalVariableBinding.USED) {
>+					// https://bugs.eclipse.org/bugs/show_bug.cgi?id=185682
>+					// access from compound assignment does not prevent "unused" warning, unless unboxing is involved:
>+					if (isReachable && (this.implicitConversion & TypeIds.UNBOXING) != 0) {
>+						localBinding.useFlag = LocalVariableBinding.USED;
>+					} else {
>+						localBinding.useFlag = LocalVariableBinding.FAKE_USED;
>+					}
> 				}
> 		}
> 	}
>@@ -462,6 +467,30 @@
>  * are optimized in one access: e.g "a = a + 1" optimized into "a++".
>  */
> public void generateCompoundAssignment(BlockScope currentScope, CodeStream codeStream, Expression expression, int operator, int assignmentImplicitConversion, boolean valueRequired) {
>+	// https://bugs.eclipse.org/bugs/show_bug.cgi?id=185682
>+	if (!valueRequired && ((this.bits & ASTNode.RestrictiveFlagMASK) == Binding.LOCAL)) {
>+		LocalVariableBinding localBinding = (LocalVariableBinding) this.binding;
>+		if (localBinding.useFlag == LocalVariableBinding.FAKE_USED
>+				&& (localBinding.declaration != null) // unused (and non secret) local
>+				&& ((localBinding.declaration.bits & ASTNode.IsLocalDeclarationReachable) != 0)) {  // declaration is reachable
>+			if (localBinding.declaration instanceof Argument) {
>+				// check compiler options to report against unused arguments
>+				if (currentScope instanceof MethodScope) {
>+					MethodBinding method = ((MethodDeclaration)currentScope.referenceContext()).binding;
>+					if (!(method.isImplementing() && !currentScope.compilerOptions().reportUnusedParameterWhenImplementingAbstract)
>+							&& !(method.isOverriding() && !method.isImplementing() && !currentScope.compilerOptions().reportUnusedParameterWhenOverridingConcrete)
>+							&& !method.isMain()) {
>+						// report the case of an argument that is unread except for postIncrement expressions
>+						currentScope.problemReporter().unusedArgument(localBinding.declaration);
>+					}
>+				}
>+			} else {
>+				// report the case of a local variable that is unread except for this compound assignment
>+				currentScope.problemReporter().unusedLocalVariable(localBinding.declaration);
>+			}
>+			localBinding.useFlag = LocalVariableBinding.USED; // don't report again
>+		}
>+	}
> 	this.generateCompoundAssignment(
> 		currentScope,
> 		codeStream,
>@@ -662,6 +691,30 @@
> 			return;
> 		case Binding.LOCAL : // assigning to a local variable
> 			LocalVariableBinding localBinding = (LocalVariableBinding) this.binding;
>+			// https://bugs.eclipse.org/bugs/show_bug.cgi?id=185682
>+			if (!valueRequired) {
>+				if (localBinding.useFlag == LocalVariableBinding.FAKE_USED
>+						&& (localBinding.declaration != null) // unused (and non secret) local
>+						&& ((localBinding.declaration.bits & ASTNode.IsLocalDeclarationReachable) != 0)) {  // declaration is reachable
>+					if (localBinding.declaration instanceof Argument) {
>+						// check compiler options to report against unused arguments
>+						if (currentScope instanceof MethodScope) {
>+							MethodBinding method = ((MethodDeclaration)currentScope.referenceContext()).binding;
>+							if (!(method.isImplementing() && !currentScope.compilerOptions().reportUnusedParameterWhenImplementingAbstract)
>+									&& !(method.isOverriding() && !method.isImplementing() && !currentScope.compilerOptions().reportUnusedParameterWhenOverridingConcrete)
>+									&& !method.isMain()) {
>+								// report the case of an argument that is unread except for postIncrement expressions
>+								currentScope.problemReporter().unusedArgument(localBinding.declaration);
>+							}
>+						}
>+					} else {
>+						// report the case of a local variable that is unread except for postIncrement expressions
>+						currentScope.problemReporter().unusedLocalVariable(localBinding.declaration);
>+					}
>+					localBinding.useFlag = LocalVariableBinding.USED; // don't report again
>+				}
>+			}
>+
> 			// using incr bytecode if possible
> 			if (localBinding.type == TypeBinding.INT) {
> 				if (valueRequired) {
>#P org.eclipse.jdt.core.tests.compiler
>Index: src/org/eclipse/jdt/core/tests/compiler/regression/ProgrammingProblemsTest.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/ProgrammingProblemsTest.java,v
>retrieving revision 1.23
>diff -u -r1.23 ProgrammingProblemsTest.java
>--- src/org/eclipse/jdt/core/tests/compiler/regression/ProgrammingProblemsTest.java	14 Jul 2010 10:37:24 -0000	1.23
>+++ src/org/eclipse/jdt/core/tests/compiler/regression/ProgrammingProblemsTest.java	13 Sep 2010 06:17:25 -0000
>@@ -7,19 +7,21 @@
>  *
>  * Contributors:
>  *     IBM Corporation - initial API and implementation
>+ *     Stephan Herrmann <stephan@cs.tu-berlin.de> - Contribution for bug 185682 - Increment/decrement operators mark local variables as read
>  *******************************************************************************/
> package org.eclipse.jdt.core.tests.compiler.regression;
> 
> import java.util.HashMap;
> import java.util.Map;
> 
>+import junit.framework.Test;
>+
> import org.eclipse.jdt.core.JavaCore;
> import org.eclipse.jdt.core.compiler.CategorizedProblem;
> import org.eclipse.jdt.internal.compiler.CompilationResult;
> import org.eclipse.jdt.internal.compiler.ICompilerRequestor;
> import org.eclipse.jdt.internal.compiler.classfmt.ClassFileConstants;
> import org.eclipse.jdt.internal.compiler.impl.CompilerOptions;
>-import junit.framework.Test;
> 
> /* Collects potential programming problems tests that are not segregated in a
>  * dedicated test class (aka NullReferenceTest). */
>@@ -1683,4 +1685,250 @@
> 		"The assignment to variable nvx has no effect\n" + 
> 		"----------\n");
> }
>+// https://bugs.eclipse.org/bugs/show_bug.cgi?id=185682
>+public void test0046() {
>+	if (this.complianceLevel < ClassFileConstants.JDK1_5)
>+		return;
>+	Map customOptions = getCompilerOptions();
>+	customOptions.put(CompilerOptions.OPTION_ReportUnusedLocal, CompilerOptions.WARNING);
>+	this.runNegativeTest(
>+			new String[] {
>+				"X.java",
>+				"class X {\n" + 
>+				"    int foo() {\n" + 
>+				"        int i=1;\n" + 
>+				"        boolean b=false;\n" + 
>+				"        b|=true;\n" + 			// not a relevant usage
>+				"        int k = 2;\n" + 
>+				"        --k;\n" + 				// not a relevant usage
>+				"        k+=3;\n" + 			// not a relevant usage
>+				"        Integer j = 3;\n" + 
>+				"        j++;\n" + 				// relevant because unboxing is involved
>+				"        return i++;\n" + 		// value after increment is used
>+				"    }\n" + 
>+				"}"
>+			},
>+			"----------\n" + 
>+			"1. WARNING in X.java (at line 4)\n" + 
>+			"	boolean b=false;\n" + 
>+			"	        ^\n" + 
>+			"The local variable b is never read\n" + 
>+			"----------\n" + 
>+			"2. WARNING in X.java (at line 6)\n" + 
>+			"	int k = 2;\n" + 
>+			"	    ^\n" + 
>+			"The local variable k is never read\n" + 
>+			"----------\n",
>+			null/*classLibraries*/,
>+			true/*shouldFlushOutputDirectory*/,
>+			customOptions);
>+}
>+// https://bugs.eclipse.org/bugs/show_bug.cgi?id=185682
>+public void test0047() {
>+	if (this.complianceLevel < ClassFileConstants.JDK1_5)
>+		return;
>+	Map customOptions = getCompilerOptions();
>+	customOptions.put(CompilerOptions.OPTION_ReportUnusedParameter, CompilerOptions.WARNING);
>+	customOptions.put(CompilerOptions.OPTION_ReportUnusedLocal, CompilerOptions.WARNING);
>+	this.runNegativeTest(
>+			new String[] {
>+				"X.java",
>+				"class X {\n" + 
>+				"    void foo(int param1, int param2, Integer param3) {\n" + 
>+				"        boolean b=false;\n" + 
>+				"        b|=true;\n" + 			// not a relevant usage
>+				"        param1++;\n" + 		// not a relevant usage
>+				"        param2 += 1;\n" + 				// not a relevant usage
>+				"        param3++;\n" + 			// relevant because unboxing is involved
>+				"    }\n" + 
>+				"}"
>+			},
>+			"----------\n" + 
>+			"1. WARNING in X.java (at line 2)\n" + 
>+			"	void foo(int param1, int param2, Integer param3) {\n" + 
>+			"	             ^^^^^^\n" + 
>+			"The parameter param1 is never read\n" + 
>+			"----------\n" + 
>+			"2. WARNING in X.java (at line 2)\n" + 
>+			"	void foo(int param1, int param2, Integer param3) {\n" + 
>+			"	                         ^^^^^^\n" + 
>+			"The parameter param2 is never read\n" + 
>+			"----------\n" + 
>+			"3. WARNING in X.java (at line 3)\n" + 
>+			"	boolean b=false;\n" + 
>+			"	        ^\n" + 
>+			"The local variable b is never read\n" + 
>+			"----------\n",
>+			null/*classLibraries*/,
>+			true/*shouldFlushOutputDirectory*/,
>+			customOptions);
>+}
>+// https://bugs.eclipse.org/bugs/show_bug.cgi?id=185682
>+// To verify that unused parameter warning is not shown for an implementing method's parameter when
>+// CompilerOptions.OPTION_ReportUnusedParameterWhenImplementingAbstract is disabled
>+public void test0048() {
>+	if (this.complianceLevel < ClassFileConstants.JDK1_5)
>+		return;
>+	Map customOptions = getCompilerOptions();
>+	customOptions.put(CompilerOptions.OPTION_ReportUnusedParameter, CompilerOptions.WARNING);
>+	customOptions.put(CompilerOptions.OPTION_ReportUnusedParameterWhenImplementingAbstract, CompilerOptions.DISABLED);
>+	customOptions.put(CompilerOptions.OPTION_ReportUnusedLocal, CompilerOptions.WARNING);
>+	this.runNegativeTest(
>+			new String[] {
>+				"X.java",
>+				"public class X extends A implements Y{\n" + 
>+				"   public void foo(int param1, int param2, Integer param3) {\n" + // implementing method, so dont warn
>+				"        boolean b=false;\n" + 
>+				"        b|=true;\n" + 			// not a relevant usage
>+				"        param1++;\n" + 		// not a relevant usage
>+				"        param2 += 1;\n" + 				// not a relevant usage
>+				"        param3++;\n" + 			// relevant because unboxing is involved
>+				"    }\n" + 
>+				"   public void foo(int param1, int param2) {\n" + // warn
>+				"        boolean b=false;\n" + 
>+				"        b|=true;\n" + 			// not a relevant usage
>+				"        param1++;\n" + 		// not a relevant usage
>+				"        param2 += 1;\n" + 				// not a relevant usage
>+				"    }\n" +
>+				"   public void bar(int param1, int param2, Integer param3) {\n" + // implementing method, so dont warn
>+				"        param1++;\n" + 		// not a relevant usage
>+				"        param2 += 1;\n" + 				// not a relevant usage
>+				"        param3++;\n" + 			// relevant because unboxing is involved
>+				"    }\n" +
>+				"}\n" +
>+				"interface Y{\n" +
>+				"	public void foo(int param1, int param2, Integer param3);" +
>+				"}\n" +
>+				"abstract class A{\n" +
>+				"	public abstract void bar(int param1, int param2, Integer param3);" +
>+				"}\n"
>+			},
>+			"----------\n" + 
>+			"1. WARNING in X.java (at line 3)\n" + 
>+			"	boolean b=false;\n" + 
>+			"	        ^\n" + 
>+			"The local variable b is never read\n" + 
>+			"----------\n" + 
>+			"2. WARNING in X.java (at line 9)\n" + 
>+			"	public void foo(int param1, int param2) {\n" + 
>+			"	                    ^^^^^^\n" + 
>+			"The parameter param1 is never read\n" + 
>+			"----------\n" + 
>+			"3. WARNING in X.java (at line 9)\n" + 
>+			"	public void foo(int param1, int param2) {\n" + 
>+			"	                                ^^^^^^\n" + 
>+			"The parameter param2 is never read\n" + 
>+			"----------\n" + 
>+			"4. WARNING in X.java (at line 10)\n" + 
>+			"	boolean b=false;\n" + 
>+			"	        ^\n" + 
>+			"The local variable b is never read\n" + 
>+			"----------\n",
>+			null/*classLibraries*/,
>+			true/*shouldFlushOutputDirectory*/,
>+			customOptions);
>+}
>+// https://bugs.eclipse.org/bugs/show_bug.cgi?id=185682
>+// To verify that unused parameter warning is not shown for an overriding method's parameter when
>+// CompilerOptions.OPTION_ReportUnusedParameterWhenOverridingConcrete is disabled
>+public void test0049() {
>+	if (this.complianceLevel < ClassFileConstants.JDK1_5)
>+		return;
>+	Map customOptions = getCompilerOptions();
>+	customOptions.put(CompilerOptions.OPTION_ReportUnusedParameter, CompilerOptions.WARNING);
>+	customOptions.put(CompilerOptions.OPTION_ReportUnusedParameterWhenOverridingConcrete, CompilerOptions.DISABLED);
>+	customOptions.put(CompilerOptions.OPTION_ReportUnusedLocal, CompilerOptions.WARNING);
>+	this.runNegativeTest(
>+			new String[] {
>+				"X.java",
>+				"public class X extends A {\n" + 
>+				"   public void foo(int param1, int param2, Integer param3) {\n" + // overriding method, so dont warn
>+				"        boolean b=false;\n" + 
>+				"        b|=true;\n" + 			// not a relevant usage
>+				"        param1++;\n" + 		// not a relevant usage
>+				"        param2 += 1;\n" + 				// not a relevant usage
>+				"        param3++;\n" + 			// relevant because unboxing is involved
>+				"    }\n" + 
>+				"   public void foo(int param1, Integer param3) {\n" + // overriding method, so dont warn
>+				"        param1++;\n" + 		// not a relevant usage
>+				"        param3++;\n" + 			// relevant because unboxing is involved
>+				"    }\n" + 
>+				"}\n" +
>+				"class A{\n" +
>+				"   public void foo(int param1, int param2, Integer param3) {\n" +
>+				"        param1 -=1;\n" + 		// not a relevant usage
>+				"        param2--;\n" + 				// not a relevant usage
>+				"        param3--;\n" + 			// relevant because unboxing is involved
>+				"    }\n" + 
>+				"}\n"
>+			},
>+			"----------\n" + 
>+			"1. WARNING in X.java (at line 3)\n" + 
>+			"	boolean b=false;\n" + 
>+			"	        ^\n" + 
>+			"The local variable b is never read\n" + 
>+			"----------\n" + 
>+			"2. WARNING in X.java (at line 9)\n" + 
>+			"	public void foo(int param1, Integer param3) {\n" + 
>+			"	                    ^^^^^^\n" + 
>+			"The parameter param1 is never read\n" + 
>+			"----------\n" + 
>+			"3. WARNING in X.java (at line 15)\n" + 
>+			"	public void foo(int param1, int param2, Integer param3) {\n" + 
>+			"	                    ^^^^^^\n" + 
>+			"The parameter param1 is never read\n" + 
>+			"----------\n" + 
>+			"4. WARNING in X.java (at line 15)\n" + 
>+			"	public void foo(int param1, int param2, Integer param3) {\n" + 
>+			"	                                ^^^^^^\n" + 
>+			"The parameter param2 is never read\n" + 
>+			"----------\n",
>+			null/*classLibraries*/,
>+			true/*shouldFlushOutputDirectory*/,
>+			customOptions);
>+}
>+// https://bugs.eclipse.org/bugs/show_bug.cgi?id=185682
>+// To verify that unused local warning is not shown for locals declared in unreachable code
>+public void test0050() {
>+	if (this.complianceLevel < ClassFileConstants.JDK1_5)
>+		return;
>+	Map customOptions = getCompilerOptions();
>+	customOptions.put(CompilerOptions.OPTION_ReportUnusedLocal, CompilerOptions.WARNING);
>+	this.runNegativeTest(
>+			new String[] {
>+				"X.java",
>+				"class X {\n" + 
>+				"    int foo() {\n" + 
>+				"        int i=1;\n" +
>+				"		 if (false) {\n" + 
>+				"        	boolean b=false;\n" + 
>+				"        	b|=true;\n" +
>+				"		 }\n" + 			// not a relevant usage
>+				"        int k = 2;\n" + 
>+				"        --k;\n" + 				// not a relevant usage
>+				"        k+=3;\n" + 			// not a relevant usage
>+				"        Integer j = 3;\n" + 
>+				"        j++;\n" + 				// relevant because unboxing is involved
>+				"        return i++;\n" + 		// value after increment is used
>+				"    }\n" + 
>+				"}"
>+			},
>+			"----------\n" + 
>+			"1. WARNING in X.java (at line 4)\n" + 
>+			"	if (false) {\n" + 
>+			"        	boolean b=false;\n" + 
>+			"        	b|=true;\n" + 
>+			"		 }\n" + 
>+			"	           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n" + 
>+			"Dead code\n" + 
>+			"----------\n" + 
>+			"2. WARNING in X.java (at line 8)\n" + 
>+			"	int k = 2;\n" + 
>+			"	    ^\n" + 
>+			"The local variable k is never read\n" + 
>+			"----------\n",
>+			null/*classLibraries*/,
>+			true/*shouldFlushOutputDirectory*/,
>+			customOptions);
>+}
> }
>\ No newline at end of file

Actions: View | Diff

Attachments on bug 185682: 174421 | 178716 | 178806 | 181232 | 181235 | 181303 | 181409 | 181419 | 181439 | 181481 | 181557