Mostly LG, just some nits and food-for-thought.
http://gwt-code-reviews.appspot.com/473801/diff/78001/79002 File dev/core/src/com/google/gwt/dev/shell/CompilingClassLoader.java (right): http://gwt-code-reviews.appspot.com/473801/diff/78001/79002#newcode695 dev/core/src/com/google/gwt/dev/shell/CompilingClassLoader.java:695: * Convert a binary class name into a resource-like name. Replace this method with a call to BinaryName.toInternalName() or something. http://gwt-code-reviews.appspot.com/473801/diff/78001/79003 File dev/core/src/com/google/gwt/dev/shell/DispatchClassInfo.java (right): http://gwt-code-reviews.appspot.com/473801/diff/78001/79003#newcode21 dev/core/src/com/google/gwt/dev/shell/DispatchClassInfo.java:21: import java.lang.annotation.Annotation; Unused. http://gwt-code-reviews.appspot.com/473801/diff/78001/79003#newcode159 dev/core/src/com/google/gwt/dev/shell/DispatchClassInfo.java:159: if (member instanceof AccessibleObject) { This should never be false... all subclasses of AccessibleObject implement Member. http://gwt-code-reviews.appspot.com/473801/diff/78001/79005 File dev/core/src/com/google/gwt/dev/shell/rewrite/DebugAnalyzerAdapter.java (right): http://gwt-code-reviews.appspot.com/473801/diff/78001/79005#newcode96 dev/core/src/com/google/gwt/dev/shell/rewrite/DebugAnalyzerAdapter.java:96: private final Map<Label, String> debugData = EXTRA_DEBUG_DATA I think we should explicitly type this as LinkedHashMap consistently throughout this file, to make the iteration semantics obvious. I know we haven't done that elsewhere, but I think we should transition to this pattern when it's actually important that the iteration is in order. Otherwise, a reader has to go digging to find instantiation sites to be sure an iteration over map is safe. http://gwt-code-reviews.appspot.com/473801/diff/78001/79006 File dev/core/src/com/google/gwt/dev/shell/rewrite/HostedModeClassRewriter.java (right): http://gwt-code-reviews.appspot.com/473801/diff/78001/79006#newcode126 dev/core/src/com/google/gwt/dev/shell/rewrite/HostedModeClassRewriter.java:126: * In order to be able to upcost all JSO subtype arrays to JavaScriptObject "upcast" http://gwt-code-reviews.appspot.com/473801/diff/78001/79006#newcode139 dev/core/src/com/google/gwt/dev/shell/rewrite/HostedModeClassRewriter.java:139: public int getConstructorDisambiguator(String owner, String desc) { General feedback.. it seems like this will create a lot more types than necessary. In reality, it feels like you only ever need one disambiguation type per JSOSubclass[] type. In other words... foo(JsoSub1[]) -> foo(Jso[], DisAmb1) foo(JsoSub2[]) -> foo(Jso[], DisAmb2) foo(JsoSub1[],JsoSub2[]) -> foo(Jso[],Jso[],DisAmb1,DisAmb2) http://gwt-code-reviews.appspot.com/473801/diff/78001/79008 File dev/core/src/com/google/gwt/dev/shell/rewrite/RewriteJsoArrays.java (right): http://gwt-code-reviews.appspot.com/473801/diff/78001/79008#newcode40 dev/core/src/com/google/gwt/dev/shell/rewrite/RewriteJsoArrays.java:40: private static final String ORIGINAL_JSNI_SIGNATURE_DESC = "L" Type.getDescriptor() http://gwt-code-reviews.appspot.com/473801/diff/78001/79013 File dev/core/src/com/google/gwt/dev/shell/rewrite/SingleJsoImplSupport.java (right): http://gwt-code-reviews.appspot.com/473801/diff/78001/79013#newcode94 dev/core/src/com/google/gwt/dev/shell/rewrite/SingleJsoImplSupport.java:94: if (getJsoClass(o).isInstance(o)) { Isn't there a faster way to do this? Can't we just deref the field? I stepped through this and it seems slower than it has to be. http://gwt-code-reviews.appspot.com/473801/diff/78001/79017 File user/test/com/google/gwt/dev/jjs/test/JsoTest.java (right): http://gwt-code-reviews.appspot.com/473801/diff/78001/79017#newcode332 user/test/com/google/gwt/dev/jjs/test/JsoTest.java:332: } catch (ArrayStoreException e) { Just name the variable 'expected' and you can kill the comment. http://gwt-code-reviews.appspot.com/473801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
