Bug 3213 - No compile error for bad interface (1G7G6M1)
Summary: No compile error for bad interface (1G7G6M1)
Status: RESOLVED WORKSFORME
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 2.0   Edit
Hardware: All Windows NT
: P3 normal (vote)
Target Milestone: 2.0 M4   Edit
Assignee: Kent Johnson CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2001-10-10 22:51 EDT by Kent Johnson CLA
Modified: 2002-03-13 11:20 EST (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Kent Johnson CLA 2001-10-10 22:51:24 EDT
AK (1/12/01 5:51:04 PM)
	1. create this:
	interface I{
		int toString();
	}
	2. you get no compiler errors.
	the java lang spec (2nd ed.) 9.2 says a compile-time error should occur.

NOTES:


PM (1/18/2001 12:00:23 PM)

9.2 Interface Members
The members of an interface are:

Those members declared in the interface. 
Those members inherited from direct superinterfaces. 
If an interface has no direct superinterfaces, then the interface implicitly 
declares a public abstract member method m with signature s, r
eturn type r, and throws clause t corresponding to each public instance method 
m with signature s, return type r, and throws clause t declared 
in Object, unless a method with the same signature, same return type, and a 
compatible throws clause is explicitly declared by the interface. 
It follows that it is a compile-time error if the interface declares a method 
with the same signature and different return type or incompatible 
throws clause.

The interface inherits, from the interfaces it extends, all members of those 
interfaces, except for fields, classes, and interfaces that it hides 
and methods that it overrides.

PM (6/19/2001 12:43:01 PM)
	Should investigate a fix.

KJ (6/19/2001 12:33:12 PM) - The fix is in the MethodVerifier... need to add 
Object's methods to the inherited list for interfaces:

private void computeInheritedMethods() {
	this.inheritedMethods = new HashtableOfObject(51); // maps method 
selectors to an array of methods... must search to match paramaters & return 
type
	ReferenceBinding[][] interfacesToVisit = new ReferenceBinding[5][];
	int lastPosition = 0;
	interfacesToVisit[lastPosition] = type.superInterfaces();

	if (this.type.isClass()) {
		ReferenceBinding superType = this.type;
		MethodBinding[] nonVisibleDefaultMethods = null;
		int nonVisibleCount = 0;

		while ((superType = superType.superclass()) != null) {
			if (superType.isValidBinding()) {
				ReferenceBinding[] itsInterfaces = 
superType.superInterfaces();
				if (itsInterfaces != NoSuperInterfaces) {
					if (++lastPosition == 
interfacesToVisit.length)
						System.arraycopy
(interfacesToVisit, 0, interfacesToVisit = new ReferenceBinding[lastPosition * 
2][], 0, lastPosition);
					interfacesToVisit[lastPosition] = 
itsInterfaces;
				}

				MethodBinding[] methods = superType.methods();
				nextMethod : for (int m = methods.length; --m 
>= 0;) {

					MethodBinding method = methods[m];
					if (!(method.isPrivate() || 
method.isConstructor() || method.isDefaultAbstract())) { // look at all methods 
which are NOT private or constructors or default abstract
						MethodBinding[] existingMethods 
= (MethodBinding[]) this.inheritedMethods.get(method.selector);
						if (existingMethods != null)
							for (int i = 0, length 
= existingMethods.length; i < length; i++)
								if 
(method.returnType == existingMethods[i].returnType)
									if 
(method.areParametersEqual(existingMethods[i]))
									
	continue nextMethod;
						if (nonVisibleDefaultMethods != 
null)
							for (int i = 0; i < 
nonVisibleCount; i++)
								if 
(method.returnType == nonVisibleDefaultMethods[i].returnType)
									if 
(CharOperation.equals(method.selector, nonVisibleDefaultMethods[i].selector))
									
	if (method.areParametersEqual(nonVisibleDefaultMethods[i]))
										
	continue nextMethod;

						if (!(method.isDefault() && 
(method.declaringClass.fPackage != type.fPackage))) { // ignore methods which 
have default visibility and are NOT defined in another package
							if (existingMethods == 
null)
								existingMethods 
= new MethodBinding[1];
							else
								System.arraycopy
(existingMethods, 0,
								
	(existingMethods = new MethodBinding[existingMethods.length + 1]), 0, 
existingMethods.length - 1);
							existingMethods
[existingMethods.length - 1] = method;
						
	this.inheritedMethods.put(method.selector, existingMethods);
						} else {
							if 
(nonVisibleDefaultMethods == null)
							
	nonVisibleDefaultMethods = new MethodBinding[10];
							else if 
(nonVisibleCount == nonVisibleDefaultMethods.length)
								System.arraycopy
(nonVisibleDefaultMethods, 0,
								
	(nonVisibleDefaultMethods = new MethodBinding[nonVisibleCount * 2]), 0, 
nonVisibleCount);
							nonVisibleDefaultMethods
[nonVisibleCount++] = method;

							if (method.isAbstract() 
&& !this.type.isAbstract()) // non visible abstract methods cannot be 
overridden so the type must be defined abstract
							
	this.problemReporter().abstractMethodCannotBeOverridden(this.type, 
method);

							MethodBinding[] current 
= (MethodBinding[]) this.currentMethods.get(method.selector);
							if (current != null) 
{ // non visible methods cannot be overridden so a warning is issued
								foundMatch : 
for (int i = 0, length = current.length; i < length; i++) {
									if 
(method.returnType == current[i].returnType) {
									
	if (method.areParametersEqual(current[i])) {
										
	this.problemReporter().overridesPackageDefaultMethod(current[i], 
method);
										
	break foundMatch;
									
	}
									}
								}
							}
						}
					}
				}
			}
		}
	} else {
		MethodBinding[] methods = this.type.scope.getJavaLangObject
().methods();
		for (int m = methods.length; --m >= 0;) {
			MethodBinding method = methods[m];
			if (!method.isConstructor()) {
				MethodBinding[] existingMethods = (MethodBinding
[]) this.inheritedMethods.get(method.selector);
				if (existingMethods == null)
					existingMethods = new MethodBinding[1];
				else
					System.arraycopy(existingMethods, 0,
						(existingMethods = new 
MethodBinding[existingMethods.length + 1]), 0, existingMethods.length - 1);
				existingMethods[existingMethods.length - 1] = 
method;
				this.inheritedMethods.put(method.selector, 
existingMethods);
			}
		}
	}

	for (int i = 0; i <= lastPosition; i++) {
		ReferenceBinding[] interfaces = interfacesToVisit[i];
		for (int j = 0, length = interfaces.length; j < length; j++) {
			ReferenceBinding superType = interfaces[j];
			if ((superType.tagBits & InterfaceVisited) == 0) {
				superType.tagBits |= InterfaceVisited;
				if (superType.isValidBinding()) {
					ReferenceBinding[] itsInterfaces = 
superType.superInterfaces();
					if (itsInterfaces != NoSuperInterfaces) 
{
						if (++lastPosition == 
interfacesToVisit.length)
							System.arraycopy
(interfacesToVisit, 0, interfacesToVisit = new ReferenceBinding[lastPosition * 
2][], 0, lastPosition);
						interfacesToVisit[lastPosition] 
= itsInterfaces;
					}

					MethodBinding[] methods = 
superType.methods();
					for (int m = methods.length; --m >= 0;) 
{ // Interface methods are all abstract public
						MethodBinding method = methods
[m];
						MethodBinding[] existingMethods 
= (MethodBinding[]) this.inheritedMethods.get(method.selector);
						if (existingMethods == null)
							existingMethods = new 
MethodBinding[1];
						else
							System.arraycopy
(existingMethods, 0,
							
	(existingMethods = new MethodBinding[existingMethods.length + 1]), 0, 
existingMethods.length - 1);
						existingMethods
[existingMethods.length - 1] = method;
						this.inheritedMethods.put
(method.selector, existingMethods);
					}
				}
			}
		}
	}

	// bit reinitialization
	for (int i = 0; i <= lastPosition; i++) {
		ReferenceBinding[] interfaces = interfacesToVisit[i];
		for (int j = 0, length = interfaces.length; j < length; j++)
			interfaces[j].tagBits &= ~InterfaceVisited;
	}
}

JBL (6/25/2001 12:58:32 PM)
	The above change fixes the original problem, but it fails to correctly 
report a problem on the following test case:
[public interface I1 {
	public Object clone() throws CloneNotSupportedException;
}]
[public interface I2 extends I1 {

}]
	The following error is reported on I2: 'The method clone() cannot hide 
the public abstract method in I1.'
	The error should be on I1 or should not be.

KJ (6/25/2001 11:12:37 AM) - Additonal change in MethodVerifier needed:

private void checkInheritedMethods(MethodBinding[] methods, int length) {
	TypeBinding returnType = methods[0].returnType;
	int index = length;
	while ((--index > 0) && (returnType == methods[index].returnType));
	if (index > 0) {		// All inherited methods do NOT have 
the same vmSignature
		this.problemReporter
().inheritedMethodsHaveIncompatibleReturnTypes(this.type, methods, length);
		return;
	}

	MethodBinding concreteMethod = null;
	for (int i = length; --i >= 0;)		// Remember that only one of 
the methods can be non-abstract
		if (!methods[i].isAbstract()) {
			concreteMethod = methods[i];
			break;
		}
	if (concreteMethod == null) {
		if (this.type.isClass() && !this.type.isAbstract()) {
			for (int i = length; --i >= 0;)
				if (!mustImplementAbstractMethod(methods[i]))
					return;		// in this case, we 
have already reported problem against the concrete superclass
			this.problemReporter().abstractMethodMustBeImplemented
(this.type, methods[0]);
		}
		return;
	} else if (concreteMethod.declaringClass == 
this.type.scope.getJavaLangObject()) {
		// no reason to compare Object's method against inherited 
interface methods
		return;
	}

	MethodBinding[] abstractMethods = new MethodBinding[length - 1];
	index = 0;
	for (int i = length; --i >= 0;)
		if (methods[i] != concreteMethod)
			abstractMethods[index++] = methods[i];

	// Remember that interfaces can only define public instance methods
	if (concreteMethod.isStatic())
		// Cannot inherit a static method which is specified as an 
instance method by an interface
		this.problemReporter().staticInheritedMethodConflicts(type, 
concreteMethod, abstractMethods);	
	if (!concreteMethod.isPublic())
		// Cannot reduce visibility of a public method specified by an 
interface
		this.problemReporter().inheritedMethodReducesVisibility(type, 
concreteMethod, abstractMethods);
	if (concreteMethod.thrownExceptions != NoExceptions)
		for (int i = abstractMethods.length; --i >= 0;)
			this.checkExceptions(concreteMethod, abstractMethods
[i]);
}

JBL (6/25/2001 6:23:06 PM)
	With the above fix don't compare Onject's methods. But shouldn't we 
fail on this test case
	(CloneNotSupportedException is not declared in the redefinition)
[public interface I1 {
	public Object clone();
}]

KJ (6/26/2001 12:24:52 PM) - javac doesn't seem to... (or at least the version 
I have on my machine).

I just tried the following example with javac:

interface J1 {
//	public Object clone();
	public Object clone() throws CloneNotSupportedException;
}
interface J2 extends J1 {}

class C1 implements J2 {}

and got:

zz.java:7: clone() in java.lang.Object cannot implement clone() in J1; 
attempting to assign weaker access privileges; was public
class C1 implements J2 {}
^
1 error

With the latest change we do not complain at all... so are we better off to 
complain that the interface J1 has a problem
than wait for a type to implement the problematic interface?

PM (6/26/2001 5:33:03 PM)
	Waiting for more info on this one (ping'ed Gilad).
	Backing out changes for now.

PM (6/28/2001 1:24:04 PM)
	Actually, Javac's behavior is correct. It is legite to override a 
method with a more public one and with less exceptions.

WRONG===============================
interface J1 {
        int clone();
}
interface J2 extends J1 {}

class C1 implements J2 {}

WRONG===============================
interface J1 {
        Object clone() throws java.io.IOException;
}
interface J2 extends J1 {}

class C2 implements J2 {
        public Object clone() throws  java.io.IOException {
				if (false) throw new java.io.IOException();
                return super.clone();
        }
}


	Interestingly enough, with javac in the following case, the unreachable 
IOException hides the error on J1.
WRONG===============================
interface J1 {
        Object clone() throws java.io.IOException;
}
interface J2 extends J1 {}

class C2 implements J2 {
        public Object clone() throws  java.io.IOException {
                return super.clone();
        }
}

KJ (6/28/2001 10:58:14 AM) - This example compiles without errors:

interface J1 {
	Object clone();
//	Object clone() throws CloneNotSupportedException;
}

interface J2 extends J1 {}

class C1 implements J2 {
	public Object clone() {
//	public Object clone() throws CloneNotSupportedException {
		try { return super.clone(); } catch (CloneNotSupportedException 
e) {return null;}
	}
}
Comment 1 Kent Johnson CLA 2002-03-06 17:14:06 EST
Verified that we have the same behavior as javac 1.3.1.