Skip to content

Commit

Permalink
Fix ICE due to abstract supertype method and default method conflict.
Browse files Browse the repository at this point in the history
Change-Id: I679470829cc568108f23d97d10b18f00d64fc4e3
  • Loading branch information
rluble committed May 20, 2016
1 parent 170a84e commit 4e0b1d6
Show file tree
Hide file tree
Showing 4 changed files with 83 additions and 30 deletions.
Expand Up @@ -138,15 +138,14 @@ private void computeOverrides(JDeclaredType type) {
// Compute interface overrides, fix accidental overrides and implement default methods. // Compute interface overrides, fix accidental overrides and implement default methods.
for (String signature : interfaceMethodsBySignature.keySet()) { for (String signature : interfaceMethodsBySignature.keySet()) {
Collection<JMethod> interfaceMethods = interfaceMethodsBySignature.get(signature); Collection<JMethod> interfaceMethods = interfaceMethodsBySignature.get(signature);
JMethod baseImplementingMethod = polymorphicMethodsByExtendedSignature.get(signature); JMethod implementingMethod = polymorphicMethodsByExtendedSignature.get(signature);
if (baseImplementingMethod == null) { if (implementingMethod == null) {
// See if there is a package private method whose visibility is made public by the // See if there is a package private method whose visibility is made public by the
// override (can actually only happen for abstract methods, as it is a compiler error // override (can actually only happen for abstract methods, as it is a compiler error
// otherwise. // otherwise.
baseImplementingMethod = polymorphicMethodsByExtendedSignature.get( implementingMethod = polymorphicMethodsByExtendedSignature.get(
computePackagePrivateSignature(type.getPackageName(), signature)); computePackagePrivateSignature(type.getPackageName(), signature));
} }
JMethod implementingMethod = baseImplementingMethod;
if (implementingMethod == null || implementingMethod.getEnclosingType() != type) { if (implementingMethod == null || implementingMethod.getEnclosingType() != type) {
implementingMethod = maybeAddSyntheticOverride(type, implementingMethod, interfaceMethods); implementingMethod = maybeAddSyntheticOverride(type, implementingMethod, interfaceMethods);
if (implementingMethod == null) { if (implementingMethod == null) {
Expand Down Expand Up @@ -246,17 +245,17 @@ private static String computePackagePrivateSignature(String packageName, String
* interfaces when is only one declaration of the method in the super interface hierarchy. * interfaces when is only one declaration of the method in the super interface hierarchy.
*/ */
private JMethod maybeAddSyntheticOverride( private JMethod maybeAddSyntheticOverride(
JDeclaredType type, JMethod method, Collection<JMethod> interfaceMethods) { JDeclaredType type, JMethod superMethod, Collection<JMethod> interfaceMethods) {


// If there is a default implementation it will be first and the only default in the collection // If there is a default implementation it will be first and the only default in the collection
// (as multiple "active" defaults are a compiler error). // (as multiple "active" defaults are a compiler error).
JMethod interfaceMethod = interfaceMethods.iterator().next(); JMethod interfaceMethod = interfaceMethods.iterator().next();
assert !interfaceMethod.isStatic(); assert !interfaceMethod.isStatic();


JMethod implementingMethod = method; JMethod implementingMethod = superMethod;


// Only populate classes with stubs, forwarding methods or default implementations. // Only populate classes with stubs, forwarding methods or default implementations.
if (needsStubMethod(type, method, interfaceMethod)) { if (needsDefaultImplementationStubMethod(type, superMethod, interfaceMethod)) {


assert FluentIterable.from(interfaceMethods).filter(new Predicate<JMethod>() { assert FluentIterable.from(interfaceMethods).filter(new Predicate<JMethod>() {
@Override @Override
Expand All @@ -277,25 +276,25 @@ public boolean apply(JMethod jMethod) {
implementingMethod = JjsUtils.createForwardingMethod(type, interfaceMethod); implementingMethod = JjsUtils.createForwardingMethod(type, interfaceMethod);


defaultMethodsByForwardingMethod.put(implementingMethod, interfaceMethod); defaultMethodsByForwardingMethod.put(implementingMethod, interfaceMethod);
} else if (method == null && interfaceMethod.isAbstract() && } else if (superMethod == null && interfaceMethod.isAbstract() &&
(type instanceof JClassType || interfaceMethods.size() > 1)) { (type instanceof JClassType || interfaceMethods.size() > 1)) {
// It is an abstract stub // It is an abstract stub
implementingMethod = JjsUtils.createSyntheticAbstractStub(type, interfaceMethod); implementingMethod = JjsUtils.createSyntheticAbstractStub(type, interfaceMethod);
} else if (type instanceof JClassType && method.getEnclosingType() != type && } else if (type instanceof JClassType && superMethod.getEnclosingType() != type &&
!FluentIterable.from(interfaceMethods) !FluentIterable.from(interfaceMethods)
.allMatch(Predicates.in(method.getOverriddenMethods()))) { .allMatch(Predicates.in(superMethod.getOverriddenMethods()))) {
// the implementing method does not override all interface declared methods with the same // the implementing method does not override all interface declared methods with the same
// signature. // signature.
if (method.isAbstract()) { if (superMethod.isAbstract()) {
implementingMethod = JjsUtils.createSyntheticAbstractStub(type, interfaceMethod); implementingMethod = JjsUtils.createSyntheticAbstractStub(type, interfaceMethod);
} else { } else {
// Creates a forwarding method to act as the place holder for this accidental override. // Creates a forwarding method to act as the place holder for this accidental override.
implementingMethod = JjsUtils.createForwardingMethod(type, method); implementingMethod = JjsUtils.createForwardingMethod(type, superMethod);
implementingMethod.setSyntheticAccidentalOverride(); implementingMethod.setSyntheticAccidentalOverride();


if (method.isFinal()) { if (superMethod.isFinal()) {
// To keep consistency we reset the final mark // To keep consistency we reset the final mark
method.setFinal(false); superMethod.setFinal(false);
} }
} }
} }
Expand All @@ -304,23 +303,34 @@ public boolean apply(JMethod jMethod) {
polymorphicMethodsByExtendedSignatureByType.get(type) polymorphicMethodsByExtendedSignatureByType.get(type)
.put(implementingMethod.getSignature(), implementingMethod); .put(implementingMethod.getSignature(), implementingMethod);


if (method != null && method != implementingMethod) { if (superMethod != null && superMethod != implementingMethod) {
addOverridingMethod(method, implementingMethod); addOverridingMethod(superMethod, implementingMethod);
} }
} }
return implementingMethod; return implementingMethod;
} }


/** /**
* Return true if the type {@code type} need to replace {@code method} (possibly {@code null}) * Return true if the type {@code type} need to replace {@code superMethod} (possibly {@code null})
* with a (forwarding) stub due to {@code interfaceMethod}? * with a (forwarding) stub due to default {@code interfaceMethod}.
*/ */
private boolean needsStubMethod(JDeclaredType type, JMethod method, JMethod interfaceMethod) { private boolean needsDefaultImplementationStubMethod(
return type instanceof JClassType && JDeclaredType type, JMethod superMethod, JMethod interfaceMethod) {
interfaceMethod.isDefaultMethod() && (method == null || if (!interfaceMethod.isDefaultMethod() || type instanceof JInterfaceType) {
method.isDefaultMethod() && // Only implement default methods in classes.
defaultMethodsByForwardingMethod.keySet().contains(method) && return false;
defaultMethodsByForwardingMethod.get(method) != interfaceMethod); }
if (superMethod == null || (superMethod.isAbstract() && superMethod.isSynthetic())) {
// The interface method is not implemented or an abstract stub was synthesized as the super
// (not necessarily direct) implementation.
return true;
}
JMethod superForwardingMethod = defaultMethodsByForwardingMethod.get(superMethod);
// A default superMethod stub is in place in the supertype, and needs to be replaced if it does
// not forward to the required default implementation.
return superForwardingMethod != null
&& superForwardingMethod.isDefaultMethod()
&& superForwardingMethod != interfaceMethod;
} }


/** /**
Expand Down
5 changes: 0 additions & 5 deletions dev/core/src/com/google/gwt/dev/jjs/impl/JjsUtils.java
Expand Up @@ -198,11 +198,6 @@ public static JMethod createForwardingMethod(JDeclaredType type,
JMethod forwardingMethod = createEmptyMethodFromExample(type, methodToDelegateTo, false); JMethod forwardingMethod = createEmptyMethodFromExample(type, methodToDelegateTo, false);
forwardingMethod.setForwarding(); forwardingMethod.setForwarding();


// This is a synthetic forwading method due to a default.
if (methodToDelegateTo.isDefaultMethod()) {
forwardingMethod.setDefaultMethod();
}

if (methodToDelegateTo.isJsOverlay() && type.isJsNative()) { if (methodToDelegateTo.isJsOverlay() && type.isJsNative()) {
forwardingMethod.isJsOverlay(); forwardingMethod.isJsOverlay();
} }
Expand Down
Expand Up @@ -1123,6 +1123,46 @@ class B extends A implements IRight, ILeft { }
assertEquals("IRight.m()", new B().m()); assertEquals("IRight.m()", new B().m());
} }


static class DefaultTrumpsOverSyntheticAbstractStub {
interface SuperInterface {
String m();
}

interface SubInterface extends SuperInterface {
default String m() {
return "SubInterface.m()";
}
}
}

public void testMultipleDefaults_defaultShadowsOverSyntheticAbstractStub() {
abstract class A implements DefaultTrumpsOverSyntheticAbstractStub.SuperInterface { }
class B extends A implements DefaultTrumpsOverSyntheticAbstractStub.SubInterface { }

assertEquals("SubInterface.m()", new B().m());
}

static class DefaultTrumpsOverDefaultOnSuperAbstract {
interface SuperInterface {
default String m() {
return "SuperInterface.m()";
}
}

interface SubInterface extends SuperInterface {
default String m() {
return "SubInterface.m()";
}
}
}

public void testMultipleDefaults_defaultShadowsOverDefaultOnSuperAbstract() {
abstract class A implements DefaultTrumpsOverDefaultOnSuperAbstract.SuperInterface { }
class B extends A implements DefaultTrumpsOverDefaultOnSuperAbstract.SubInterface { }

assertEquals("SubInterface.m()", new B().m());
}

interface InterfaceWithThisReference { interface InterfaceWithThisReference {
default String n() { default String n() {
return "default n"; return "default n";
Expand Down
10 changes: 9 additions & 1 deletion user/test/com/google/gwt/dev/jjs/test/Java8Test.java
Expand Up @@ -252,6 +252,14 @@ public void testMultipleDefaults_superclass_right() {
assertFalse(isGwtSourceLevel8()); assertFalse(isGwtSourceLevel8());
} }


public void testMultipleDefaults_defaultShadowsOverSyntheticAbstractStub() {
assertFalse(isGwtSourceLevel8());
}

public void testMultipleDefaults_defaultShadowsOverDefaultOnSuperAbstract() {
assertFalse(isGwtSourceLevel8());
}

public void testInterfaceThis() { public void testInterfaceThis() {
assertFalse(isGwtSourceLevel8()); assertFalse(isGwtSourceLevel8());
} }
Expand All @@ -276,7 +284,7 @@ public void testMethodReferenceWithGenericTypeParameters() {
assertFalse(isGwtSourceLevel8()); assertFalse(isGwtSourceLevel8());
} }


private boolean isGwtSourceLevel8() { private boolean isGwtSourceLevel8() {
return JUnitShell.getCompilerOptions().getSourceLevel().compareTo(SourceLevel.JAVA8) >= 0; return JUnitShell.getCompilerOptions().getSourceLevel().compareTo(SourceLevel.JAVA8) >= 0;
} }
} }

0 comments on commit 4e0b1d6

Please sign in to comment.