Skip to main content
aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorStephan Herrmann2012-12-09 16:53:59 -0500
committerStephan Herrmann2012-12-09 16:53:59 -0500
commit7b88699d7247f89fea7506d2e4eadd50330f19b3 (patch)
tree7fa0330d3e61f28e6bfe611f0ac3700fcc1100da
parent90ea20a73e1f3806124ca3cc35a9024b510c4dc7 (diff)
downloadeclipse.jdt.core-sherrmann/Bug388739.tar.gz
eclipse.jdt.core-sherrmann/Bug388739.tar.xz
eclipse.jdt.core-sherrmann/Bug388739.zip
Bug 388739 - [1.8][compiler] consider default methods when detectingsherrmann/Bug388739
whether a class needs to be declared abstract - cleanup of loops "first round" & "second round"; - the latter part incidentally fixes bug 395681.
-rw-r--r--org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/MethodVerifyTest.java5
-rw-r--r--org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/MethodVerifier15.java96
2 files changed, 58 insertions, 43 deletions
diff --git a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/MethodVerifyTest.java b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/MethodVerifyTest.java
index dd0f6ec953..83753dc470 100644
--- a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/MethodVerifyTest.java
+++ b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/MethodVerifyTest.java
@@ -13396,11 +13396,6 @@ public void testBug317719f() throws Exception {
" Zork z;\n" +
" ^^^^\n" +
"Zork cannot be resolved to a type\n" +
- "----------\n" +
- "5. ERROR in X.java (at line 7)\n" +
- " class ChildX<Z> extends X<Z>{}\n" +
- " ^^^^^^\n" +
- "Duplicate methods named forAccountSet with the parameters (List<R>) and (List) are defined by the type X<Z>\n" +
"----------\n":
"----------\n" +
"1. ERROR in X.java (at line 3)\n" +
diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/MethodVerifier15.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/MethodVerifier15.java
index 5d951d919a..677b8292d4 100644
--- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/MethodVerifier15.java
+++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/lookup/MethodVerifier15.java
@@ -509,7 +509,7 @@ void checkMethods() {
}
}
}
- // first round: only check which methods are skippable by comparing all pairs
+ // first round: collect information into skip and isOverridden by comparing all pairs:
// (and perform some side effects : bridge methods & use flags)
for (int i = 0; i < inheritedLength; i++) {
MethodBinding matchMethod = foundMatch[i];
@@ -535,31 +535,15 @@ void checkMethods() {
// This elimination used to happen rather eagerly in computeInheritedMethods step
// itself earlier. (https://bugs.eclipse.org/bugs/show_bug.cgi?id=302358)
if (inheritedMethod.declaringClass != otherInheritedMethod.declaringClass) {
- if (otherInheritedMethod.declaringClass.isInterface() && !inheritedMethod.declaringClass.isInterface()) {
- if (isInterfaceMethodImplemented(otherInheritedMethod, inheritedMethod, otherInheritedMethod.declaringClass)) {
- skip[j] = true;
- isOverridden[j] = true;
- continue;
- }
- } else if (inheritedMethod.declaringClass.isInterface() && !otherInheritedMethod.declaringClass.isInterface()) {
- if (isInterfaceMethodImplemented(inheritedMethod, otherInheritedMethod, inheritedMethod.declaringClass)) {
- skip[i] = true;
- isOverridden[i] = true;
- continue;
- }
- } else if (isParameterSubsignature(inheritedMethod, otherInheritedMethod)) {// FIXME(stephan) ; shouldn't we keep using areMethodsCompatible(inheritedMethod, otherInheritedMethod) instead?
- skip[j] = true;
- isOverridden[j] = inheritedMethod.declaringClass.isCompatibleWith(otherInheritedMethod.declaringClass);
+ // these method calls produce their effect as side-effects into skip and isOverridden:
+ if (isSkippableOrOverridden(inheritedMethod, otherInheritedMethod, skip, isOverridden, j))
continue;
- } else if (isParameterSubsignature(otherInheritedMethod, inheritedMethod)) {// FIXME(stephan) ; see above
- skip[i] = true;
- isOverridden[i] = otherInheritedMethod.declaringClass.isCompatibleWith(inheritedMethod.declaringClass);
+ if (isSkippableOrOverridden(otherInheritedMethod, inheritedMethod, skip, isOverridden, i))
continue;
- }
}
}
}
- // second round: collect and check matchingInherited:
+ // second round: collect and check matchingInherited, directly check methods with no replacing etc.
for (int i = 0; i < inheritedLength; i++) {
MethodBinding matchMethod = foundMatch[i];
if (skip[i]) continue;
@@ -567,25 +551,22 @@ void checkMethods() {
if (matchMethod == null)
matchingInherited[++index] = inheritedMethod;
for (int j = i + 1; j < inheritedLength; j++) {
- MethodBinding otherInheritedMethod = inherited[j];
- if (matchMethod == foundMatch[j] && matchMethod != null)
- continue; // both inherited methods matched the same currentMethod
- if (canSkipInheritedMethods(inheritedMethod, otherInheritedMethod))
- continue;
+ if (foundMatch[j] == null) {
+ MethodBinding otherInheritedMethod = inherited[j];
+ if (matchMethod == foundMatch[j] && matchMethod != null)
+ continue; // both inherited methods matched the same currentMethod
+ if (canSkipInheritedMethods(inheritedMethod, otherInheritedMethod))
+ continue;
- otherInheritedMethod = computeSubstituteMethod(otherInheritedMethod, inheritedMethod);
- if (otherInheritedMethod != null) {
- if (((!inheritedMethod.isAbstract() || otherInheritedMethod.isAbstract()) // if (abstract(inherited) => abstract(other)) check if inherited overrides other
- && isSubstituteParameterSubsignature(inheritedMethod, otherInheritedMethod))
- || ((!otherInheritedMethod.isAbstract() || inheritedMethod.isAbstract()) // if (abstract(other) => abstract(inherited)) check if other overrides inherited
- && isSubstituteParameterSubsignature(otherInheritedMethod, inheritedMethod))) // FIXME(stephan): shouldn't we use a substitute for inheritedMethod here?
- {
- if (index == -1)
- matchingInherited[++index] = inheritedMethod;
- if (foundMatch[j] == null)
- matchingInherited[++index] = otherInheritedMethod;
+ MethodBinding replaceMatch;
+ if ((replaceMatch = findReplacedMethod(inheritedMethod, otherInheritedMethod)) != null) {
+ matchingInherited[++index] = replaceMatch;
skip[j] = true;
- } else if (matchMethod == null && foundMatch[j] == null) {
+ } else if ((replaceMatch = findReplacedMethod(otherInheritedMethod, inheritedMethod)) != null) {
+ matchingInherited[++index] = replaceMatch;
+ skip[j] = true;
+ } else if (matchMethod == null) {
+ // none replaced by the other, check these methods against each other now:
checkInheritedMethods(inheritedMethod, otherInheritedMethod);
}
}
@@ -600,6 +581,45 @@ void checkMethods() {
}
}
}
+/* mark as skippable
+ * - any interface method implemented by a class method
+ * - an x method (x in {class, interface}), for which another x method with a subsignature was found
+ * mark as isOverridden
+ * - any skippable method as defined above iff it is actually overridden by the specific method (disregarding visibility etc.)
+ * Note, that 'idx' corresponds to the position of 'general' in the arrays 'skip' and 'isOverridden'
+ */
+boolean isSkippableOrOverridden(MethodBinding specific, MethodBinding general, boolean[] skip, boolean[] isOverridden, int idx) {
+ boolean specificIsInterface = specific.declaringClass.isInterface();
+ boolean generalIsInterface = general.declaringClass.isInterface();
+ if (!specificIsInterface && generalIsInterface) {
+ if (isInterfaceMethodImplemented(general, specific, general.declaringClass)) {
+ skip[idx] = true;
+ isOverridden[idx] = true;
+ return true;
+ }
+ } else if (specificIsInterface == generalIsInterface) {
+ if (isParameterSubsignature(specific, general)) {
+ skip[idx] = true;
+ isOverridden[idx] = specific.declaringClass.isCompatibleWith(general.declaringClass);
+ return true;
+ }
+ }
+ return false;
+}
+/* 'general' is considered as replaced by 'specific' if
+ * - 'specific' is "at least as concrete as" 'general'
+ * - 'specific' has a signature that is a subsignature of the substituted signature of 'general' (as seen from specific's declaring class)
+ */
+MethodBinding findReplacedMethod(MethodBinding specific, MethodBinding general) {
+ MethodBinding generalSubstitute = computeSubstituteMethod(general, specific);
+ if (generalSubstitute != null
+ && (!specific.isAbstract() || general.isAbstract()) // if (abstract(specific) => abstract(general)) check if 'specific' overrides 'general'
+ && isSubstituteParameterSubsignature(specific, generalSubstitute))
+ {
+ return generalSubstitute;
+ }
+ return null;
+}
void checkTypeVariableMethods(TypeParameter typeParameter) {
char[][] methodSelectors = this.inheritedMethods.keyTable;
nextSelector : for (int s = methodSelectors.length; --s >= 0;) {

Back to the top