Patch updated.
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. On 2010/06/17 15:47:31, scottb wrote:
Replace this method with a call to BinaryName.toInternalName() or
something. Done. Also removed some unused private methods. 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#newcode159 dev/core/src/com/google/gwt/dev/shell/DispatchClassInfo.java:159: if (member instanceof AccessibleObject) { On 2010/06/17 15:47:31, scottb wrote:
This should never be false... all subclasses of AccessibleObject
implement
Member.
It's false for SyntheticClassMember, which was introduced to handle JSNI references to the ::class field. 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 On 2010/06/17 15:47:31, scottb wrote:
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.
Done. 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 On 2010/06/17 15:47:31, scottb wrote:
"upcast"
Done. 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) { On 2010/06/17 15:47:31, scottb wrote:
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)
That's a better approach anyway, from the standpoint of being able to cache the rewritten bytecode in the future. The new setup uses disambiguator types based on the name of the declared array type. JsoSub1[] --> Disambiguator_JsoSub1 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" On 2010/06/17 15:47:31, scottb wrote:
Type.getDescriptor()
Done. 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)) { On 2010/06/17 15:47:31, scottb wrote:
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.
Now using reflection to pull the canonical field. 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) { On 2010/06/17 15:47:31, scottb wrote:
Just name the variable 'expected' and you can kill the comment.
Done. http://gwt-code-reviews.appspot.com/473801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
