Attachment 159149 Details for Bug 302358 – Patch under consideration.

[patch] Patch under consideration.

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

on 2010-02-16 02:59:20 EST

(hide)

Description:

Filename:

MIME Type:

Creator: Srikanth Sankaran

Created: 2010-02-16 02:59:20 EST

Size: 15.26 KB

patch

obsolete

>### Eclipse Workspace Patch 1.0
>#P org.eclipse.jdt.core
>Index: compiler/org/eclipse/jdt/internal/compiler/lookup/MethodVerifier.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/MethodVerifier.java,v
>retrieving revision 1.110
>diff -u -r1.110 MethodVerifier.java
>--- compiler/org/eclipse/jdt/internal/compiler/lookup/MethodVerifier.java	2 Dec 2009 08:33:59 -0000	1.110
>+++ compiler/org/eclipse/jdt/internal/compiler/lookup/MethodVerifier.java	16 Feb 2010 07:57:40 -0000
>@@ -1,5 +1,5 @@
> /*******************************************************************************
>- * Copyright (c) 2000, 2009 IBM Corporation and others.
>+ * Copyright (c) 2000, 2010 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
>@@ -549,9 +549,9 @@
> 	- check that each exception type is Throwable or a subclass of it
> */
> void computeInheritedMethods(ReferenceBinding superclass, ReferenceBinding[] superInterfaces) {
>-	// only want to remember inheritedMethods that can have an impact on the current type
>-	// if an inheritedMethod has been 'replaced' by a supertype's method then skip it
>-
>+	// only want to remember inheritedMethods that can have an impact on the current type or a subtype.
>+	// if an inheritedMethod has been 'replaced' by a supertype's method then skip it, however
>+    // see usage of canOverridingMethodDifferInErasure below.
> 	this.inheritedMethods = new HashtableOfObject(51); // maps method selectors to an array of methods... must search to match paramaters & return type
> 	ReferenceBinding[] interfacesToVisit = null;
> 	int nextPosition = 0;
>@@ -563,30 +563,24 @@
> 
> 	ReferenceBinding superType = superclass;
> 	HashtableOfObject nonVisibleDefaultMethods = new HashtableOfObject(3); // maps method selectors to an array of methods
>-	boolean allSuperclassesAreAbstract = true;
> 
> 	while (superType != null && superType.isValidBinding()) {
>-	    if (allSuperclassesAreAbstract) {
>-		    if (superType.isAbstract()) {
>-				// only need to include superinterfaces if immediate superclasses are abstract
>-				if ((itsInterfaces = superType.superInterfaces()) != Binding.NO_SUPERINTERFACES) {
>-					if (interfacesToVisit == null) {
>-						interfacesToVisit = itsInterfaces;
>-						nextPosition = interfacesToVisit.length;
>-					} else {
>-						int itsLength = itsInterfaces.length;
>-						if (nextPosition + itsLength >= interfacesToVisit.length)
>-							System.arraycopy(interfacesToVisit, 0, interfacesToVisit = new ReferenceBinding[nextPosition + itsLength + 5], 0, nextPosition);
>-						nextInterface : for (int a = 0; a < itsLength; a++) {
>-							ReferenceBinding next = itsInterfaces[a];
>-							for (int b = 0; b < nextPosition; b++)
>-								if (next == interfacesToVisit[b]) continue nextInterface;
>-							interfacesToVisit[nextPosition++] = next;
>-						}
>-					}
>-				}
>+		// We used to only include superinterfaces if immediate superclasses are abstract
>+		// but that is problematic. See https://bugs.eclipse.org/bugs/show_bug.cgi?id=302358
>+		if ((itsInterfaces = superType.superInterfaces()) != Binding.NO_SUPERINTERFACES) {
>+			if (interfacesToVisit == null) {
>+				interfacesToVisit = itsInterfaces;
>+				nextPosition = interfacesToVisit.length;
> 			} else {
>-			    allSuperclassesAreAbstract = false;
>+				int itsLength = itsInterfaces.length;
>+				if (nextPosition + itsLength >= interfacesToVisit.length)
>+					System.arraycopy(interfacesToVisit, 0, interfacesToVisit = new ReferenceBinding[nextPosition + itsLength + 5], 0, nextPosition);
>+				nextInterface : for (int a = 0; a < itsLength; a++) {
>+					ReferenceBinding next = itsInterfaces[a];
>+					for (int b = 0; b < nextPosition; b++)
>+						if (next == interfacesToVisit[b]) continue nextInterface;
>+					interfacesToVisit[nextPosition++] = next;
>+				}
> 			}
> 		}
> 
>@@ -599,7 +593,9 @@
> 			if (existingMethods != null) {
> 				existing : for (int i = 0, length = existingMethods.length; i < length; i++) {
> 					MethodBinding existingMethod = existingMethods[i];
>-					if (existingMethod.declaringClass != inheritedMethod.declaringClass && areMethodsCompatible(existingMethod, inheritedMethod)) {
>+					// https://bugs.eclipse.org/bugs/show_bug.cgi?id=302358, skip inherited method only if any overriding version
>+					// in a subclass is guaranteed to have the same erasure as an existing method.
>+					if (existingMethod.declaringClass != inheritedMethod.declaringClass && areMethodsCompatible(existingMethod, inheritedMethod) && !canOverridingMethodDifferInErasure(existingMethod, inheritedMethod)) {
> 						if (inheritedMethod.isDefault()) {
> 							if (inheritedMethod.isAbstract()) {
> 								checkPackagePrivateAbstractMethod(inheritedMethod);
>@@ -681,8 +677,10 @@
> 				} else {
> 					int length = existingMethods.length;
> 					// look to see if any of the existingMethods implement this inheritedMethod
>+					// https://bugs.eclipse.org/bugs/show_bug.cgi?id=302358, skip inherited method only if any overriding version
>+					// in a subclass is guaranteed to have the same erasure as an existing method.
> 					for (int e = 0; e < length; e++)
>-						if (isInterfaceMethodImplemented(inheritedMethod, existingMethods[e], superType))
>+						if (isInterfaceMethodImplemented(inheritedMethod, existingMethods[e], superType) && !canOverridingMethodDifferInErasure(existingMethods[e], inheritedMethod))
> 							continue nextMethod; // skip interface method with the same signature if visible to its declaringClass
> 					System.arraycopy(existingMethods, 0, existingMethods = new MethodBinding[length + 1], 0, length);
> 					existingMethods[length] = inheritedMethod;
>@@ -692,7 +690,11 @@
> 		}
> 	}
> }
>-
>+// Given `overridingMethod' which overrides `inheritedMethod' answer whether some subclass method that
>+// differs in erasure from overridingMethod could override `inheritedMethod'
>+protected boolean canOverridingMethodDifferInErasure(MethodBinding overridingMethod, MethodBinding inheritedMethod) {
>+	return false;   // the case for <= 1.4  (cannot differ)
>+}
> void computeMethods() {
> 	MethodBinding[] methods = this.type.methods();
> 	int size = methods.length;
>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.108
>diff -u -r1.108 MethodVerifier15.java
>--- compiler/org/eclipse/jdt/internal/compiler/lookup/MethodVerifier15.java	2 Dec 2009 08:33:59 -0000	1.108
>+++ compiler/org/eclipse/jdt/internal/compiler/lookup/MethodVerifier15.java	16 Feb 2010 07:57:41 -0000
>@@ -1,5 +1,5 @@
> /*******************************************************************************
>- * Copyright (c) 2000, 2009 IBM Corporation and others.
>+ * Copyright (c) 2000, 2010 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
>@@ -96,6 +96,15 @@
> 	//		return ((UnresolvedReferenceBinding) two).resolvedType == one;
> 	return false; // all other type bindings are identical
> }
>+// Given `overridingMethod' which overrides `inheritedMethod' answer whether some subclass method that
>+// differs in erasure from overridingMethod could override `inheritedMethod'
>+protected boolean canOverridingMethodDifferInErasure(MethodBinding overridingMethod, MethodBinding inheritedMethod) {
>+	if (overridingMethod.areParameterErasuresEqual(inheritedMethod))
>+		return false;  // no further change in signature is possible due to parameterization.
>+	if (overridingMethod.declaringClass.isRawType())
>+		return false;  // no parameterization is happening anyways.
>+	return true;
>+}
> boolean canSkipInheritedMethods() {
> 	if (this.type.superclass() != null)
> 		if (this.type.superclass().isAbstract() || this.type.superclass().isParameterizedType())
>@@ -577,10 +586,14 @@
> boolean detectInheritedNameClash(MethodBinding inherited, MethodBinding otherInherited) {
> 	if (!inherited.areParameterErasuresEqual(otherInherited))
> 		return false;
>-	// skip it if otherInherited is defined by a subtype of inherited's declaringClass
>-	if (inherited.declaringClass.erasure() != otherInherited.declaringClass.erasure())
>+	// skip it if otherInherited is defined by a subtype of inherited's declaringClass or vice versa.
>+	// avoid being order sensitive and check with the roles reversed also.
>+	if (inherited.declaringClass.erasure() != otherInherited.declaringClass.erasure()) {
> 		if (inherited.declaringClass.findSuperTypeOriginatingFrom(otherInherited.declaringClass) != null)
> 			return false;
>+		if (otherInherited.declaringClass.findSuperTypeOriginatingFrom(inherited.declaringClass) != null)
>+			return false;
>+	}
> 
> 	problemReporter().inheritedMethodsHaveNameClash(this.type, inherited, otherInherited);
> 	return true;
>#P org.eclipse.jdt.core.tests.compiler
>Index: src/org/eclipse/jdt/core/tests/compiler/regression/AmbiguousMethodTest.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/AmbiguousMethodTest.java,v
>retrieving revision 1.73
>diff -u -r1.73 AmbiguousMethodTest.java
>--- src/org/eclipse/jdt/core/tests/compiler/regression/AmbiguousMethodTest.java	2 Nov 2009 14:19:05 -0000	1.73
>+++ src/org/eclipse/jdt/core/tests/compiler/regression/AmbiguousMethodTest.java	16 Feb 2010 07:57:44 -0000
>@@ -3528,4 +3528,166 @@
> 		},
> 		"SUCCESS");
> }
>+// https://bugs.eclipse.org/bugs/show_bug.cgi?id=302358
>+public void test081() {
>+	this.runConformTest(
>+		new String[] {
>+			"C.java",
>+			"class A<ModelType extends D, ValueType> implements I<ModelType, ValueType> {\n" +
>+			"    public void doSet(ModelType valueGetter) {\n" +
>+			"        this.set((ValueType) valueGetter.getObject());\n" +
>+			"    }\n" +
>+			"    public void set(Object object) {\n" +
>+			"        System.out.println(\"In A.set(Object)\");\n" +
>+			"    }\n" +
>+			"}\n" +
>+			"class B extends A<E, CharSequence> {\n" +
>+			"	public void set(CharSequence string) {\n" +
>+			"        System.out.println(\"In B.set(CharSequence)\");\n" +
>+			"    }\n" +
>+			"}\n" +
>+			"public class C extends B {\n" +
>+			"    static public void main(String[] args) {\n" +
>+			"        C c = new C();\n" +
>+			"        c.run();\n" +
>+			"    }\n" +
>+			"    public void run() {\n" +
>+			"        E e = new E<String>(String.class);\n" +
>+			"        this.doSet(e);\n" +
>+			"    }\n" +
>+			"}\n" +
>+			"class D {\n" +
>+			"    public Object getObject() {\n" +
>+			"        return null;\n" +
>+			"    }\n" +
>+			"}\n" +
>+			"class E<Type extends CharSequence> extends D {\n" +
>+			"    private Class<Type> typeClass;\n" +
>+			"    public E(Class<Type> typeClass) {\n" +
>+			"        this.typeClass = typeClass;\n" +
>+			"    }\n" +
>+			"    public Type getObject() {\n" +
>+			"        try {\n" +
>+			"            return (Type) typeClass.newInstance();\n" +
>+			"        } catch (Exception e) {\n" +
>+			"            throw new RuntimeException(e);\n" +
>+			"        }\n" +
>+			"    }\n" +
>+			"}\n" +
>+			"interface I<ModelType, ValueType> {\n" +
>+			"    public void doSet(ModelType model);\n" +
>+			"    public void set(ValueType value);\n" +
>+			"}\n"
>+
>+		},
>+		"In B.set(CharSequence)");
>+}
>+// https://bugs.eclipse.org/bugs/show_bug.cgi?id=302358
>+public void test082() {
>+	this.runConformTest(
>+		new String[] {
>+			"C.java",
>+			"class A<ModelType extends D, ValueType> extends I<ModelType, ValueType> {\n" +
>+			"    public void doSet(ModelType valueGetter) {\n" +
>+			"        this.set((ValueType) valueGetter.getObject());\n" +
>+			"    }\n" +
>+			"    public void set(Object object) {\n" +
>+			"        System.out.println(\"In A.set(Object)\");\n" +
>+			"    }\n" +
>+			"}\n" +
>+			"class B extends A<E, CharSequence> {\n" +
>+			"	public void set(CharSequence string) {\n" +
>+			"        System.out.println(\"In B.set(CharSequence)\");\n" +
>+			"    }\n" +
>+			"}\n" +
>+			"public class C extends B {\n" +
>+			"    static public void main(String[] args) {\n" +
>+			"        C c = new C();\n" +
>+			"        c.run();\n" +
>+			"    }\n" +
>+			"    public void run() {\n" +
>+			"        E e = new E<String>(String.class);\n" +
>+			"        this.doSet(e);\n" +
>+			"    }\n" +
>+			"}\n" +
>+			"class D {\n" +
>+			"    public Object getObject() {\n" +
>+			"        return null;\n" +
>+			"    }\n" +
>+			"}\n" +
>+			"class E<Type extends CharSequence> extends D {\n" +
>+			"    private Class<Type> typeClass;\n" +
>+			"    public E(Class<Type> typeClass) {\n" +
>+			"        this.typeClass = typeClass;\n" +
>+			"    }\n" +
>+			"    public Type getObject() {\n" +
>+			"        try {\n" +
>+			"            return (Type) typeClass.newInstance();\n" +
>+			"        } catch (Exception e) {\n" +
>+			"            throw new RuntimeException(e);\n" +
>+			"        }\n" +
>+			"    }\n" +
>+			"}\n" +
>+			"abstract class I<ModelType, ValueType> {\n" +
>+			"    public abstract void doSet(ModelType model);\n" +
>+			"    public abstract void set(ValueType value);\n" +
>+			"}\n"
>+
>+		},
>+		"In B.set(CharSequence)");
>+}
>+// https://bugs.eclipse.org/bugs/show_bug.cgi?id=302358
>+public void test083() {
>+	this.runConformTest(
>+		new String[] {
>+			"C.java",
>+			"class A<ModelType extends D, ValueType> implements I<ModelType, ValueType> {\n" +
>+			"    public void doSet(ModelType valueGetter) {\n" +
>+			"        this.set((ValueType) valueGetter.getObject());\n" +
>+			"    }\n" +
>+			"    public void set(Object object) {\n" +
>+			"        System.out.println(\"In A.set(Object)\");\n" +
>+			"    }\n" +
>+			"}\n" +
>+			"class B extends A<E, CharSequence> implements I<E, CharSequence> {\n" +
>+			"	public void set(CharSequence string) {\n" +
>+			"        System.out.println(\"In B.set(CharSequence)\");\n" +
>+			"    }\n" +
>+			"}\n" +
>+			"public class C extends B {\n" +
>+			"    static public void main(String[] args) {\n" +
>+			"        C c = new C();\n" +
>+			"        c.run();\n" +
>+			"    }\n" +
>+			"    public void run() {\n" +
>+			"        E e = new E<String>(String.class);\n" +
>+			"        this.doSet(e);\n" +
>+			"    }\n" +
>+			"}\n" +
>+			"class D {\n" +
>+			"    public Object getObject() {\n" +
>+			"        return null;\n" +
>+			"    }\n" +
>+			"}\n" +
>+			"class E<Type extends CharSequence> extends D {\n" +
>+			"    private Class<Type> typeClass;\n" +
>+			"    public E(Class<Type> typeClass) {\n" +
>+			"        this.typeClass = typeClass;\n" +
>+			"    }\n" +
>+			"    public Type getObject() {\n" +
>+			"        try {\n" +
>+			"            return (Type) typeClass.newInstance();\n" +
>+			"        } catch (Exception e) {\n" +
>+			"            throw new RuntimeException(e);\n" +
>+			"        }\n" +
>+			"    }\n" +
>+			"}\n" +
>+			"interface I<ModelType, ValueType> {\n" +
>+			"    public void doSet(ModelType model);\n" +
>+			"    public void set(ValueType value);\n" +
>+			"}\n"
>+
>+		},
>+		"In B.set(CharSequence)");
>+}
> }

Actions: View | Diff

Attachments on bug 302358: 158659 | 159149 | 159151 | 159272