Attachment 151683 Details for Bug 293615 – Potential patch

[patch] Potential patch

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

on 2009-11-09 06:18:32 EST

(hide)

Description:

Filename:

MIME Type:

Creator: Srikanth Sankaran

Created: 2009-11-09 06:18:32 EST

Size: 8.02 KB

patch

obsolete

>### Eclipse Workspace Patch 1.0
>#P org.eclipse.jdt.core
>Index: compiler/org/eclipse/jdt/internal/compiler/lookup/MethodVerifier15.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/MethodVerifier15.java,v
>retrieving revision 1.106
>diff -u -r1.106 MethodVerifier15.java
>--- compiler/org/eclipse/jdt/internal/compiler/lookup/MethodVerifier15.java	15 Oct 2009 14:49:48 -0000	1.106
>+++ compiler/org/eclipse/jdt/internal/compiler/lookup/MethodVerifier15.java	9 Nov 2009 10:48:53 -0000
>@@ -138,11 +138,25 @@
> 		if (!isAcceptableReturnTypeOverride(currentMethod, inheritedMethod))
> 			problemReporter(currentMethod).unsafeReturnTypeOverride(currentMethod, originalInherited, this.type);
> 
>-	if (this.type.addSyntheticBridgeMethod(originalInherited, currentMethod.original()) != null) {
>+	MethodBinding bridge = this.type.addSyntheticBridgeMethod(originalInherited, currentMethod.original());
>+	if (bridge != null) {
> 		for (int i = 0, l = allInheritedMethods == null ? 0 : allInheritedMethods.length; i < l; i++) {
> 			if (allInheritedMethods[i] != null && detectInheritedNameClash(originalInherited, allInheritedMethods[i].original()))
> 				return;
> 		}
>+		// See if the new bridge clashes with any of the user methods of the class. For this check
>+		// we should check for "method descriptor clash" and not just "method signature clash". Really
>+		// what we are checking is whether there is a contention for the method dispatch table slot.
>+		// See https://bugs.eclipse.org/bugs/show_bug.cgi?id=293615.
>+		MethodBinding[] current = (MethodBinding[]) this.currentMethods.get(bridge.selector);
>+		for (int i = current.length - 1; i >= 0; --i) {
>+			final MethodBinding thisMethod = current[i];
>+			if (thisMethod.areParameterErasuresEqual(bridge) && thisMethod.returnType.erasure() == bridge.returnType.erasure()) {
>+				// use inherited method for problem reporting.
>+				problemReporter(thisMethod).methodNameClash(thisMethod, inheritedMethod.declaringClass.isRawType() ? inheritedMethod : inheritedMethod.original());
>+				return;	
>+			}
>+		}
> 	}
> }
> void checkForNameClash(MethodBinding currentMethod, MethodBinding inheritedMethod) {
>@@ -180,7 +194,7 @@
> 
> 	if (currentMethod.declaringClass.isInterface() || inheritedMethod.isStatic()) return;
> 
>-	if (!detectNameClash(currentMethod, inheritedMethod)) { // check up the hierarchy for skipped inherited methods
>+	if (!detectNameClash(currentMethod, inheritedMethod, false)) { // check up the hierarchy for skipped inherited methods
> 		TypeBinding[] currentParams = currentMethod.parameters;
> 		TypeBinding[] inheritedParams = inheritedMethod.parameters;
> 		int length = currentParams.length;
>@@ -204,7 +218,7 @@
> 			MethodBinding[] methods = superType.getMethods(currentMethod.selector);
> 			for (int m = 0, n = methods.length; m < n; m++) {
> 				MethodBinding substitute = computeSubstituteMethod(methods[m], currentMethod);
>-				if (substitute != null && !isSubstituteParameterSubsignature(currentMethod, substitute) && detectNameClash(currentMethod, substitute))
>+				if (substitute != null && !isSubstituteParameterSubsignature(currentMethod, substitute) && detectNameClash(currentMethod, substitute, true))
> 					return;
> 			}
> 			if ((itsInterfaces = superType.superInterfaces()) != Binding.NO_SUPERINTERFACES) {
>@@ -232,7 +246,7 @@
> 				MethodBinding[] methods = superType.getMethods(currentMethod.selector);
> 				for (int m = 0, n = methods.length; m < n; m++){
> 					MethodBinding substitute = computeSubstituteMethod(methods[m], currentMethod);
>-					if (substitute != null && !isSubstituteParameterSubsignature(currentMethod, substitute) && detectNameClash(currentMethod, substitute))
>+					if (substitute != null && !isSubstituteParameterSubsignature(currentMethod, substitute) && detectNameClash(currentMethod, substitute, true))
> 						return;
> 				}
> 				if ((itsInterfaces = superType.superInterfaces()) != Binding.NO_SUPERINTERFACES) {
>@@ -555,11 +569,31 @@
> 	problemReporter().inheritedMethodsHaveNameClash(this.type, inherited, otherInherited);
> 	return true;
> }
>-boolean detectNameClash(MethodBinding current, MethodBinding inherited) {
>-	MethodBinding original = inherited.original(); // can be the same as inherited
>+boolean detectNameClash(MethodBinding current, MethodBinding inherited, boolean treatAsSynthetic) {
>+	MethodBinding methodToCheck = inherited;
>+	if (treatAsSynthetic) {
>+		// descriptors matter here, not just signature.
>+		if (current.returnType.erasure() != methodToCheck.returnType.erasure())
>+			return false;
>+	} else {
>+		// For a user method, see if current class overrides the inherited method. If it does,
>+		// then any grievance we may have ought to be against the current class's method and
>+		// NOT against any super implementations. https://bugs.eclipse.org/bugs/show_bug.cgi?id=293615
>+		MethodBinding[] currentNamesakes = (MethodBinding[]) this.currentMethods.get(inherited.selector);
>+		if (currentNamesakes.length > 1) { // we know it ought to at least one and that current is NOT the override
>+			for (int i = 0, length = currentNamesakes.length; i < length; i++) {
>+				MethodBinding currentMethod = currentNamesakes[i];
>+				if (currentMethod != current && doesMethodOverride(currentMethod, inherited)) {
>+					methodToCheck = currentMethod;
>+					break;
>+				}
>+			}
>+		}
>+	}
>+	MethodBinding original = methodToCheck.original(); // can be the same as inherited
> 	if (!current.areParameterErasuresEqual(original))
> 		return false;
>-
>+	original = inherited.original();  // For error reporting use, inherited.original()
> 	problemReporter(current).methodNameClash(current, inherited.declaringClass.isRawType() ? inherited : original);
> 	return true;
> }
>#P org.eclipse.jdt.core.tests.compiler
>Index: src/org/eclipse/jdt/core/tests/compiler/regression/MethodVerifyTest.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/MethodVerifyTest.java,v
>retrieving revision 1.195
>diff -u -r1.195 MethodVerifyTest.java
>--- src/org/eclipse/jdt/core/tests/compiler/regression/MethodVerifyTest.java	14 Oct 2009 17:23:50 -0000	1.195
>+++ src/org/eclipse/jdt/core/tests/compiler/regression/MethodVerifyTest.java	9 Nov 2009 10:49:36 -0000
>@@ -10684,4 +10684,51 @@
> 		""
> 	);
> }
>+// see https://bugs.eclipse.org/bugs/show_bug.cgi?id=293615 (bad name clash error)
>+// No user vs user clash or user vs synthetic clash in this test
>+public void test204() {
>+	this.runConformTest(
>+		new String[] {
>+			"OverrideBug.java",
>+			"import java.util.List;\n" +
>+			"interface Map<K, V> {\n" + 
>+			"	public V put(K key, V value);\n" +
>+			"}\n" +
>+			"public class OverrideBug<K, V> implements Map<K, List<V>> {\n" +
>+			"public List<V> put(final K arg0, final List<V> arg1) {\n" +
>+			"    return null;\n" +
>+			"}\n" +
>+			"public List<V> put(final K arg0, final V arg1) {\n" +
>+			"    return null;\n" +
>+			"}\n" +
>+			"}"
>+		},
>+		"");
>+}
>+// see https://bugs.eclipse.org/bugs/show_bug.cgi?id=293615 (bad name clash error)
>+// verify that we report user vs bridge clash properly.
>+public void test204a() {
>+	this.runNegativeTest(
>+		new String[] {
>+			"OverrideBug.java",
>+			"import java.util.List;\n" +
>+			"interface Map<K, V> {\n" + 
>+			"	public V put(K key, V value);\n" +
>+			"}\n" +
>+			"public class OverrideBug<K, V> implements Map<K, List<V>> {\n" +
>+			"public List<V> put(final K arg0, final List<V> arg1) {\n" +
>+			"    return null;\n" +
>+			"}\n" +
>+			"public V put(final K arg0, final V arg1) {\n" +
>+			"    return null;\n" +
>+			"}\n" +
>+			"}"
>+		},
>+		"----------\n" + 
>+		"1. ERROR in OverrideBug.java (at line 9)\n" + 
>+		"	public V put(final K arg0, final V arg1) {\n" + 
>+		"	         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n" + 
>+		"Name clash: The method put(K, V) of type OverrideBug<K,V> has the same erasure as put(K, V) of type Map<K,V> but does not override it\n" + 
>+		"----------\n");
>+}
> }

Actions: View | Diff

Attachments on bug 293615: 151683 | 151916