Attachment 182485 Details for Bug 328519 – Proposed fix + regression tests

[patch] Proposed fix + regression tests

patch_328519.txt (text/plain), 15.24 KB, created by Olivier Thomann

on 2010-11-05 11:23:17 EDT

(hide)

Description:

Filename:

MIME Type:

Creator: Olivier Thomann

Created: 2010-11-05 11:23:17 EDT

Size: 15.24 KB

patch

obsolete

>### Eclipse Workspace Patch 1.0
>#P org.eclipse.jdt.core
>Index: compiler/org/eclipse/jdt/internal/compiler/ast/AbstractMethodDeclaration.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/AbstractMethodDeclaration.java,v
>retrieving revision 1.107
>diff -u -r1.107 AbstractMethodDeclaration.java
>--- compiler/org/eclipse/jdt/internal/compiler/ast/AbstractMethodDeclaration.java	9 Aug 2010 03:54:33 -0000	1.107
>+++ compiler/org/eclipse/jdt/internal/compiler/ast/AbstractMethodDeclaration.java	5 Nov 2010 15:22:35 -0000
>@@ -194,6 +194,10 @@
> 					System.arraycopy(problems, 0, problemsCopy, 0, problemsLength);
> 					classFile.addProblemMethod(this, this.binding, problemsCopy, problemResetPC);
> 				}
>+			} else if (e.compilationResult == CodeStream.RESTART_CODE_GEN_FOR_UNUSED_LOCALS_MODE) {
>+				classFile.contentsOffset = problemResetPC;
>+				classFile.methodCount--;
>+				this.generateCode(classFile); // restart method generation
> 			} else {
> 				// produce a problem method accounting for this fatal error
> 				int problemsLength;
>Index: compiler/org/eclipse/jdt/internal/compiler/ast/Clinit.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/Clinit.java,v
>retrieving revision 1.58
>diff -u -r1.58 Clinit.java
>--- compiler/org/eclipse/jdt/internal/compiler/ast/Clinit.java	4 Jan 2010 19:19:59 -0000	1.58
>+++ compiler/org/eclipse/jdt/internal/compiler/ast/Clinit.java	5 Nov 2010 15:22:35 -0000
>@@ -123,6 +123,10 @@
> 					classFile.contentsOffset = clinitOffset;
> 					classFile.methodCount--;
> 				}
>+			} else if (e.compilationResult == CodeStream.RESTART_CODE_GEN_FOR_UNUSED_LOCALS_MODE) {
>+				classFile.contentsOffset = clinitOffset;
>+				classFile.methodCount--;
>+				this.generateCode(classFile); // restart method generation
> 			} else {
> 				// produce a problem method accounting for this fatal error
> 				classFile.contentsOffset = clinitOffset;
>Index: compiler/org/eclipse/jdt/internal/compiler/ast/ConstructorDeclaration.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/ConstructorDeclaration.java,v
>retrieving revision 1.109
>diff -u -r1.109 ConstructorDeclaration.java
>--- compiler/org/eclipse/jdt/internal/compiler/ast/ConstructorDeclaration.java	20 Jul 2010 20:23:19 -0000	1.109
>+++ compiler/org/eclipse/jdt/internal/compiler/ast/ConstructorDeclaration.java	5 Nov 2010 15:22:35 -0000
>@@ -228,6 +228,10 @@
> 				System.arraycopy(problems, 0, problemsCopy, 0, problemsLength);
> 				classFile.addProblemConstructor(this, this.binding, problemsCopy, problemResetPC);
> 			}
>+		} else if (e.compilationResult == CodeStream.RESTART_CODE_GEN_FOR_UNUSED_LOCALS_MODE) {
>+			classFile.contentsOffset = problemResetPC;
>+			classFile.methodCount--;
>+			this.generateCode(classFile); // restart method generation
> 		} else {
> 			int problemsLength;
> 			CategorizedProblem[] problems =
>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.125
>diff -u -r1.125 SingleNameReference.java
>--- compiler/org/eclipse/jdt/internal/compiler/ast/SingleNameReference.java	22 Oct 2010 22:42:55 -0000	1.125
>+++ compiler/org/eclipse/jdt/internal/compiler/ast/SingleNameReference.java	5 Nov 2010 15:22:35 -0000
>@@ -39,6 +39,7 @@
> import org.eclipse.jdt.internal.compiler.lookup.TypeBinding;
> import org.eclipse.jdt.internal.compiler.lookup.TypeIds;
> import org.eclipse.jdt.internal.compiler.lookup.VariableBinding;
>+import org.eclipse.jdt.internal.compiler.problem.AbortMethod;
> import org.eclipse.jdt.internal.compiler.problem.ProblemSeverities;
> 
> public class SingleNameReference extends NameReference implements OperatorIds {
>@@ -422,6 +423,15 @@
> 				break;
> 			case Binding.LOCAL : // reading a local
> 				LocalVariableBinding localBinding = (LocalVariableBinding) this.binding;
>+				if (localBinding.resolvedPosition == -1) {
>+					if (valueRequired) {
>+						// restart code gen
>+						localBinding.useFlag = LocalVariableBinding.USED;
>+						throw new AbortMethod(CodeStream.RESTART_CODE_GEN_FOR_UNUSED_LOCALS_MODE, null);
>+					}
>+					codeStream.recordPositionsFrom(pc, this.sourceStart);
>+					return;
>+				}
> 				if (!valueRequired && (this.implicitConversion & TypeIds.UNBOXING) == 0) {
> 					// if no valueRequired, optimize out entire gen
> 					codeStream.recordPositionsFrom(pc, this.sourceStart);
>@@ -525,6 +535,14 @@
> 		case Binding.LOCAL : // assigning to a local variable (cannot assign to outer local)
> 			LocalVariableBinding localBinding = (LocalVariableBinding) this.binding;
> 			// using incr bytecode if possible
>+			if (localBinding.resolvedPosition == -1) {
>+				if (valueRequired) {
>+					// restart code gen
>+					localBinding.useFlag = LocalVariableBinding.USED;
>+					throw new AbortMethod(CodeStream.RESTART_CODE_GEN_FOR_UNUSED_LOCALS_MODE, null);
>+				}
>+				return;
>+			}
> 			switch (localBinding.type.id) {
> 				case T_JavaLangString :
> 					codeStream.generateStringConcatenationAppend(currentScope, this, expression);
>@@ -687,6 +705,14 @@
> 			// https://bugs.eclipse.org/bugs/show_bug.cgi?id=185682
> 			// check if postIncrement is the only usage of this local
> 			Reference.reportOnlyUselesslyReadLocal(currentScope, localBinding, valueRequired);
>+			if (localBinding.resolvedPosition == -1) {
>+				if (valueRequired) {
>+					// restart code gen
>+					localBinding.useFlag = LocalVariableBinding.USED;
>+					throw new AbortMethod(CodeStream.RESTART_CODE_GEN_FOR_UNUSED_LOCALS_MODE, null);
>+				}
>+				return;
>+			}
> 
> 			// using incr bytecode if possible
> 			if (localBinding.type == TypeBinding.INT) {
>Index: compiler/org/eclipse/jdt/internal/compiler/codegen/CodeStream.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/codegen/CodeStream.java,v
>retrieving revision 1.176
>diff -u -r1.176 CodeStream.java
>--- compiler/org/eclipse/jdt/internal/compiler/codegen/CodeStream.java	4 Jan 2010 19:19:58 -0000	1.176
>+++ compiler/org/eclipse/jdt/internal/compiler/codegen/CodeStream.java	5 Nov 2010 15:22:35 -0000
>@@ -41,7 +41,8 @@
> 	static LocalVariableBinding[] noLocals = new LocalVariableBinding[LOCALS_INCREMENT];
> 	static LocalVariableBinding[] noVisibleLocals = new LocalVariableBinding[LOCALS_INCREMENT];
> 	public static final CompilationResult RESTART_IN_WIDE_MODE = new CompilationResult((char[])null, 0, 0, 0);
>-	
>+	public static final CompilationResult RESTART_CODE_GEN_FOR_UNUSED_LOCALS_MODE = new CompilationResult((char[])null, 0, 0, 0);
>+
> 	public int allLocalsCounter;
> 	public byte[] bCodeStream;
> 	public ClassFile classFile; // The current classfile it is associated to.
>Index: compiler/org/eclipse/jdt/internal/compiler/lookup/BlockScope.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/BlockScope.java,v
>retrieving revision 1.119
>diff -u -r1.119 BlockScope.java
>--- compiler/org/eclipse/jdt/internal/compiler/lookup/BlockScope.java	13 Sep 2010 13:26:20 -0000	1.119
>+++ compiler/org/eclipse/jdt/internal/compiler/lookup/BlockScope.java	5 Nov 2010 15:22:35 -0000
>@@ -191,7 +191,7 @@
> 			LocalVariableBinding local = this.locals[ilocal]; // if no local at all, will be locals[ilocal]==null
> 
> 			// check if variable is actually used, and may force it to be preserved
>-			boolean generateCurrentLocalVar = (local.useFlag != LocalVariableBinding.UNUSED && local.constant() == Constant.NotAConstant);
>+			boolean generateCurrentLocalVar = (local.useFlag > LocalVariableBinding.UNUSED && local.constant() == Constant.NotAConstant);
> 
> 			// do not report fake used variable
> 			if (local.useFlag == LocalVariableBinding.UNUSED
>Index: eval/org/eclipse/jdt/internal/eval/CodeSnippetSingleNameReference.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.jdt.core/eval/org/eclipse/jdt/internal/eval/CodeSnippetSingleNameReference.java,v
>retrieving revision 1.59
>diff -u -r1.59 CodeSnippetSingleNameReference.java
>--- eval/org/eclipse/jdt/internal/eval/CodeSnippetSingleNameReference.java	22 Oct 2010 22:42:56 -0000	1.59
>+++ eval/org/eclipse/jdt/internal/eval/CodeSnippetSingleNameReference.java	5 Nov 2010 15:22:35 -0000
>@@ -38,6 +38,7 @@
> import org.eclipse.jdt.internal.compiler.lookup.TypeBinding;
> import org.eclipse.jdt.internal.compiler.lookup.TypeIds;
> import org.eclipse.jdt.internal.compiler.lookup.VariableBinding;
>+import org.eclipse.jdt.internal.compiler.problem.AbortMethod;
> 
> /**
>  * A single name reference inside a code snippet can denote a field of a remote
>@@ -270,6 +271,15 @@
> 				break;
> 			case Binding.LOCAL : // reading a local
> 				LocalVariableBinding localBinding = (LocalVariableBinding) this.binding;
>+				if (localBinding.resolvedPosition == -1) {
>+					if (valueRequired) {
>+						// restart code gen
>+						localBinding.useFlag = LocalVariableBinding.USED;
>+						throw new AbortMethod(CodeStream.RESTART_CODE_GEN_FOR_UNUSED_LOCALS_MODE, null);
>+					}
>+					codeStream.recordPositionsFrom(pc, this.sourceStart);
>+					return;
>+				}
> 				if (!valueRequired)
> 					break;
> 				// outer local?
>@@ -340,6 +350,14 @@
> 			break;
> 		case Binding.LOCAL : // assigning to a local variable (cannot assign to outer local)
> 			LocalVariableBinding localBinding = (LocalVariableBinding) this.binding;
>+			if (localBinding.resolvedPosition == -1) {
>+				if (valueRequired) {
>+					// restart code gen
>+					localBinding.useFlag = LocalVariableBinding.USED;
>+					throw new AbortMethod(CodeStream.RESTART_CODE_GEN_FOR_UNUSED_LOCALS_MODE, null);
>+				}
>+				return;
>+			}
> 			// using incr bytecode if possible
> 			switch (localBinding.type.id) {
> 				case T_JavaLangString :
>#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.26
>diff -u -r1.26 ProgrammingProblemsTest.java
>--- src/org/eclipse/jdt/core/tests/compiler/regression/ProgrammingProblemsTest.java	22 Oct 2010 22:42:32 -0000	1.26
>+++ src/org/eclipse/jdt/core/tests/compiler/regression/ProgrammingProblemsTest.java	5 Nov 2010 15:22:36 -0000
>@@ -11,6 +11,7 @@
>  *******************************************************************************/
> package org.eclipse.jdt.core.tests.compiler.regression;
> 
>+import java.io.File;
> import java.util.HashMap;
> import java.util.Map;
> 
>@@ -36,7 +37,7 @@
>   	// -Dcompliance=1.4 (for example) to lower it if needed
>   	static {
> //    	TESTS_NAMES = new String[] { "test001" };
>-//		TESTS_NUMBERS = new int[] { 43 };
>+//		TESTS_NUMBERS = new int[] { 53 };
> //  	TESTS_RANGE = new int[] { 1, -1 };
>   	}
> 
>@@ -47,7 +48,11 @@
> public static Class testClass() {
>     return ProgrammingProblemsTest.class;
> }
>-
>+protected Map getCompilerOptions() {
>+	Map compilerOptions = super.getCompilerOptions();
>+	compilerOptions.put(CompilerOptions.OPTION_PreserveUnusedLocal,  CompilerOptions.OPTIMIZE_OUT);
>+	return compilerOptions;
>+}
> void runTest(
> 		String[] testFiles,
> 		String[] errorOptions,
>@@ -2120,5 +2125,111 @@
> 			true/*shouldFlushOutputDirectory*/,
> 			customOptions);
> }
>-
>+//https://bugs.eclipse.org/bugs/show_bug.cgi?id=328519
>+public void test0052() throws Exception {
>+	Map customOptions = getCompilerOptions();
>+	customOptions.put(CompilerOptions.OPTION_ReportUnusedLocal, CompilerOptions.ERROR);
>+	this.runConformTest(
>+		new String[] {
>+			"X.java",
>+			"class X {\n" + 
>+			"    int foo() {\n" + 
>+			"        int i=1;\n" +
>+			"        return i++;\n" + 	// value is used as it is returned
>+			"    }\n" + 
>+			"}"
>+		},
>+		"",
>+		null/*classLibraries*/,
>+		true/*shouldFlushOutputDirectory*/,
>+		null,
>+		customOptions,
>+		null);
>+	String expectedOutput =
>+		"  // Method descriptor #15 ()I\n" + 
>+		"  // Stack: 1, Locals: 2\n" + 
>+		"  int foo();\n" + 
>+		"    0  iconst_1\n" + 
>+		"    1  istore_1 [i]\n" + 
>+		"    2  iload_1 [i]\n" + 
>+		"    3  iinc 1 1 [i]\n" + 
>+		"    6  ireturn\n" + 
>+		"      Line numbers:\n" + 
>+		"        [pc: 0, line: 3]\n" + 
>+		"        [pc: 2, line: 4]\n" + 
>+		"      Local variable table:\n" + 
>+		"        [pc: 0, pc: 7] local: this index: 0 type: X\n" + 
>+		"        [pc: 2, pc: 7] local: i index: 1 type: int\n";
>+	checkDisassembledClassFile(OUTPUT_DIR + File.separator + "X.class", "X", expectedOutput);
>+}
>+//https://bugs.eclipse.org/bugs/show_bug.cgi?id=328519
>+public void test0053() throws Exception {
>+	Map customOptions = getCompilerOptions();
>+	customOptions.put(CompilerOptions.OPTION_ReportUnusedLocal, CompilerOptions.WARNING);
>+	this.runConformTest(
>+		new String[] {
>+			"X.java",
>+			"class X {\n" + 
>+			"    int foo() {\n" + 
>+			"        int i=1;\n" +
>+			"        i++;\n" + 	// value after increment is still not used
>+			"        return 0;\n" + 
>+			"    }\n" + 
>+			"}"
>+		},
>+		"",
>+		null/*classLibraries*/,
>+		true/*shouldFlushOutputDirectory*/,
>+		null,
>+		customOptions,
>+		null);
>+	String expectedOutput =
>+		"  // Method descriptor #15 ()I\n" + 
>+		"  // Stack: 1, Locals: 1\n" + 
>+		"  int foo();\n" + 
>+		"    0  iconst_0\n" + 
>+		"    1  ireturn\n" + 
>+		"      Line numbers:\n" + 
>+		"        [pc: 0, line: 5]\n" + 
>+		"      Local variable table:\n" + 
>+		"        [pc: 0, pc: 2] local: this index: 0 type: X\n";
>+	checkDisassembledClassFile(OUTPUT_DIR + File.separator + "X.class", "X", expectedOutput);
>+}
>+//https://bugs.eclipse.org/bugs/show_bug.cgi?id=328519
>+public void test0054() throws Exception {
>+	Map customOptions = getCompilerOptions();
>+	customOptions.put(CompilerOptions.OPTION_ReportUnusedLocal, CompilerOptions.ERROR);
>+	this.runConformTest(
>+		new String[] {
>+			"X.java",
>+			"class X {\n" + 
>+			"    int foo() {\n" + 
>+			"        int i=1;\n" +
>+			"        return i+=1;\n" + 	// value is used as it is returned
>+			"    }\n" + 
>+			"}"
>+		},
>+		"",
>+		null/*classLibraries*/,
>+		true/*shouldFlushOutputDirectory*/,
>+		null,
>+		customOptions,
>+		null);
>+	String expectedOutput =
>+		"  // Method descriptor #15 ()I\n" + 
>+		"  // Stack: 1, Locals: 2\n" + 
>+		"  int foo();\n" + 
>+		"    0  iconst_1\n" + 
>+		"    1  istore_1 [i]\n" + 
>+		"    2  iinc 1 1 [i]\n" + 
>+		"    5  iload_1 [i]\n" + 
>+		"    6  ireturn\n" + 
>+		"      Line numbers:\n" + 
>+		"        [pc: 0, line: 3]\n" + 
>+		"        [pc: 2, line: 4]\n" + 
>+		"      Local variable table:\n" + 
>+		"        [pc: 0, pc: 7] local: this index: 0 type: X\n" + 
>+		"        [pc: 2, pc: 7] local: i index: 1 type: int\n";
>+	checkDisassembledClassFile(OUTPUT_DIR + File.separator + "X.class", "X", expectedOutput);
>+}
> }
>\ No newline at end of file

Actions: View | Diff

Attachments on bug 328519: 181676 | 181728 | 181737 | 182485 | 182577 | 182648 | 182652 | 182714 | 182763