Attachment 202328 Details for Bug 356002 – Proposed fix + regression tests

Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

[patch] Proposed fix + regression tests

patch_356002.txt (text/plain), 6.54 KB, created by Olivier Thomann

on 2011-08-29 10:35:20 EDT

(hide)

Description:

Filename:

MIME Type:

Creator: Olivier Thomann

Created: 2011-08-29 10:35:20 EDT

Size: 6.54 KB

patch

obsolete

>### Eclipse Workspace Patch 1.0
>#P org.eclipse.jdt.core
>Index: compiler/org/eclipse/jdt/internal/compiler/ast/SwitchStatement.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/SwitchStatement.java,v
>retrieving revision 1.80
>diff -u -r1.80 SwitchStatement.java
>--- compiler/org/eclipse/jdt/internal/compiler/ast/SwitchStatement.java	28 Jul 2011 17:06:59 -0000	1.80
>+++ compiler/org/eclipse/jdt/internal/compiler/ast/SwitchStatement.java	29 Aug 2011 14:31:30 -0000
>@@ -175,7 +175,6 @@
> 			}
> 
> 			final boolean hasCases = this.caseCount != 0;
>-			final boolean valueRequired = this.expression.constant == Constant.NotAConstant || hasCases;
> 
> 			StringSwitchCase [] stringCases = new StringSwitchCase[this.caseCount]; // may have to shrink later if multiple strings hash to same code.
> 			BranchLabel[] sourceCaseLabels = new BranchLabel[this.caseCount];
>@@ -242,7 +241,7 @@
> 					codeStream.ifne(stringCases[i].label);
> 				}
> 				codeStream.goto_(defaultBranchLabel);
>-			} else if (valueRequired) {
>+			} else {
> 				codeStream.pop();
> 			}
> 
>@@ -474,6 +473,15 @@
> 					expressionType = null; // fault-tolerance: ignore type mismatch from constants from hereon
> 				}
> 			}
>+			if (isStringSwitch) {
>+				// the secret variable should be created before iterating over the switch's statements that could
>+				// create more locals. This must be done to prevent overlapping of locals
>+				// See https://bugs.eclipse.org/bugs/show_bug.cgi?id=356002
>+				this.dispatchStringCopy  = new LocalVariableBinding(SecretStringVariableName, upperScope.getJavaLangString(), ClassFileConstants.AccDefault, false);
>+				upperScope.addLocalVariable(this.dispatchStringCopy);
>+				this.dispatchStringCopy.setConstant(Constant.NotAConstant);
>+				this.dispatchStringCopy.useFlag = LocalVariableBinding.USED;
>+			}
> 			if (this.statements != null) {
> 				this.scope = new BlockScope(upperScope);
> 				int length;
>@@ -522,12 +530,6 @@
> 					upperScope.problemReporter().undocumentedEmptyBlock(this.blockStart, this.sourceEnd);
> 				}
> 			}
>-			if (isStringSwitch) {
>-				this.dispatchStringCopy  = new LocalVariableBinding(SecretStringVariableName, upperScope.getJavaLangString(), ClassFileConstants.AccDefault, false);
>-				upperScope.addLocalVariable(this.dispatchStringCopy);
>-				this.dispatchStringCopy.setConstant(Constant.NotAConstant);
>-				this.dispatchStringCopy.useFlag = LocalVariableBinding.USED;
>-			}
> 			// for enum switch, check if all constants are accounted for (if no default)
> 			if (isEnumSwitch && this.defaultCase == null
> 					&& upperScope.compilerOptions().getSeverity(CompilerOptions.IncompleteEnumSwitch) != ProblemSeverities.Ignore) {
>#P org.eclipse.jdt.core.tests.compiler
>Index: src/org/eclipse/jdt/core/tests/compiler/regression/SwitchTest.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/SwitchTest.java,v
>retrieving revision 1.29
>diff -u -r1.29 SwitchTest.java
>--- src/org/eclipse/jdt/core/tests/compiler/regression/SwitchTest.java	9 Aug 2011 14:32:44 -0000	1.29
>+++ src/org/eclipse/jdt/core/tests/compiler/regression/SwitchTest.java	29 Aug 2011 14:31:30 -0000
>@@ -30,6 +30,7 @@
> 
> static {
> //	TESTS_NUMBERS = new int[] { 22 };
>+//	TESTS_NAMES = new String[] { "testFor356002", "testFor356002_2", "testFor356002_3" };
> }
> public SwitchTest(String name) {
> 	super(name);
>@@ -2029,6 +2030,109 @@
> 										 "DONE");
> 	}
> }
>+public void testFor356002() {
>+	String errorMsg = 		
>+			"----------\n" + 
>+			"1. ERROR in X.java (at line 6)\n" + 
>+			"	switch (foo()) {\n" + 
>+			"	        ^^^^^\n" + 
>+			"Cannot switch on a value of type String for source level below 1.7. Only convertible int values or enum variables are permitted\n" + 
>+			"----------\n";
>+	
>+	String [] sourceFiles = 
>+		new String[] {
>+		"X.java",
>+		"public class X {\n" + 
>+		"	private static String foo() {\n" + 
>+		"		return \"\";\n" + 
>+		"	}\n" + 
>+		"	public static void main(String[] args) {\n" + 
>+		"		switch (foo()) {\n" + 
>+		"			default: {\n" + 
>+		"				int j = 0;\n" + 
>+		"				if (j <= 0)\n" + 
>+		"					System.out.println(\"DONE\");\n" +
>+		"			}\n" + 
>+		"			return;\n" + 
>+		"		}\n" + 
>+		"	}\n" + 
>+		"}",
>+	};
>+	if (this.complianceLevel < JDKLevelSupportingStringSwitch) {
>+		this.runNegativeTest(sourceFiles, errorMsg);
>+	} else {
>+		this.runConformTest(sourceFiles, "DONE");
>+	}
>+}
>+public void testFor356002_2() {
>+	String errorMsg = 		
>+			"----------\n" + 
>+			"1. ERROR in X.java (at line 3)\n" + 
>+			"	switch (\"\") {\n" + 
>+			"	        ^^\n" + 
>+			"Cannot switch on a value of type String for source level below 1.7. Only convertible int values or enum variables are permitted\n" + 
>+			"----------\n";
>+	
>+	String [] sourceFiles = 
>+		new String[] {
>+		"X.java",
>+		"public class X {\n" + 
>+		"	public static void main(String[] args) {\n" + 
>+		"		switch (\"\") {\n" + 
>+		"			default: {\n" + 
>+		"				int j = 0;\n" + 
>+		"				if (j <= 0)\n" + 
>+		"					System.out.println(\"DONE\");\n" +
>+		"			}\n" + 
>+		"			return;\n" + 
>+		"		}\n" + 
>+		"	}\n" + 
>+		"}",
>+	};
>+	if (this.complianceLevel < JDKLevelSupportingStringSwitch) {
>+		this.runNegativeTest(sourceFiles, errorMsg);
>+	} else {
>+		this.runConformTest(sourceFiles, "DONE");
>+	}
>+}
>+public void testFor356002_3() {
>+	String errorMsg =
>+			"----------\n" + 
>+			"1. ERROR in X.java (at line 7)\n" + 
>+			"	switch (foo()) {\n" + 
>+			"	        ^^^^^\n" + 
>+			"Cannot switch on a value of type String for source level below 1.7. Only convertible int values or enum variables are permitted\n" + 
>+			"----------\n";
>+	
>+	String [] sourceFiles =
>+		new String[] {
>+		"X.java",
>+		"public class X {\n" + 
>+		"	private static String foo() {\n" + 
>+		"		return null;\n" + 
>+		"	}\n" + 
>+		"	public static void main(String[] args) {\n" +
>+		"		try {\n" +
>+		"			switch (foo()) {\n" + 
>+		"				default: {\n" + 
>+		"					int j = 0;\n" + 
>+		"					if (j <= 0)\n" + 
>+		"						;\n" +
>+		"				}\n" + 
>+		"				return;\n" + 
>+		"			}\n" + 
>+		"		} catch(NullPointerException e) {\n" +
>+		"			System.out.println(\"DONE\");\n" +
>+		"		}\n" +
>+		"	}\n" + 
>+		"}",
>+	};
>+	if (this.complianceLevel < JDKLevelSupportingStringSwitch) {
>+		this.runNegativeTest(sourceFiles, errorMsg);
>+	} else {
>+		this.runConformTest(sourceFiles, "DONE");
>+	}
>+}
> public static Class testClass() {
> 	return SwitchTest.class;
> }

Actions: View | Diff

Attachments on bug 356002: 202328 | 202347