Revision: 6672 Author: b...@google.com Date: Wed Nov 4 13:43:21 2009 Log: Merge tr...@6669,6670 to fix hosted mode problems with SingleJsoImpl $ svn merge --ignore-ancestry -c6669,6670 https://google-web-toolkit.googlecode.com/svn/trunk/ . -This line, and those below, will be ignored--
M branch-info.txt M dev/core/src/com/google/gwt/dev/shell/CompilingClassLoader.java M dev/core/src/com/google/gwt/dev/shell/rewrite/HostedModeClassRewriter.java M dev/core/src/com/google/gwt/dev/shell/rewrite/WriteJsoImpl.java M dev/core/src/com/google/gwt/dev/shell/rewrite/RewriteSingleJsoImplDispatches.java M user/test/com/google/gwt/dev/jjs/test/SingleJsoImplTest.java http://code.google.com/p/google-web-toolkit/source/detail?r=6672 Modified: /releases/2.0/branch-info.txt /releases/2.0/dev/core/src/com/google/gwt/dev/shell/CompilingClassLoader.java /releases/2.0/dev/core/src/com/google/gwt/dev/shell/rewrite/HostedModeClassRewriter.java /releases/2.0/dev/core/src/com/google/gwt/dev/shell/rewrite/RewriteSingleJsoImplDispatches.java /releases/2.0/dev/core/src/com/google/gwt/dev/shell/rewrite/WriteJsoImpl.java /releases/2.0/user/test/com/google/gwt/dev/jjs/test/SingleJsoImplTest.java ======================================= --- /releases/2.0/branch-info.txt Wed Nov 4 13:11:32 2009 +++ /releases/2.0/branch-info.txt Wed Nov 4 13:43:21 2009 @@ -303,3 +303,7 @@ Increasing timeouts of RPC tests. svn merge --ignore-ancestry -c 6666 https://google-web-toolkit.googlecode.com/svn/trunk . +tr...@6669,6670 was merged into the branch + Hosted mode fixes for SingleJsoImpl + svn merge --ignore-ancestry -c6669,6670 https://google-web-toolkit.googlecode.com/svn/trunk/ . + ======================================= --- /releases/2.0/dev/core/src/com/google/gwt/dev/shell/CompilingClassLoader.java Wed Nov 4 06:59:39 2009 +++ /releases/2.0/dev/core/src/com/google/gwt/dev/shell/CompilingClassLoader.java Wed Nov 4 13:43:21 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); @@ -538,11 +539,6 @@ String mangledName = getBinaryName(type).replace('.', '_') + "_" + intfMethod.getName(); mangledNames.add(mangledName); - - JType[] parameterTypes = new JType[intfMethod.getParameters().length]; - for (int i = 0; i < parameterTypes.length; i++) { - parameterTypes[i] = intfMethod.getParameters()[i].getType(); - } /* * Handle virtual overrides by finding the method that we would @@ -550,12 +546,12 @@ * target. */ JMethod implementingMethod; - while ((implementingMethod = implementingType.findMethod( - intfMethod.getName(), parameterTypes)) == null) { + while ((implementingMethod = findOverloadUsingErasure( + implementingType, intfMethod)) == null) { implementingType = implementingType.getSuperclass(); } // implementingmethod and implementingType cannot be null here - + /* * Create a pseudo-method declaration for the interface method. This * should look something like @@ -565,16 +561,16 @@ * This must be kept in sync with the WriteJsoImpl class. */ { - String decl = getBinaryOrPrimitiveName(intfMethod.getReturnType()) + String decl = getBinaryOrPrimitiveName(intfMethod.getReturnType().getErasedType()) + " " + intfMethod.getName() + "("; - for (JType paramType : parameterTypes) { + for (JParameter param : intfMethod.getParameters()) { decl += ","; - decl += getBinaryOrPrimitiveName(paramType); + decl += getBinaryOrPrimitiveName(param.getType().getErasedType()); } 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); } /* @@ -586,19 +582,19 @@ * This must be kept in sync with the WriteJsoImpl class. */ { - String returnName = getBinaryOrPrimitiveName(implementingMethod.getReturnType()); + String returnName = getBinaryOrPrimitiveName(implementingMethod.getReturnType().getErasedType()); String jsoName = getBinaryOrPrimitiveName(implementingType); String decl = returnName + " " + intfMethod.getName() + "$ (" + jsoName; - for (JType paramType : parameterTypes) { + for (JParameter param : implementingMethod.getParameters()) { decl += ","; - decl += getBinaryOrPrimitiveName(paramType); + decl += getBinaryOrPrimitiveName(param.getType().getErasedType()); } 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 +602,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 +628,49 @@ 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); + } + } + } + + /** + * Looks for a concrete implementation of <code>intfMethod</code> in + * <code>implementingType</code>. + */ + private JMethod findOverloadUsingErasure(JClassType implementingType, + JMethod intfMethod) { + + int numParams = intfMethod.getParameters().length; + JType[] erasedTypes = new JType[numParams]; + for (int i = 0; i < numParams; i++) { + erasedTypes[i] = intfMethod.getParameters()[i].getType().getErasedType(); + } + + outer : for (JMethod method : implementingType.getOverloads(intfMethod.getName())) { + JParameter[] params = method.getParameters(); + if (params.length != numParams) { + continue; + } + for (int i = 0; i < numParams; i++) { + if (params[i].getType().getErasedType() != erasedTypes[i]) { + continue outer; + } + } + return method; + } + return null; + } } /** ======================================= --- /releases/2.0/dev/core/src/com/google/gwt/dev/shell/rewrite/HostedModeClassRewriter.java Sun Oct 18 11:48:45 2009 +++ /releases/2.0/dev/core/src/com/google/gwt/dev/shell/rewrite/HostedModeClassRewriter.java Wed Nov 4 13:43:21 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. ======================================= --- /releases/2.0/dev/core/src/com/google/gwt/dev/shell/rewrite/RewriteSingleJsoImplDispatches.java Sun Oct 18 11:48:45 2009 +++ /releases/2.0/dev/core/src/com/google/gwt/dev/shell/rewrite/RewriteSingleJsoImplDispatches.java Wed Nov 4 13:43:21 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(); + } } } } ======================================= --- /releases/2.0/dev/core/src/com/google/gwt/dev/shell/rewrite/WriteJsoImpl.java Sun Oct 18 11:48:45 2009 +++ /releases/2.0/dev/core/src/com/google/gwt/dev/shell/rewrite/WriteJsoImpl.java Wed Nov 4 13:43:21 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()); + } } } ======================================= --- /releases/2.0/user/test/com/google/gwt/dev/jjs/test/SingleJsoImplTest.java Sun May 24 16:56:31 2009 +++ /releases/2.0/user/test/com/google/gwt/dev/jjs/test/SingleJsoImplTest.java Wed Nov 4 13:43:21 2009 @@ -19,6 +19,7 @@ import com.google.gwt.dev.jjs.test.SingleJsoImplTest.JsoHasInnerJsoType.InnerType; import com.google.gwt.dev.jjs.test.jsointfs.JsoInterfaceWithUnreferencedImpl; import com.google.gwt.junit.client.GWTTestCase; +import com.google.gwt.user.client.rpc.AsyncCallback; import java.io.IOException; @@ -310,6 +311,10 @@ public String a() { return "a"; } + + public String a(boolean overload) { + return overload ? "Kaboom!" : "OK"; + } public String ex() throws IOException { throw new IOException(); @@ -373,6 +378,32 @@ return new String[1]; } } + + static class JsoUsesGeneric extends JavaScriptObject implements UsesGenerics { + public static native <T extends CharSequence> JsoUsesGeneric create() /*-{ + return {suffix : "42"}; + }-*/; + + protected JsoUsesGeneric() { + } + + public final native <T> String acceptsGeneric(T chars) /*-{ + return chars + this.suffix; + }-*/; + + public final native <T> void callback(AsyncCallback<T> callback, T chars) /*-{ + callba...@com.google.gwt.user.client.rpc.asynccallback::onSuccess(Ljava/lang/Object;)(chars + this.suffix); + }-*/; + + /** + * What you're seeing here is would be achieved by an unsafe (T) cast and + * would break with a ClassCastException if accessed via JsoIsGenericFinal + * in normal Java. + */ + public final native <T> T returnsGeneric(String chars) /*-{ + return chars + this.suffix; + }-*/; + } /** * Ensure that SingleJsoImpl interfaces can be extended and implemented by @@ -403,6 +434,8 @@ interface Simple { String a(); + String a(boolean overload); + String ex() throws IOException; String rte(); @@ -420,7 +453,6 @@ interface SimpleOnlyJavaInterface { String simpleOnlyJava(); } - interface Tag { } @@ -452,6 +484,14 @@ String[] returnStringArray(); } + + interface UsesGenerics { + <T extends Object> String acceptsGeneric(T chars); + + <T> void callback(AsyncCallback<T> callback, T chars); + + <T> T returnsGeneric(String chars); + } private static native JsoAdder makeAdder(int offset) /*-{ return {offset:offset}; @@ -581,6 +621,20 @@ assertSame(Adder.SAME_OBJECT, Adder.SAME_OBJECT2); assertNotSame(Adder.DIFFERENT_OBJECT, Adder.SAME_OBJECT); } + + public void testGenerics() { + UsesGenerics j = JsoUsesGeneric.create(); + assertEquals("Hello42", j.acceptsGeneric("Hello")); + assertEquals("Hello42", j.returnsGeneric("Hello")); + j.callback(new AsyncCallback<CharSequence>() { + public void onFailure(Throwable caught) { + } + + public void onSuccess(CharSequence result) { + assertEquals("Hello42", result); + } + }, "Hello"); + } @SuppressWarnings("cast") public void testSimpleCase() { @@ -589,6 +643,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 +664,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 +692,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 -~----------~----~----~----~------~----~------~--~---