Attachment 191902 Details for Bug 339478 – Revised patch for rejecting ill formed programs

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

[patch] Revised patch for rejecting ill formed programs

patch.txt (text/plain), 18.99 KB, created by Srikanth Sankaran

on 2011-03-25 07:03:02 EDT

(hide)

Description:

Filename:

MIME Type:

Creator: Srikanth Sankaran

Created: 2011-03-25 07:03:02 EDT

Size: 18.99 KB

patch

obsolete

>### Eclipse Workspace Patch 1.0
>#P org.eclipse.jdt.core
>Index: compiler/org/eclipse/jdt/internal/compiler/ast/ParameterizedQualifiedTypeReference.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/ParameterizedQualifiedTypeReference.java,v
>retrieving revision 1.63.2.1
>diff -u -r1.63.2.1 ParameterizedQualifiedTypeReference.java
>--- compiler/org/eclipse/jdt/internal/compiler/ast/ParameterizedQualifiedTypeReference.java	25 Jan 2011 20:33:00 -0000	1.63.2.1
>+++ compiler/org/eclipse/jdt/internal/compiler/ast/ParameterizedQualifiedTypeReference.java	25 Mar 2011 10:58:24 -0000
>@@ -208,6 +208,7 @@
> 			    	((ClassScope) scope).superTypeReference = null;
> 			    }
> 				int argLength = args.length;
>+				boolean isDiamond = argLength == 0 && (i == (max -1)) && ((this.bits & ASTNode.IsDiamond) != 0);
> 				TypeBinding[] argTypes = new TypeBinding[argLength];
> 				boolean argHasError = false;
> 				ReferenceBinding currentOriginal = (ReferenceBinding)currentType.original();
>@@ -246,7 +247,7 @@
> 						this.resolvedType = scope.createArrayType(this.resolvedType, this.dimensions);
> 					}
> 					return this.resolvedType;
>-				} else if (argLength != typeVariables.length) { // check arity
>+				} else if (argLength != typeVariables.length && !isDiamond) { // check arity
> 					scope.problemReporter().incorrectArityForParameterizedType(this, currentType, argTypes, i);
> 					return null;
> 				}
>Index: compiler/org/eclipse/jdt/internal/compiler/ast/ParameterizedSingleTypeReference.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/ParameterizedSingleTypeReference.java,v
>retrieving revision 1.54.2.1
>diff -u -r1.54.2.1 ParameterizedSingleTypeReference.java
>--- compiler/org/eclipse/jdt/internal/compiler/ast/ParameterizedSingleTypeReference.java	25 Jan 2011 20:33:00 -0000	1.54.2.1
>+++ compiler/org/eclipse/jdt/internal/compiler/ast/ParameterizedSingleTypeReference.java	25 Mar 2011 10:58:24 -0000
>@@ -221,7 +221,7 @@
> 				return this.resolvedType = type;
> 			}
> 			// if missing generic type, and compliance >= 1.5, then will rebuild a parameterized binding
>-		} else if (argLength != typeVariables.length) { // check arity
>+		} else if (argLength != typeVariables.length && (this.bits & ASTNode.IsDiamond) == 0) { // check arity, IsDiamond never set for 1.6-
> 			scope.problemReporter().incorrectArityForParameterizedType(this, currentType, argTypes);
> 			return null;
> 		} else if (!currentType.isStatic()) {
>Index: compiler/org/eclipse/jdt/internal/compiler/parser/Parser.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/parser/Parser.java,v
>retrieving revision 1.422.2.18
>diff -u -r1.422.2.18 Parser.java
>--- compiler/org/eclipse/jdt/internal/compiler/parser/Parser.java	19 Mar 2011 16:00:05 -0000	1.422.2.18
>+++ compiler/org/eclipse/jdt/internal/compiler/parser/Parser.java	25 Mar 2011 10:58:33 -0000
>@@ -1177,6 +1177,9 @@
> 				length);
> 		}
> 		alloc.type = getTypeReference(0);
>+		if (this.options.sourceLevel >= ClassFileConstants.JDK1_7) {
>+			checkForDiamond(alloc.type);
>+		}
> 
> 		//the default constructor with the correct number of argument
> 		//will be created and added by the TC (see createsInternalConstructorWithBinding)
>@@ -1197,6 +1200,20 @@
> 		this.astLengthPtr--;
> 	}
> }
>+private void checkForDiamond(TypeReference allocType) {
>+	if (allocType instanceof ParameterizedSingleTypeReference) {
>+		ParameterizedSingleTypeReference type = (ParameterizedSingleTypeReference) allocType;
>+		if (type.typeArguments == TypeReference.NO_TYPE_ARGUMENTS) {
>+			type.bits |= ASTNode.IsDiamond;
>+		}
>+	} 
>+	else if (allocType instanceof ParameterizedQualifiedTypeReference) {
>+		ParameterizedQualifiedTypeReference type = (ParameterizedQualifiedTypeReference) allocType;
>+		if (type.typeArguments[type.typeArguments.length - 1] == TypeReference.NO_TYPE_ARGUMENTS) { // Don't care for X<>.Y<> and X<>.Y<String>
>+			type.bits |= ASTNode.IsDiamond;
>+		}
>+	}
>+}
> protected ParameterizedQualifiedTypeReference computeQualifiedGenericsFromRightSide(TypeReference rightSide, int dim) {
> 	int nameSize = this.identifierLengthStack[this.identifierLengthPtr];
> 	int tokensSize = nameSize;
>@@ -2440,7 +2457,9 @@
> 				length);
> 		}
> 		alloc.type = getTypeReference(0);
>-
>+		if (this.options.sourceLevel >= ClassFileConstants.JDK1_7) {
>+			checkForDiamond(alloc.type);
>+		}
> 		length = this.genericsLengthStack[this.genericsLengthPtr--];
> 		this.genericsPtr -= length;
> 		System.arraycopy(this.genericsStack, this.genericsPtr + 1, alloc.typeArguments = new TypeReference[length], 0, length);
>@@ -2504,6 +2523,9 @@
> 				length);
> 		}
> 		alloc.type = getTypeReference(0);
>+		if (this.options.sourceLevel >= ClassFileConstants.JDK1_7) {
>+			checkForDiamond(alloc.type);
>+		}		
> 
> 		length = this.genericsLengthStack[this.genericsLengthPtr--];
> 		this.genericsPtr -= length;
>@@ -3956,6 +3978,7 @@
> 	// zero type arguments == <>
> 	pushOnGenericsLengthStack(-1);
> 	concatGenericsLists();
>+	this.intPtr--;	// pop the null dimension pushed in by consumeReferenceType, as we have no type between <>, getTypeReference won't kick in 
> }
> protected void consumeImportDeclaration() {
> 	// SingleTypeImportDeclaration ::= SingleTypeImportDeclarationName ';'
>@@ -8900,8 +8923,7 @@
> 	if (identifierLength == 1 && numberOfIdentifiers == 1) {
> 		int currentTypeArgumentsLength = this.genericsLengthStack[this.genericsLengthPtr--];
> 		TypeReference[] typeArguments = null;
>-		boolean isDiamond = currentTypeArgumentsLength < 0;
>-		if (isDiamond) {
>+		if (currentTypeArgumentsLength < 0) {
> 			typeArguments = TypeReference.NO_TYPE_ARGUMENTS;
> 		} else {
> 			typeArguments = new TypeReference[currentTypeArgumentsLength];
>@@ -8912,9 +8934,11 @@
> 		if (dim != 0) {
> 			parameterizedSingleTypeReference.sourceEnd = this.endStatementPosition;
> 		}
>-		if (isDiamond) {
>-			parameterizedSingleTypeReference.bits |= ASTNode.IsDiamond;
>-		}
>+		/* We used to eagerly mark the PSTR as constituting diamond usage if we encountered <>, but that is too eager and
>+		   complicates error handling by making it hard to distinguish legitimate use cases from ill formed ones. We are
>+		   more discriminating now and tag a type as being diamond only where <> can legally occur. 
>+		   See https://bugs.eclipse.org/bugs/show_bug.cgi?id=339478#c11
>+		*/
> 		return parameterizedSingleTypeReference;
> 	} else {
> 		TypeReference[][] typeArguments = new TypeReference[numberOfIdentifiers][];
>@@ -8922,7 +8946,6 @@
> 		long[] positions = new long[numberOfIdentifiers];
> 		int index = numberOfIdentifiers;
> 		int currentIdentifiersLength = identifierLength;
>-		boolean isDiamond = false;
> 		while (index > 0) {
> 			int currentTypeArgumentsLength = this.genericsLengthStack[this.genericsLengthPtr--];
> 			if (currentTypeArgumentsLength > 0) {
>@@ -8931,7 +8954,6 @@
> 			} else if (currentTypeArgumentsLength < 0) {
> 				// diamond case for qualified type reference (java.util.ArrayList<>)
> 				typeArguments[index - 1] = TypeReference.NO_TYPE_ARGUMENTS;
>-				isDiamond = true;
> 			}
> 			switch(currentIdentifiersLength) {
> 				case 1 :
>@@ -8954,9 +8976,11 @@
> 		if (dim != 0) {
> 			parameterizedQualifiedTypeReference.sourceEnd = this.endStatementPosition;
> 		}
>-		if (isDiamond) {
>-			parameterizedQualifiedTypeReference.bits |= ASTNode.IsDiamond;
>-		}
>+		/* We used to eagerly mark the PQTR as constituting diamond usage if we encountered <>, but that is too eager and
>+		   complicates error handling by making it hard to distinguish legitimate use cases from ill formed ones. We are
>+		   more discriminating now and tag a type as being diamond only where <> can legally occur. 
>+		   See https://bugs.eclipse.org/bugs/show_bug.cgi?id=339478#c11
>+		*/
> 		return parameterizedQualifiedTypeReference;
> 	}
> }
>Index: dom/org/eclipse/jdt/core/dom/ASTConverter.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.jdt.core/dom/org/eclipse/jdt/core/dom/ASTConverter.java,v
>retrieving revision 1.273.2.9
>diff -u -r1.273.2.9 ASTConverter.java
>--- dom/org/eclipse/jdt/core/dom/ASTConverter.java	18 Mar 2011 19:36:32 -0000	1.273.2.9
>+++ dom/org/eclipse/jdt/core/dom/ASTConverter.java	25 Mar 2011 10:58:37 -0000
>@@ -3349,7 +3349,22 @@
> 		if (this.resolveBindings) {
> 			this.recordNodes(type, typeReference);
> 		}
>-		if ((typeReference.bits & org.eclipse.jdt.internal.compiler.ast.ASTNode.IsDiamond) != 0) {
>+		boolean sawDiamond = false;
>+		if (typeReference instanceof ParameterizedSingleTypeReference) {
>+			ParameterizedSingleTypeReference pstr = (ParameterizedSingleTypeReference) typeReference;
>+			if (pstr.typeArguments == TypeReference.NO_TYPE_ARGUMENTS) {
>+				sawDiamond = true;
>+			}
>+		} else if (typeReference instanceof ParameterizedQualifiedTypeReference) {
>+			ParameterizedQualifiedTypeReference pqtr = (ParameterizedQualifiedTypeReference) typeReference;
>+			for (int i = 0, len = pqtr.typeArguments.length; i < len; i++) {
>+				if (pqtr.typeArguments[i] == TypeReference.NO_TYPE_ARGUMENTS) {
>+					sawDiamond = true;
>+					break;
>+				}
>+			}
>+		} 
>+		if (sawDiamond) {
> 			switch(this.ast.apiLevel) {
> 				case AST.JLS2_INTERNAL :
> 				case AST.JLS3 :
>Index: model/org/eclipse/jdt/core/Signature.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.jdt.core/model/org/eclipse/jdt/core/Signature.java,v
>retrieving revision 1.99
>diff -u -r1.99 Signature.java
>--- model/org/eclipse/jdt/core/Signature.java	26 Nov 2010 07:31:43 -0000	1.99
>+++ model/org/eclipse/jdt/core/Signature.java	25 Mar 2011 10:58:38 -0000
>@@ -1,10 +1,14 @@
> /*******************************************************************************
>- * Copyright (c) 2000, 2010 IBM Corporation and others.
>+ * Copyright (c) 2000, 2011 IBM Corporation and others.
>  * All rights reserved. This program and the accompanying materials
>  * are made available under the terms of the Eclipse Public License v1.0
>  * which accompanies this distribution, and is available at
>  * http://www.eclipse.org/legal/epl-v10.html
>  *
>+ * This is an implementation of an early-draft specification developed under the Java
>+ * Community Process (JCP) and is made available for testing and evaluation purposes
>+ * only. The code is not compatible with any specification of the JCP.
>+ *
>  * Contributors:
>  *     IBM Corporation - initial API and implementation
>  *     IBM Corporation - added J2SE 1.5 support
>@@ -851,12 +855,17 @@
> 		checkPos = checkNextChar(typeName, '<', pos, length, true);
> 		if (checkPos > 0) {
> 			buffer.append(C_GENERIC_START);
>-		    pos = encodeTypeSignature(typeName, checkPos, isResolved, length, buffer);
>-		    while ((checkPos = checkNextChar(typeName, ',', pos, length, true)) > 0) {
>-			    pos = encodeTypeSignature(typeName, checkPos, isResolved, length, buffer);
>-		    }
>-		    pos = checkNextChar(typeName, '>', pos, length, false);
>-			buffer.append(C_GENERIC_END);
>+			// Stop gap fix for <>.
>+			if ((pos = checkNextChar(typeName, '>', checkPos, length, true)) > 0) {
>+				buffer.append(C_GENERIC_END);
>+			} else {
>+				pos = encodeTypeSignature(typeName, checkPos, isResolved, length, buffer);
>+				while ((checkPos = checkNextChar(typeName, ',', pos, length, true)) > 0) {
>+					pos = encodeTypeSignature(typeName, checkPos, isResolved, length, buffer);
>+				}
>+				pos = checkNextChar(typeName, '>', pos, length, false);
>+				buffer.append(C_GENERIC_END);
>+			}
> 		}
> 		checkPos = checkNextChar(typeName, '.', pos, length, true);
> 		if (checkPos > 0) {
>#P org.eclipse.jdt.core.tests.compiler
>Index: src/org/eclipse/jdt/core/tests/compiler/regression/GenericsRegressionTest.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/GenericsRegressionTest.java,v
>retrieving revision 1.5.2.4
>diff -u -r1.5.2.4 GenericsRegressionTest.java
>--- src/org/eclipse/jdt/core/tests/compiler/regression/GenericsRegressionTest.java	15 Mar 2011 12:06:25 -0000	1.5.2.4
>+++ src/org/eclipse/jdt/core/tests/compiler/regression/GenericsRegressionTest.java	25 Mar 2011 10:58:43 -0000
>@@ -5,6 +5,10 @@
>  * which accompanies this distribution, and is available at
>  * http://www.eclipse.org/legal/epl-v10.html
>  *
>+ * This is an implementation of an early-draft specification developed under the Java
>+ * Community Process (JCP) and is made available for testing and evaluation purposes
>+ * only. The code is not compatible with any specification of the JCP.
>+ * 
>  * Contributors:
>  *     IBM Corporation - initial API and implementation
>  *     Stephan Herrmann <stephan@cs.tu-berlin.de> - Contribution for bug 282152 - [1.5][compiler] Generics code rejected by Eclipse but accepted by javac
>@@ -1740,4 +1744,191 @@
> 		compilerOptions15,
> 		null);
> }
>+// https://bugs.eclipse.org/bugs/show_bug.cgi?id=339478
>+// To verify that diamond construct is not allowed in source level 1.6 or below
>+public void test339478a() {
>+	if (this.complianceLevel >= ClassFileConstants.JDK1_7)
>+		return;
>+	this.runNegativeTest(
>+		new String[] {
>+			"X.java",
>+			"public class X<T> {\n" + 
>+			"	public static void main(String[] args) {\n" + 
>+			"		X<String> x = new X<>();\n" + 
>+			"		x.testFunction(\"SUCCESS\");\n" + 
>+			"	}\n" +
>+			"	public void testFunction(T param){\n" +
>+			"		System.out.println(param);\n" +
>+			"	}\n" + 
>+			"}",
>+		},
>+		"----------\n" + 
>+		"1. ERROR in X.java (at line 3)\n" + 
>+		"	X<String> x = new X<>();\n" + 
>+		"	                  ^\n" + 
>+		"Incorrect number of arguments for type X<T>; it cannot be parameterized with arguments <>\n" + 
>+		"----------\n");
>+}
>+// https://bugs.eclipse.org/bugs/show_bug.cgi?id=339478
>+// To verify that diamond construct is not allowed in source level 1.6 or below
>+public void test339478b() {
>+	this.runNegativeTest(
>+		new String[] {
>+			"X.java",
>+			"public class X<T> {\n" +
>+			"	public static void main(String[] args) {\n" + 
>+			"		X<> x1 = null;\n" +
>+			"	}\n" +
>+			"}",
>+		},
>+		"----------\n" + 
>+		"1. ERROR in X.java (at line 3)\n" + 
>+		"	X<> x1 = null;\n" + 
>+		"	^\n" + 
>+		"Incorrect number of arguments for type X<T>; it cannot be parameterized with arguments <>\n" + 
>+		"----------\n");
>+}
>+public void test339478c() {
>+	this.runNegativeTest(
>+		new String[] {
>+			"X.java",
>+			"import java.util.Map;\n" +
>+			"public class X implements Map<> {\n" +
>+			"    static Map<> foo (Map<> x) { \n" +
>+			"        return null;\n" +
>+			"    }\n" +
>+			"}\n",
>+		},
>+		"----------\n" + 
>+		"1. ERROR in X.java (at line 2)\n" + 
>+		"	public class X implements Map<> {\n" + 
>+		"	                          ^^^\n" + 
>+		"Incorrect number of arguments for type Map<K,V>; it cannot be parameterized with arguments <>\n" + 
>+		"----------\n" + 
>+		"2. ERROR in X.java (at line 3)\n" + 
>+		"	static Map<> foo (Map<> x) { \n" + 
>+		"	       ^^^\n" + 
>+		"Incorrect number of arguments for type Map<K,V>; it cannot be parameterized with arguments <>\n" + 
>+		"----------\n" + 
>+		"3. ERROR in X.java (at line 3)\n" + 
>+		"	static Map<> foo (Map<> x) { \n" + 
>+		"	                  ^^^\n" + 
>+		"Incorrect number of arguments for type Map<K,V>; it cannot be parameterized with arguments <>\n" + 
>+		"----------\n");
>+}
>+public void test339478d() {
>+	this.runNegativeTest(
>+		new String[] {
>+			"X.java",
>+			"import java.util.Map;\n" +
>+			"public class X  {\n" +
>+			"    static Map<> foo () { \n" +
>+			"        return null;\n" +
>+			"    }\n" +
>+			"}\n",
>+		},
>+		"----------\n" + 
>+		"1. ERROR in X.java (at line 3)\n" + 
>+		"	static Map<> foo () { \n" + 
>+		"	       ^^^\n" + 
>+		"Incorrect number of arguments for type Map<K,V>; it cannot be parameterized with arguments <>\n" + 
>+		"----------\n");
>+}
>+public void test339478e() {
>+	this.runNegativeTest(
>+		new String[] {
>+			"X.java",
>+			"public class X<T>  {\n" +
>+			"    class Y<K> {\n" +
>+			"    }\n" +
>+			"    public static void main(String [] args) {\n" +
>+			"        X<String>.Y<> [] y = null; \n" +
>+			"    }\n" +
>+			"}\n",
>+		},
>+		"----------\n" + 
>+		"1. ERROR in X.java (at line 5)\n" + 
>+		"	X<String>.Y<> [] y = null; \n" + 
>+		"	^^^^^^^^^^^\n" + 
>+		"Incorrect number of arguments for type X<String>.Y; it cannot be parameterized with arguments <>\n" + 
>+		"----------\n");
>+}
>+public void test339478f() {
>+	this.runNegativeTest(
>+		new String[] {
>+			"X.java",
>+			"public class X<T>  {\n" +
>+			"    class Y<K> {\n" +
>+			"    }\n" +
>+			"    public static void main(String [] args) {\n" +
>+			"        X<String>.Y<>  y = null; \n" +
>+			"    }\n" +
>+			"}\n",
>+		},
>+		"----------\n" + 
>+		"1. ERROR in X.java (at line 5)\n" + 
>+		"	X<String>.Y<>  y = null; \n" + 
>+		"	^^^^^^^^^^^\n" + 
>+		"Incorrect number of arguments for type X<String>.Y; it cannot be parameterized with arguments <>\n" + 
>+		"----------\n");
>+}
>+public void test339478g() {
>+	this.runNegativeTest(
>+		new String[] {
>+			"X.java",
>+			"public class X<T>  {\n" +
>+			"    public void foo(Object x) {\n" +
>+			"        if (x instanceof X<>) {    \n" +
>+			"        }\n" +
>+			"    }\n" +
>+			"}\n",
>+		},
>+		"----------\n" + 
>+		"1. ERROR in X.java (at line 3)\n" + 
>+		"	if (x instanceof X<>) {    \n" + 
>+		"	                 ^\n" + 
>+		"Incorrect number of arguments for type X<T>; it cannot be parameterized with arguments <>\n" + 
>+		"----------\n");
>+}
>+public void test339478h() {
>+	this.runNegativeTest(
>+		new String[] {
>+			"X.java",
>+			"public class X<T>  {\n" +
>+			"    public void foo(Object x) throws X.Y<>.LException {\n" +
>+			"    }\n" +
>+			"    static class Y<T> {\n" +
>+			"    static class LException extends Throwable {}\n" +
>+			"    }\n" +
>+			"}\n",
>+		},
>+		"----------\n" + 
>+		"1. ERROR in X.java (at line 2)\n" + 
>+		"	public void foo(Object x) throws X.Y<>.LException {\n" + 
>+		"	                                 ^^^\n" + 
>+		"Incorrect number of arguments for type X.Y; it cannot be parameterized with arguments <>\n" + 
>+		"----------\n" + 
>+		"2. WARNING in X.java (at line 5)\n" + 
>+		"	static class LException extends Throwable {}\n" + 
>+		"	             ^^^^^^^^^^\n" + 
>+		"The serializable class LException does not declare a static final serialVersionUID field of type long\n" + 
>+		"----------\n");
>+}
>+public void test339478i() {
>+	this.runNegativeTest(
>+		new String[] {
>+			"X.java",
>+			"public class X<T>  {\n" +
>+			"    public void foo () {\n" +
>+			"        Object o = new X<> [10];\n" +
>+			"    }\n" +
>+			"}\n",
>+		},
>+		"----------\n" + 
>+		"1. ERROR in X.java (at line 3)\n" + 
>+		"	Object o = new X<> [10];\n" + 
>+		"	               ^\n" + 
>+		"Incorrect number of arguments for type X<T>; it cannot be parameterized with arguments <>\n" + 
>+		"----------\n");
>+}
> }
>\ No newline at end of file

Actions: View | Diff

Attachments on bug 339478: 190845 | 191332 | 191458 | 191707 | 191785 | 191891 | 191902 | 191946 | 191947 | 191984 | 192058 | 193887 | 193901 | 193918 | 193951 | 193962 | 193963