Revision: 6669 Author: b...@google.com Date: Wed Nov 4 13:19:11 2009 Log: Method overloads fix for SingleJsoImpl.
Patch by: bobv Review by: spoon http://code.google.com/p/google-web-toolkit/source/detail?r=6669 Modified: /trunk/dev/core/src/com/google/gwt/dev/shell/CompilingClassLoader.java /trunk/dev/core/src/com/google/gwt/dev/shell/rewrite/HostedModeClassRewriter.java /trunk/dev/core/src/com/google/gwt/dev/shell/rewrite/RewriteSingleJsoImplDispatches.java /trunk/dev/core/src/com/google/gwt/dev/shell/rewrite/WriteJsoImpl.java /trunk/user/test/com/google/gwt/dev/jjs/test/SingleJsoImplTest.java ======================================= --- /trunk/dev/core/src/com/google/gwt/dev/shell/CompilingClassLoader.java Mon Oct 26 14:02:26 2009 +++ /trunk/dev/core/src/com/google/gwt/dev/shell/CompilingClassLoader.java Wed Nov 4 13:19:11 2009 @@ -41,6 +41,7 @@ import com.google.gwt.dev.util.Util; import com.google.gwt.dev.util.Name.InternalName; import com.google.gwt.dev.util.Name.SourceOrBinaryName; +import com.google.gwt.dev.util.collect.Lists; import com.google.gwt.util.tools.Utility; import org.apache.commons.collections.map.AbstractReferenceMap; @@ -473,8 +474,8 @@ */ private class MySingleJsoImplData implements SingleJsoImplData { private final SortedSet<String> mangledNames = new TreeSet<String>(); - private final Map<String, com.google.gwt.dev.asm.commons.Method> mangledNamesToDeclarations = new HashMap<String, com.google.gwt.dev.asm.commons.Method>(); - private final Map<String, com.google.gwt.dev.asm.commons.Method> mangledNamesToImplementations = new HashMap<String, com.google.gwt.dev.asm.commons.Method>(); + private final Map<String, List<com.google.gwt.dev.asm.commons.Method>> mangledNamesToDeclarations = new HashMap<String, List<com.google.gwt.dev.asm.commons.Method>>(); + private final Map<String, List<com.google.gwt.dev.asm.commons.Method>> mangledNamesToImplementations = new HashMap<String, List<com.google.gwt.dev.asm.commons.Method>>(); private final SortedSet<String> unmodifiableNames = Collections.unmodifiableSortedSet(mangledNames); private final Set<String> unmodifiableIntfNames = Collections.unmodifiableSet(singleJsoImplTypes); @@ -555,7 +556,7 @@ implementingType = implementingType.getSuperclass(); } // implementingmethod and implementingType cannot be null here - + /* * Create a pseudo-method declaration for the interface method. This * should look something like @@ -574,7 +575,7 @@ decl += ")"; com.google.gwt.dev.asm.commons.Method declaration = com.google.gwt.dev.asm.commons.Method.getMethod(decl); - mangledNamesToDeclarations.put(mangledName, declaration); + addToMap(mangledNamesToDeclarations, mangledName, declaration); } /* @@ -598,7 +599,7 @@ decl += ")"; com.google.gwt.dev.asm.commons.Method toImplement = com.google.gwt.dev.asm.commons.Method.getMethod(decl); - mangledNamesToImplementations.put(mangledName, toImplement); + addToMap(mangledNamesToImplementations, mangledName, toImplement); } } } @@ -606,20 +607,23 @@ if (logger.isLoggable(Type.SPAM)) { TreeLogger dumpLogger = logger.branch(Type.SPAM, "SingleJsoImpl method mappings"); - for (Map.Entry<String, com.google.gwt.dev.asm.commons.Method> entry : mangledNamesToImplementations.entrySet()) { + for (Map.Entry<String, List<com.google.gwt.dev.asm.commons.Method>> entry : mangledNamesToImplementations.entrySet()) { dumpLogger.log(Type.SPAM, entry.getKey() + " -> " + entry.getValue()); } } } - public com.google.gwt.dev.asm.commons.Method getDeclaration( + public List<com.google.gwt.dev.asm.commons.Method> getDeclarations( String mangledName) { - return mangledNamesToDeclarations.get(mangledName); + List<com.google.gwt.dev.asm.commons.Method> toReturn = mangledNamesToDeclarations.get(mangledName); + return toReturn == null ? null : Collections.unmodifiableList(toReturn); } - public com.google.gwt.dev.asm.commons.Method getImplementation( + public List<com.google.gwt.dev.asm.commons.Method> getImplementations( String mangledName) { - return mangledNamesToImplementations.get(mangledName); + List<com.google.gwt.dev.asm.commons.Method> toReturn = mangledNamesToImplementations.get(mangledName); + return toReturn == null ? toReturn + : Collections.unmodifiableList(toReturn); } public SortedSet<String> getMangledNames() { @@ -629,6 +633,21 @@ public Set<String> getSingleJsoIntfTypes() { return unmodifiableIntfNames; } + + /** + * Assumes that the usual case is a 1:1 mapping. + */ + private <K, V> void addToMap(Map<K, List<V>> map, K key, V value) { + List<V> list = map.get(key); + if (list == null) { + map.put(key, Lists.create(value)); + } else { + List<V> maybeOther = Lists.add(list, value); + if (maybeOther != list) { + map.put(key, maybeOther); + } + } + } } /** ======================================= --- /trunk/dev/core/src/com/google/gwt/dev/shell/rewrite/HostedModeClassRewriter.java Sun Oct 18 11:48:45 2009 +++ /trunk/dev/core/src/com/google/gwt/dev/shell/rewrite/HostedModeClassRewriter.java Wed Nov 4 13:19:11 2009 @@ -91,16 +91,18 @@ */ public interface SingleJsoImplData { /** - * Returns a Method corresponding to the declaration of the abstract method - * in an interface type. + * Returns the method declarations that should be generated for a given + * mangled method name. {...@link #getDeclarations} and + * {...@link #getImplementations} maintain a pairwise mapping. */ - Method getDeclaration(String mangledName); + List<Method> getDeclarations(String mangledName); /** - * Return a Method corresponding to the concrete implementation of the - * method in a JSO type. + * Returns the implementations that the method declarations above should + * delegate t...@link #getDeclarations} and {...@link #getImplementations} + * maintain a pairwise mapping. */ - Method getImplementation(String mangledName); + List<Method> getImplementations(String mangledName); /** * Returns all of the mangled method names for SingleJsoImpl methods. ======================================= --- /trunk/dev/core/src/com/google/gwt/dev/shell/rewrite/RewriteSingleJsoImplDispatches.java Sun Oct 18 11:48:45 2009 +++ /trunk/dev/core/src/com/google/gwt/dev/shell/rewrite/RewriteSingleJsoImplDispatches.java Wed Nov 4 13:19:11 2009 @@ -85,7 +85,7 @@ * * void bar() { ((IB) object).foo(); } */ - for (String intf : computeAllInterfaces(owner)) { + outer : for (String intf : computeAllInterfaces(owner)) { if (jsoData.getSingleJsoIntfTypes().contains(intf)) { /* * Check that it really should be mangled and is not a reference @@ -95,23 +95,25 @@ * is undefined. */ String maybeMangled = intf.replace('/', '_') + "_" + name; - Method method = jsoData.getImplementation(maybeMangled); - if (method != null) { - /* - * Found a method with the right name, but we need to check the - * parameters and the return type. In order to do this, we'll - * look at the arguments and return type of the target method, - * removing the first argument, which is the instance. - */ - assert method.getArgumentTypes().length >= 1; - Type[] argumentTypes = new Type[method.getArgumentTypes().length - 1]; - System.arraycopy(method.getArgumentTypes(), 1, argumentTypes, - 0, argumentTypes.length); - String maybeDescriptor = Type.getMethodDescriptor( - method.getReturnType(), argumentTypes); - if (maybeDescriptor.equals(desc)) { - name = maybeMangled; - break; + List<Method> methods = jsoData.getImplementations(maybeMangled); + if (methods != null) { + for (Method method : methods) { + /* + * Found a method with the right name, but we need to check + * the parameters and the return type. In order to do this, + * we'll look at the arguments and return type of the target + * method, removing the first argument, which is the instance. + */ + assert method.getArgumentTypes().length >= 1; + Type[] argumentTypes = new Type[method.getArgumentTypes().length - 1]; + System.arraycopy(method.getArgumentTypes(), 1, argumentTypes, + 0, argumentTypes.length); + String maybeDescriptor = Type.getMethodDescriptor( + method.getReturnType(), argumentTypes); + if (maybeDescriptor.equals(desc)) { + name = maybeMangled; + break outer; + } } } } @@ -179,8 +181,10 @@ * may be referenced via a more specific interface. */ if (inSingleJsoImplInterfaceType) { - for (Map.Entry<String, Method> entry : toImplement(currentTypeName).entrySet()) { - writeEmptyMethod(entry.getKey(), entry.getValue()); + for (Map.Entry<String, List<Method>> entry : toImplement(currentTypeName).entrySet()) { + for (Method method : entry.getValue()) { + writeEmptyMethod(entry.getKey(), method); + } } } super.visitEnd(); @@ -255,14 +259,14 @@ * Given a resource name of a class, find all mangled method names that must * be implemented. */ - private SortedMap<String, Method> toImplement(String typeName) { + private SortedMap<String, List<Method>> toImplement(String typeName) { String name = typeName.replace('/', '_'); String prefix = name + "_"; String suffix = name + "`"; - SortedMap<String, Method> toReturn = new TreeMap<String, Method>(); + SortedMap<String, List<Method>> toReturn = new TreeMap<String, List<Method>>(); for (String mangledName : jsoData.getMangledNames().subSet(prefix, suffix)) { - toReturn.put(mangledName, jsoData.getImplementation(mangledName)); + toReturn.put(mangledName, jsoData.getImplementations(mangledName)); } toReturn.keySet().removeAll(implementedMethods); return toReturn; @@ -293,51 +297,52 @@ * semantics of the dispatches that would make a common implementation far * more awkward than the duplication of code. */ - for (Map.Entry<String, Method> entry : toImplement(stubIntr).entrySet()) { - String mangledName = entry.getKey(); - Method method = entry.getValue(); - - String descriptor = "(" - + method.getDescriptor().substring( - 1 + method.getArgumentTypes()[0].getDescriptor().length()); - String localName = method.getName().substring(0, - method.getName().length() - 1); - Method toCall = new Method(localName, descriptor); - - // Must not be final - MethodVisitor mv = super.visitMethod(Opcodes.ACC_PUBLIC - | Opcodes.ACC_SYNTHETIC, mangledName, descriptor, null, null); - if (mv != null) { - mv.visitCode(); - - /* - * It just so happens that the stack and local variable sizes are the - * same, but they're kept distinct to aid in clarity should the dispatch - * logic change. - * - * These start at 1 because we need to load "this" onto the stack - */ - int var = 1; - int size = 1; - - // load this - mv.visitVarInsn(Opcodes.ALOAD, 0); - - // then the rest of the arguments - for (Type t : toCall.getArgumentTypes()) { - size += t.getSize(); - mv.visitVarInsn(t.getOpcode(Opcodes.ILOAD), var); - var += t.getSize(); - } - - // Make sure there's enough room for the return value - size = Math.max(size, toCall.getReturnType().getSize()); - - mv.visitMethodInsn(Opcodes.INVOKEVIRTUAL, currentTypeName, - toCall.getName(), toCall.getDescriptor()); - mv.visitInsn(toCall.getReturnType().getOpcode(Opcodes.IRETURN)); - mv.visitMaxs(size, var); - mv.visitEnd(); + for (Map.Entry<String, List<Method>> entry : toImplement(stubIntr).entrySet()) { + for (Method method : entry.getValue()) { + String mangledName = entry.getKey(); + + String descriptor = "(" + + method.getDescriptor().substring( + 1 + method.getArgumentTypes()[0].getDescriptor().length()); + String localName = method.getName().substring(0, + method.getName().length() - 1); + Method toCall = new Method(localName, descriptor); + + // Must not be final + MethodVisitor mv = super.visitMethod(Opcodes.ACC_PUBLIC + | Opcodes.ACC_SYNTHETIC, mangledName, descriptor, null, null); + if (mv != null) { + mv.visitCode(); + + /* + * It just so happens that the stack and local variable sizes are the + * same, but they're kept distinct to aid in clarity should the + * dispatch logic change. + * + * These start at 1 because we need to load "this" onto the stack + */ + int var = 1; + int size = 1; + + // load this + mv.visitVarInsn(Opcodes.ALOAD, 0); + + // then the rest of the arguments + for (Type t : toCall.getArgumentTypes()) { + size += t.getSize(); + mv.visitVarInsn(t.getOpcode(Opcodes.ILOAD), var); + var += t.getSize(); + } + + // Make sure there's enough room for the return value + size = Math.max(size, toCall.getReturnType().getSize()); + + mv.visitMethodInsn(Opcodes.INVOKEVIRTUAL, currentTypeName, + toCall.getName(), toCall.getDescriptor()); + mv.visitInsn(toCall.getReturnType().getOpcode(Opcodes.IRETURN)); + mv.visitMaxs(size, var); + mv.visitEnd(); + } } } } ======================================= --- /trunk/dev/core/src/com/google/gwt/dev/shell/rewrite/WriteJsoImpl.java Sun Oct 18 11:48:45 2009 +++ /trunk/dev/core/src/com/google/gwt/dev/shell/rewrite/WriteJsoImpl.java Wed Nov 4 13:19:11 2009 @@ -26,6 +26,8 @@ import com.google.gwt.dev.shell.rewrite.HostedModeClassRewriter.SingleJsoImplData; import java.util.ArrayList; +import java.util.Iterator; +import java.util.List; import java.util.Set; /** @@ -90,8 +92,17 @@ // Implement the trampoline methods for (String mangledName : jsoData.getMangledNames()) { - writeTrampoline(mangledName, jsoData.getDeclaration(mangledName), - jsoData.getImplementation(mangledName)); + List<Method> declarations = jsoData.getDeclarations(mangledName); + List<Method> implementations = jsoData.getImplementations(mangledName); + assert declarations.size() == implementations.size() : "Declaration / implementation size mismatch"; + + Iterator<Method> declIterator = declarations.iterator(); + Iterator<Method> implIterator = implementations.iterator(); + + while (declIterator.hasNext()) { + assert implIterator.hasNext(); + writeTrampoline(mangledName, declIterator.next(), implIterator.next()); + } } } ======================================= --- /trunk/user/test/com/google/gwt/dev/jjs/test/SingleJsoImplTest.java Sun May 24 16:56:31 2009 +++ /trunk/user/test/com/google/gwt/dev/jjs/test/SingleJsoImplTest.java Wed Nov 4 13:19:11 2009 @@ -310,6 +310,10 @@ public String a() { return "a"; } + + public String a(boolean overload) { + return overload ? "Kaboom!" : "OK"; + } public String ex() throws IOException { throw new IOException(); @@ -403,6 +407,8 @@ interface Simple { String a(); + String a(boolean overload); + String ex() throws IOException; String rte(); @@ -589,6 +595,7 @@ assertTrue(asJso instanceof Object); assertTrue(asJso instanceof Simple); assertEquals("a", asJso.a()); + assertEquals("OK", asJso.a(false)); try { asJso.ex(); fail("Should have thrown IOException"); @@ -609,7 +616,9 @@ assertTrue(asSimple instanceof JavaScriptObject); assertTrue(asSimple instanceof JsoSimple); assertEquals("a", asSimple.a()); + assertEquals("OK", asSimple.a(false)); assertEquals("a", ((JsoSimple) asSimple).a()); + assertEquals("OK", ((JsoSimple) asSimple).a(false)); try { asSimple.ex(); fail("Should have thrown IOException"); @@ -635,6 +644,8 @@ assertTrue(asObject instanceof Simple); assertEquals("a", ((Simple) asObject).a()); assertEquals("a", ((JsoSimple) asObject).a()); + assertEquals("OK", ((Simple) asObject).a(false)); + assertEquals("OK", ((JsoSimple) asObject).a(false)); // Test a cross-cast that's normally not allowed by the type system assertTrue(asObject instanceof JsoRandom); --~--~---------~--~----~------------~-------~--~----~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~----------~----~----~----~------~----~------~--~---
