Skip to content

Commit

Permalink
Fix and centralize override computations.
Browse files Browse the repository at this point in the history 
Each JMethod contains a list of the methods it overrides; such
list should be exhaustive but the previous computation would
not include all overriden methods.

In the following example

  public class A {
    public void m() { }
  }

  public interface I {
    void m();
  }

  public class B extends A implement I {
  }

A.m() overrides I.m() but was not recorded in the JMethod
representation of A.m() (we will call this type of override
accidental).

This patch creates stubs or forwarding methods to anchor
accidental overrides, e.g. in the previous example a forwarding
stub

  public void m() { super.m(); }

will be created in class and will be marked as overriding both
A.m() and I.m(). If the super method was abstract then only
an abstract stub will be created. Default methods are handled
also integrally as part of the override computation.

The override information had duplicated/overlapping
implementations in UnifyAst and JTypeOracle, neither of which
complete nor correct. Override informatin is mostly used in
optimizations resulting in at best missed optimization
opportunities and subtle bugs at worst.

This patch:
  1) fixes the override computation in UnifyAST to take into
     account package-private methods and accidental override.
  2) fixes the selection of the concrete default method
     implementation.
  3) centralizes override information in JMethod. Now JMethod
     holds both overriden methods and overriding methods, and
  4) updates the list of overrides in JMethod when pruning.

Change-Id: I1f9af8df529f48b021c8c5de1290780ab5c0f645
  • Loading branch information
@rluble
rluble committed Mar 13, 2015
1 parent 1f7ffb5 commit bc8bfaf
Show file tree
Hide file tree
Showing 21 changed files with 1,213 additions and 412 deletions.
1 change: 0 additions & 1 deletion dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java
View file
Expand Up @@ -1374,7 +1374,6 @@ protected final void optimizeJavaToFixedPoint() throws InterruptedException {
int passLimit = atMaxLevel ? MAX_PASSES : options.getOptimizationLevel();
float minChangeRate = atMaxLevel ? FIXED_POINT_CHANGE_RATE : EFFICIENT_CHANGE_RATE;
OptimizerContext optimizerCtx = new FullOptimizerContext(jprogram);
jprogram.typeOracle.computeOverrides(jprogram.getDeclaredTypes());
while (true) {
passCount++;
if (passCount > passLimit) {
Expand Down
30 changes: 25 additions & 5 deletions dev/core/src/com/google/gwt/dev/jjs/ast/JMethod.java
View file
Expand Up @@ -301,8 +301,6 @@ private Object readResolve() {
public static final JMethod NULL_METHOD = new JMethod(SourceOrigin.UNKNOWN, "nullMethod", null,
JNullType.INSTANCE, false, false, true, AccessModifier.PUBLIC);

private static final String TRACE_METHOD_WILDCARD = "*";

static {
NULL_METHOD.setSynthetic();
NULL_METHOD.freezeParamTypes();
Expand Down Expand Up @@ -335,6 +333,7 @@ private Object readResolve() {
* list will contain both A and B.
*/
private Set<JMethod> overriddenMethods = Sets.newLinkedHashSet();
private Set<JMethod> overridingMethods = Sets.newLinkedHashSet();

private List<JParameter> params = Collections.emptyList();
private JType returnType;
Expand Down Expand Up @@ -383,11 +382,20 @@ private JMethod(String signature, JDeclaredType enclosingType, boolean isStatic)
/**
* Add a method that this method overrides.
*/
public void addOverriddenMethod(JMethod toAdd) {
public void addOverriddenMethod(JMethod overriddenMethod) {
assert canBePolymorphic() : this + " is not polymorphic";
overriddenMethods.add(toAdd);
assert overriddenMethod != this : this + " cannot override itself";
overriddenMethods.add(overriddenMethod);
}

/**
* Add a method that overrides this method.
*/
public void addOverridingMethod(JMethod overridingMethod) {
assert canBePolymorphic() : this + " is not polymorphic";
assert overridingMethod != this : this + " cannot override itself";
overridingMethods.add(overridingMethod);
}
/**
* Adds a parameter to this method.
*/
Expand Down Expand Up @@ -479,6 +487,14 @@ public Set<JMethod> getOverriddenMethods() {
return overriddenMethods;
}

/**
* Returns the transitive closure of all the methods that override this method; caveat this
* list is only complete in monolithic compiles and should not be used in incremental compiles..
*/
public Set<JMethod> getOverridingMethods() {
return overridingMethods;
}

/**
* Returns the parameters of this method.
*/
Expand Down Expand Up @@ -635,7 +651,11 @@ public void setBody(JAbstractMethodBody body) {

@Override
public void setFinal() {
isFinal = true;
setFinal(true);
}

public void setFinal(boolean isFinal) {
this.isFinal = isFinal;
}

public void setOriginalTypes(JType returnType, List<JType> paramTypes) {
Expand Down
176 changes: 2 additions & 174 deletions dev/core/src/com/google/gwt/dev/jjs/ast/JTypeOracle.java
View file
Expand Up @@ -28,7 +28,6 @@
import com.google.gwt.thirdparty.guava.common.collect.ImmutableList;
import com.google.gwt.thirdparty.guava.common.collect.ImmutableSetMultimap;
import com.google.gwt.thirdparty.guava.common.collect.Iterables;
import com.google.gwt.thirdparty.guava.common.collect.LinkedHashMultimap;
import com.google.gwt.thirdparty.guava.common.collect.Lists;
import com.google.gwt.thirdparty.guava.common.collect.Maps;
import com.google.gwt.thirdparty.guava.common.collect.Multimap;
Expand Down Expand Up @@ -234,7 +233,7 @@ public boolean needsJsInteropBridgeMethod(JMethod x) {
}

if (x.needsVtable() && isJsTypeMethod(x)) {
for (JMethod override : getOverriddenMethodsOf(x)) {
for (JMethod override : x.getOverriddenMethods()) {
if (!isJsTypeMethod(override)) {
return true;
}
Expand Down Expand Up @@ -512,18 +511,6 @@ public static boolean methodsDoMatch(JMethod method1, JMethod method2) {
* NOTE: {@code superInterfacesByInterface} is NOT reflexive.
*/
private Multimap<String, String> superInterfacesByInterface;
/**
* A map of all methods with virtual overrides, onto the collection of
* overridden methods. Each key method's collections is a map of the set of
* subclasses who inherit the key method mapped onto the set of interface
* methods the key method virtually implements. For a definition of a virtual
* override, see {@link #getAllVirtualOverrides(JMethod)}.
*/
private final Map<JMethod, Multimap<JClassType, JMethod>> virtualUpRefMap =
Maps.newIdentityHashMap();

private LinkedHashMultimap<JMethod, JMethod> overriddenMethodsByOverridingMethod;
private LinkedHashMultimap<JMethod, JMethod> overridingMethodsByOverriddenMethod;

/**
* An index of all polymorphic methods for each class.
Expand Down Expand Up @@ -770,6 +757,7 @@ public void computeBeforeAST(StandardTypes standardTypes, Collection<JDeclaredTy
computeExtendedTypeRelations();

for (JDeclaredType type : declaredTypes) {

// first time through, record all exported methods
for (JMethod method : type.getMethods()) {
if (isExportedMethod(method)) {
Expand All @@ -782,31 +770,6 @@ public void computeBeforeAST(StandardTypes standardTypes, Collection<JDeclaredTy
}
}
}

for (JDeclaredType type : declaredTypes) {
if (type instanceof JClassType) {
computeVirtualUpRefs((JClassType) type);
}
}

computeOverrides(declaredTypes);
}

public void computeOverrides(Collection<JDeclaredType> declaredTypes) {
if (overriddenMethodsByOverridingMethod == null
|| overridingMethodsByOverriddenMethod == null) {
overriddenMethodsByOverridingMethod = LinkedHashMultimap.create();
overridingMethodsByOverriddenMethod = LinkedHashMultimap.create();
for (JDeclaredType type : declaredTypes) {
for (JMethod method : type.getMethods()) {
Set<JMethod> overriddens = computeAllOverriddenMethods(method);
overriddenMethodsByOverridingMethod.putAll(method, overriddens);
for (JMethod overridden : overriddens) {
overridingMethodsByOverriddenMethod.put(overridden, method);
}
}
}
}
}

private static Collection<String> getNamesOf(Collection<JDeclaredType> types) {
Expand All @@ -824,68 +787,6 @@ private void recordReferenceTypeByName(Collection<JDeclaredType> types) {
}
}

/**
* References to any methods which this method implementation might override
* or implement in any instantiable class, including strange cases where there
* is no direct relationship between the methods except in a subclass that
* inherits one and implements the other. Example:
*
* <pre>
* interface IFoo {
* foo();
* }
* class Unrelated {
* foo() { ... }
* }
* class Foo extends Unrelated implements IFoo {
* }
* </pre>
*
* In this case, <code>Unrelated.foo()</code> virtually implements
* <code>IFoo.foo()</code> in subclass <code>Foo</code>.
*/
private Set<JMethod> computeAllOverriddenMethods(JMethod method) {
Set<JMethod> results = Sets.newIdentityHashSet();
results.addAll(method.getOverriddenMethods());
getAllVirtualOverriddenMethods(method, results);
return results;
}

public Set<JMethod> getOverriddenMethodsOf(JMethod method) {
assert (overriddenMethodsByOverridingMethod != null);
return overriddenMethodsByOverridingMethod.get(method);
}

public Set<JMethod> getOverridingMethodsOf(JMethod method) {
assert (overridingMethodsByOverriddenMethod != null);
return overridingMethodsByOverriddenMethod.get(method);
}

public LinkedHashMultimap<JMethod, JMethod> getAllOverriddens() {
assert (overriddenMethodsByOverridingMethod != null);
return overriddenMethodsByOverridingMethod;
}

public LinkedHashMultimap<JMethod, JMethod> getAllOverridings() {
assert (overridingMethodsByOverriddenMethod != null);
return overridingMethodsByOverriddenMethod;
}

public void updateOverridesInfo(Set<JMethod> prunedMethods) {
assert (overriddenMethodsByOverridingMethod != null
&& overridingMethodsByOverriddenMethod != null);
for (JMethod prunedMethod : prunedMethods) {
Set<JMethod> overriddenMethods = overriddenMethodsByOverridingMethod.removeAll(prunedMethod);
for (JMethod overriddenMethod : overriddenMethods) {
overridingMethodsByOverriddenMethod.remove(overriddenMethod, prunedMethod);
}
Set<JMethod> overriderMethods = overridingMethodsByOverriddenMethod.removeAll(prunedMethod);
for (JMethod overriderMethod : overriderMethods) {
overriddenMethodsByOverridingMethod.remove(overriderMethod, prunedMethod);
}
}
}

/**
* Get the nearest JS type.
*/
Expand Down Expand Up @@ -1541,67 +1442,6 @@ && isSuperClass((JClassType) type, (JClassType) singleTarget)) {
return null;
}

/**
* WEIRD: Suppose class Foo declares void f(){} and unrelated interface I also
* declares void f(). Then suppose Bar extends Foo implements I and doesn't
* override f(). We need to record a "virtual" upref from Foo.f() to I.f() so
* that if I.f() is rescued AND Bar is instantiable, Foo.f() does not get
* pruned.
*/
private void computeVirtualUpRefs(JClassType type) {
if (type.getSuperClass() == null ||
type.getSuperClass().getName().equals(standardTypes.javaLangObject)) {
return;
}

/*
* For each interface I directly implement, check all methods and make sure
* I define implementations for them. If I don't, then check all my super
* classes to find virtual overrides.
*/
for (JInterfaceType intf : type.getImplements()) {
computeVirtualUpRefs(type, intf);
for (JReferenceType superIntf : getTypes(superInterfacesByInterface.get(intf.getName()))) {
computeVirtualUpRefs(type, (JInterfaceType) superIntf);
}
}
}

/**
* For each interface I directly implement, check all methods and make sure I
* define implementations for them. If I don't, then check all my super
* classes to find virtual overrides.
*/
private void computeVirtualUpRefs(JClassType type, JInterfaceType intf) {
outer : for (JMethod intfMethod : intf.getMethods()) {
for (JMethod classMethod : type.getMethods()) {
if (methodsDoMatch(intfMethod, classMethod)) {
// this class directly implements the interface method
continue outer;
}
}

// this class does not directly implement the interface method
// if any super classes do, create a virtual up ref
for (JClassType superType = type.getSuperClass();
!superType.getName().equals(standardTypes.javaLangObject);
superType = superType.getSuperClass()) {
for (JMethod superMethod : superType.getMethods()) {
if (methodsDoMatch(intfMethod, superMethod)) {
// this super class directly implements the interface method
// create a virtual up ref
Multimap<JClassType, JMethod> classToMethodsMultimap =
getOrCreateMultimap(virtualUpRefMap, superMethod);
classToMethodsMultimap.put(type, intfMethod);

// do not search additional super types
continue outer;
}
}
}
}
}

private JReferenceType ensureTypeExistsAndAppend(String typeName, List<JReferenceType> types) {
JReferenceType type = referenceTypesByName.get(typeName);
assert type != null;
Expand All @@ -1617,18 +1457,6 @@ private boolean extendsInterface(JInterfaceType type, JInterfaceType qType) {
return superInterfacesByInterface.containsEntry(type.getName(), qType.getName());
}

private void getAllVirtualOverriddenMethods(JMethod method, Set<JMethod> results) {
Multimap<JClassType, JMethod> overrideMap = virtualUpRefMap.get(method);
if (overrideMap == null) {
return;
}
for (JClassType classType : overrideMap.keySet()) {
if (isInstantiatedType(classType)) {
results.addAll(overrideMap.get(classType));
}
}
}

/**
* Returns an iterable set of types for the given iterable set of type names.
* <p>
Expand Down

0 comments on commit bc8bfaf

Please sign in to comment.