Skip to content

Commit

Permalink
Reset implementation for resolving ambiguous methods in favour of ano…
Browse files Browse the repository at this point in the history 
…ther attempt. Smaller clean-ups in the method graph implementation. Fixed test that was previously broken.
  • Loading branch information
@raphw
raphw committed Jul 31, 2015
1 parent 8252d13 commit b5cb5d4
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 70 deletions.
113 changes: 52 additions & 61 deletions byte-buddy-dep/src/main/java/net/bytebuddy/dynamic/scaffold/MethodGraph.java
View file
Expand Up @@ -195,12 +195,12 @@ public MethodGraph.Linked make(TypeDescription typeDescription) {
List<GenericTypeDescription> interfaceTypes = typeDescription.getInterfaces(); List<GenericTypeDescription> interfaceTypes = typeDescription.getInterfaces();
Map<TypeDescription, MethodGraph> interfaceGraphs = new HashMap<TypeDescription, MethodGraph>(interfaceTypes.size()); Map<TypeDescription, MethodGraph> interfaceGraphs = new HashMap<TypeDescription, MethodGraph>(interfaceTypes.size());
for (GenericTypeDescription interfaceType : interfaceTypes) { for (GenericTypeDescription interfaceType : interfaceTypes) {
interfaceGraphs.put(interfaceType.asRawType(), snapshots.get(interfaceType).asGraph(merger)); interfaceGraphs.put(interfaceType.asRawType(), snapshots.get(interfaceType).asGraph());
} }
return new Linked.Delegation(rootStore.asGraph(merger), return new Linked.Delegation(rootStore.asGraph(),
superType == null superType == null
? Empty.INSTANCE ? Empty.INSTANCE
: snapshots.get(superType).asGraph(merger), : snapshots.get(superType).asGraph(),
interfaceGraphs); interfaceGraphs);
} }


Expand Down Expand Up @@ -232,7 +232,7 @@ protected Key.Store<T> doAnalyze(GenericTypeDescription typeDescription,
Key.Store<T> keyStore = analyzeNullable(typeDescription.getSuperType(), snapshots, nextMatcher, nextMatcher); Key.Store<T> keyStore = analyzeNullable(typeDescription.getSuperType(), snapshots, nextMatcher, nextMatcher);
Key.Store<T> interfaceKeyStore = new Key.Store<T>(); Key.Store<T> interfaceKeyStore = new Key.Store<T>();
for (GenericTypeDescription interfaceType : typeDescription.getInterfaces()) { for (GenericTypeDescription interfaceType : typeDescription.getInterfaces()) {
interfaceKeyStore = interfaceKeyStore.combineWith(analyze(interfaceType, snapshots, nextMatcher, nextMatcher)); interfaceKeyStore = interfaceKeyStore.combineWith(analyze(interfaceType, snapshots, nextMatcher, nextMatcher), merger);
} }
keyStore = keyStore.inject(interfaceKeyStore); keyStore = keyStore.inject(interfaceKeyStore);
for (MethodDescription methodDescription : typeDescription.getDeclaredMethods().filter(currentMatcher)) { for (MethodDescription methodDescription : typeDescription.getDeclaredMethods().filter(currentMatcher)) {
Expand Down Expand Up @@ -577,37 +577,35 @@ private Store(LinkedHashMap<Harmonized<V>, Entry<V>> entries) {


protected Store<V> registerTopLevel(MethodDescription methodDescription, Harmonizer<V> harmonizer, Merger merger) { protected Store<V> registerTopLevel(MethodDescription methodDescription, Harmonizer<V> harmonizer, Merger merger) {
Harmonized<V> key = Harmonized.of(methodDescription, harmonizer); Harmonized<V> key = Harmonized.of(methodDescription, harmonizer);
Entry<V> currentEntry = entries.get(key); LinkedHashMap<Harmonized<V>, Entry<V>> entries = new LinkedHashMap<Harmonized<V>, Entry<V>>(this.entries);
Entry<V> currentEntry = entries.remove(key);
Entry<V> extendedEntry = (currentEntry == null Entry<V> extendedEntry = (currentEntry == null
? new Entry.Initial<V>(key) ? new Entry.Initial<V>(key)
: currentEntry).extendWith(methodDescription, harmonizer, merger); : currentEntry).extendWith(methodDescription, harmonizer, merger);
LinkedHashMap<Harmonized<V>, Entry<V>> entries = new LinkedHashMap<Harmonized<V>, Entry<V>>(this.entries);
entries.remove(key);
entries.put(extendedEntry.getKey(), extendedEntry); entries.put(extendedEntry.getKey(), extendedEntry);
return new Store<V>(entries); return new Store<V>(entries);
} }


protected Store<V> combineWith(Store<V> keyStore) { protected Store<V> combineWith(Store<V> keyStore, Merger merger) {
Store<V> combinedStore = this; Store<V> combinedStore = this;
for (Entry<V> entry : keyStore.entries.values()) { for (Entry<V> entry : keyStore.entries.values()) {
combinedStore = combinedStore.combineWith(entry); combinedStore = combinedStore.combineWith(entry, merger);
} }
return combinedStore; return combinedStore;
} }


protected Store<V> combineWith(Entry<V> entry) { protected Store<V> combineWith(Entry<V> entry, Merger merger) {
LinkedHashMap<Harmonized<V>, Entry<V>> entries = new LinkedHashMap<Harmonized<V>, Entry<V>>(this.entries); LinkedHashMap<Harmonized<V>, Entry<V>> entries = new LinkedHashMap<Harmonized<V>, Entry<V>>(this.entries);
Entry<V> previousEntry = entries.get(entry.getKey()); Entry<V> previousEntry = entries.remove(entry.getKey());
Entry<V> injectedEntry = previousEntry == null Entry<V> injectedEntry = previousEntry == null
? entry ? entry
: combine(previousEntry, entry); : combine(previousEntry, entry, merger);
entries.remove(entry.getKey());
entries.put(injectedEntry.getKey(), injectedEntry); entries.put(injectedEntry.getKey(), injectedEntry);
return new Store<V>(entries); return new Store<V>(entries);
} }


private static <W> Entry<W> combine(Entry<W> left, Entry<W> right) { private static <W> Entry<W> combine(Entry<W> left, Entry<W> right, Merger merger) {
Set<MethodDescription> leftMethods = left.getCandidates(), rightMethods = right.getCandidates(); MethodDescription leftMethod = left.getRepresentative(), rightMethod = right.getRepresentative();
if (leftMethod.getDeclaringType().equals(rightMethod.getDeclaringType())) { if (leftMethod.getDeclaringType().equals(rightMethod.getDeclaringType())) {
return left; return left;
} }
Expand All @@ -617,7 +615,7 @@ private static <W> Entry<W> combine(Entry<W> left, Entry<W> right) {
} else if (rightType.isAssignableTo(leftType)) { } else if (rightType.isAssignableTo(leftType)) {
return right; return right;
} else { } else {
return Entry.Ambiguous.of(left.getKey().combineWith(right.getKey()), leftMethod, rightMethod); return Entry.Ambiguous.of(left.getKey().combineWith(right.getKey()), leftMethod, rightMethod, merger);
} }
} }


Expand All @@ -631,19 +629,18 @@ protected Store<V> inject(Store<V> keyStore) {


protected Store<V> inject(Entry<V> entry) { protected Store<V> inject(Entry<V> entry) {
LinkedHashMap<Harmonized<V>, Entry<V>> entries = new LinkedHashMap<Harmonized<V>, Entry<V>>(this.entries); LinkedHashMap<Harmonized<V>, Entry<V>> entries = new LinkedHashMap<Harmonized<V>, Entry<V>>(this.entries);
Entry<V> dominantEntry = entries.get(entry.getKey()); Entry<V> dominantEntry = entries.remove(entry.getKey());
Entry<V> injectedEntry = dominantEntry == null Entry<V> injectedEntry = dominantEntry == null
? entry ? entry
: dominantEntry.inject(entry.getKey()); : dominantEntry.inject(entry.getKey());
entries.remove(entry.getKey());
entries.put(injectedEntry.getKey(), injectedEntry); entries.put(injectedEntry.getKey(), injectedEntry);
return new Store<V>(entries); return new Store<V>(entries);
} }


protected MethodGraph asGraph(Merger merger) { protected MethodGraph asGraph() {
LinkedHashMap<Key<MethodDescription.Token>, Node> entries = new LinkedHashMap<Key<MethodDescription.Token>, Node>(this.entries.size()); LinkedHashMap<Key<MethodDescription.Token>, Node> entries = new LinkedHashMap<Key<MethodDescription.Token>, Node>(this.entries.size());
for (Entry<V> entry : this.entries.values()) { for (Entry<V> entry : this.entries.values()) {
entries.put(entry.getKey().detach(), entry.asNode(merger)); entries.put(entry.getKey().detach(), entry.asNode());
} }
return new Graph(entries); return new Graph(entries);
} }
Expand Down Expand Up @@ -710,13 +707,13 @@ protected interface Entry<W> {


Harmonized<W> getKey(); Harmonized<W> getKey();


Set<MethodDescription> getCandidates(); MethodDescription getRepresentative();


Entry<W> extendWith(MethodDescription methodDescription, Harmonizer<W> harmonizer, Merger merger); Entry<W> extendWith(MethodDescription methodDescription, Harmonizer<W> harmonizer, Merger merger);


Entry<W> inject(Harmonized<W> key); Entry<W> inject(Harmonized<W> key);


Node asNode(Merger merger); Node asNode();


class Initial<U> implements Entry<U> { class Initial<U> implements Entry<U> {


Expand All @@ -732,7 +729,7 @@ public Harmonized<U> getKey() {
} }


@Override @Override
public Set<MethodDescription> getCandidates() { public MethodDescription getRepresentative() {
throw new IllegalStateException("Cannot extract method from initial entry:" + this); throw new IllegalStateException("Cannot extract method from initial entry:" + this);
} }


Expand All @@ -747,7 +744,7 @@ public Entry<U> inject(Harmonized<U> key) {
} }


@Override @Override
public Node asNode(Merger merger) { public Node asNode() {
throw new IllegalStateException("Cannot transform initial entry without a registered method: " + this); throw new IllegalStateException("Cannot transform initial entry without a registered method: " + this);
} }


Expand Down Expand Up @@ -788,15 +785,15 @@ public Harmonized<U> getKey() {
} }


@Override @Override
public Set<MethodDescription> getCandidates() { public MethodDescription getRepresentative() {
return Collections.singleton(methodDescription); return methodDescription;
} }


@Override @Override
public Entry<U> extendWith(MethodDescription methodDescription, Harmonizer<U> harmonizer, Merger merger) { public Entry<U> extendWith(MethodDescription methodDescription, Harmonizer<U> harmonizer, Merger merger) {
Harmonized<U> key = this.key.extend(methodDescription.asDefined(), harmonizer); Harmonized<U> key = this.key.extend(methodDescription.asDefined(), harmonizer);
return methodDescription.getDeclaringType().equals(this.methodDescription.getDeclaringType()) return methodDescription.getDeclaringType().equals(this.methodDescription.getDeclaringType())
? Ambiguous.of(key, methodDescription, this.methodDescription) ? Ambiguous.of(key, methodDescription, this.methodDescription, merger)
: new ForMethod<U>(key, methodDescription.isBridge() ? this.methodDescription : methodDescription, methodDescription.isBridge()); : new ForMethod<U>(key, methodDescription.isBridge() ? this.methodDescription : methodDescription, methodDescription.isBridge());
} }


Expand All @@ -806,7 +803,7 @@ public Entry<U> inject(Harmonized<U> key) {
} }


@Override @Override
public MethodGraph.Node asNode(Merger merger) { public MethodGraph.Node asNode() {
return new Node(key.detach(), methodDescription, madeVisible); return new Node(key.detach(), methodDescription, madeVisible);
} }


Expand Down Expand Up @@ -904,17 +901,17 @@ class Ambiguous<U> implements Entry<U> {


private final Harmonized<U> key; private final Harmonized<U> key;


private final Set<MethodDescription> methodDescriptions; private final MethodDescription methodDescription;


protected static <Q> Entry<Q> of(Harmonized<Q> key, MethodDescription left, MethodDescription right) { protected static <Q> Entry<Q> of(Harmonized<Q> key, MethodDescription left, MethodDescription right, Merger merger) {
return left.isBridge() ^ right.isBridge() return left.isBridge() ^ right.isBridge()
? new ForMethod<Q>(key, left.isBridge() ? right : left, false) ? new ForMethod<Q>(key, left.isBridge() ? right : left, false)
: new Ambiguous<Q>(key, new HashSet<MethodDescription>(Arrays.asList(left, right))); : new Ambiguous<Q>(key, merger.merge(left, right));
} }


protected Ambiguous(Harmonized<U> key, Set<MethodDescription> methodDescriptions) { protected Ambiguous(Harmonized<U> key, MethodDescription methodDescription) {
this.key = key; this.key = key;
this.methodDescriptions = methodDescriptions; this.methodDescription = methodDescription;
} }


@Override @Override
Expand All @@ -923,41 +920,35 @@ public Harmonized<U> getKey() {
} }


@Override @Override
public Set<MethodDescription> getCandidates() { public MethodDescription getRepresentative() {
return methodDescriptions; return methodDescription;
} }


@Override @Override
public Entry<U> extendWith(MethodDescription methodDescription, Harmonizer<U> harmonizer, Merger merger) { public Entry<U> extendWith(MethodDescription methodDescription, Harmonizer<U> harmonizer, Merger merger) {
return null; Harmonized<U> key = this.key.extend(methodDescription.asDefined(), harmonizer);
// Harmonized<U> key = this.key.extend(methodDescription.asDefined(), harmonizer); if (methodDescription.getDeclaringType().equals(this.methodDescription.getDeclaringType())) {
// if (methodDescription.getDeclaringType().equals(this.methodDescriptions.getDeclaringType())) { if (this.methodDescription.isBridge() ^ methodDescription.isBridge()) {
// if (this.methodDescriptions.isBridge() ^ methodDescription.isBridge()) { return this.methodDescription.isBridge()
// return this.methodDescriptions.isBridge() ? new ForMethod<U>(key, methodDescription, false)
// ? new ForMethod<U>(key, methodDescription, false) : new Ambiguous<U>(key, this.methodDescription);
// : new Ambiguous<U>(key, this.methodDescriptions); } else {
// } else { return new Ambiguous<U>(key, merger.merge(this.methodDescription, methodDescription));
// return new Ambiguous<U>(key, merger.merge(this.methodDescriptions, methodDescription)); }
// } } else {
// } else { return methodDescription.isBridge()
// return methodDescription.isBridge() ? new Ambiguous<U>(key, this.methodDescription)
// ? new Ambiguous<U>(key, this.methodDescriptions) : new ForMethod<U>(key, methodDescription, false);
// : new ForMethod<U>(key, methodDescription, false); }
// }
} }


@Override @Override
public Entry<U> inject(Harmonized<U> key) { public Entry<U> inject(Harmonized<U> key) {
return new Ambiguous<U>(key.inject(key), methodDescriptions); return new Ambiguous<U>(key.inject(key), methodDescription);
} }


@Override @Override
public MethodGraph.Node asNode(Merger merger) { public MethodGraph.Node asNode() {
Iterator<MethodDescription> iterator = methodDescriptions.iterator();
MethodDescription methodDescription = iterator.next();
while (iterator.hasNext()) {
methodDescription = merger.merge(methodDescription, iterator.next());
}
return new Node(key.detach(), methodDescription); return new Node(key.detach(), methodDescription);
} }


Expand All @@ -966,21 +957,21 @@ public boolean equals(Object other) {
if (this == other) return true; if (this == other) return true;
if (other == null || getClass() != other.getClass()) return false; if (other == null || getClass() != other.getClass()) return false;
Ambiguous<?> ambiguous = (Ambiguous<?>) other; Ambiguous<?> ambiguous = (Ambiguous<?>) other;
return key.equals(ambiguous.key) && methodDescriptions.equals(ambiguous.methodDescriptions); return key.equals(ambiguous.key) && methodDescription.equals(ambiguous.methodDescription);
} }


@Override @Override
public int hashCode() { public int hashCode() {
int result = key.hashCode(); int result = key.hashCode();
result = 31 * result + methodDescriptions.hashCode(); result = 31 * result + methodDescription.hashCode();
return result; return result;
} }


@Override @Override
public String toString() { public String toString() {
return "MethodGraph.Compiler.Default.Key.Store.Entry.Ambiguous{" + return "MethodGraph.Compiler.Default.Key.Store.Entry.Ambiguous{" +
"key=" + key + "key=" + key +
", methodDescriptions=" + methodDescriptions + ", methodDescription=" + methodDescription +
'}'; '}';
} }


Expand Down Expand Up @@ -1034,7 +1025,7 @@ public int hashCode() {
public String toString() { public String toString() {
return "MethodGraph.Compiler.Default.Key.Store.Entry.Ambiguous.Node{" + return "MethodGraph.Compiler.Default.Key.Store.Entry.Ambiguous.Node{" +
"key=" + key + "key=" + key +
", methodToken=" + methodDescription + ", methodDescription=" + methodDescription +
'}'; '}';
} }
} }
Expand Down
4 changes: 1 addition & 3 deletions byte-buddy-dep/src/main/java/net/bytebuddy/implementation/SuperMethodCall.java
View file
Expand Up @@ -107,9 +107,7 @@ protected Appender(Target implementationTarget, TerminationHandler terminationHa
} }


@Override @Override
public Size apply(MethodVisitor methodVisitor, public Size apply(MethodVisitor methodVisitor, Implementation.Context implementationContext, MethodDescription instrumentedMethod) {
Implementation.Context implementationContext,
MethodDescription instrumentedMethod) {
StackManipulation superMethodCall = instrumentedMethod.isDefaultMethod() StackManipulation superMethodCall = instrumentedMethod.isDefaultMethod()
&& implementationTarget.getTypeDescription().getInterfaces().asRawTypes().contains(instrumentedMethod.getDeclaringType().asRawType()) && implementationTarget.getTypeDescription().getInterfaces().asRawTypes().contains(instrumentedMethod.getDeclaringType().asRawType())
? implementationTarget.invokeDefault(instrumentedMethod.getDeclaringType().asRawType(), instrumentedMethod.asToken()) ? implementationTarget.invokeDefault(instrumentedMethod.getDeclaringType().asRawType(), instrumentedMethod.asToken())
Expand Down
8 changes: 2 additions & 6 deletions byte-buddy-dep/src/test/java/net/bytebuddy/implementation/SuperMethodCallOtherTest.java
View file
Expand Up @@ -159,17 +159,13 @@ public void testAmbiguousDirectDefaultMethodThrowsException() throws Exception {
} }




@Test @Test(expected = IllegalStateException.class)
@JavaVersionRule.Enforce(8) @JavaVersionRule.Enforce(8)
public void testNonDeclaredDefaultMethodThrowsException() throws Exception { public void testNonDeclaredDefaultMethodThrowsException() throws Exception {
DynamicType.Loaded<?> loaded = implement(classLoader.loadClass(SINGLE_DEFAULT_METHOD_CLASS), implement(classLoader.loadClass(SINGLE_DEFAULT_METHOD_CLASS),
SuperMethodCall.INSTANCE, SuperMethodCall.INSTANCE,
classLoader, classLoader,
isMethod().and(not(isDeclaredBy(Object.class)))); isMethod().and(not(isDeclaredBy(Object.class))));
assertThat(loaded.getLoaded().getDeclaredMethods().length, is(1));
Method method = loaded.getLoaded().getDeclaredMethod(FOO);
Object instance = loaded.getLoaded().newInstance();
assertThat(method.invoke(instance), is((Object) FOO));
} }


@Test @Test
Expand Down

0 comments on commit b5cb5d4

Please sign in to comment.