Ready for another look. By adding an actual @SingleJsoImpl annotation to act as a hint for devmode, the impact on I18N's CurrencyData is pretty much a one-liner.
http://gwt-code-reviews.appspot.com/473801/diff/5001/6004 File dev/core/src/com/google/gwt/dev/shell/JsValueGlue.java (right): http://gwt-code-reviews.appspot.com/473801/diff/5001/6004#newcode63 dev/core/src/com/google/gwt/dev/shell/JsValueGlue.java:63: if (desiredType != null && !desiredType.isAssignableFrom(jsoType)) { Done. http://gwt-code-reviews.appspot.com/473801/diff/5001/6005 File dev/core/src/com/google/gwt/dev/shell/rewrite/HostedModeClassRewriter.java (right): http://gwt-code-reviews.appspot.com/473801/diff/5001/6005#newcode46 dev/core/src/com/google/gwt/dev/shell/rewrite/HostedModeClassRewriter.java:46: * instantiable type.</li> Done. http://gwt-code-reviews.appspot.com/473801/diff/5001/6005#newcode60 dev/core/src/com/google/gwt/dev/shell/rewrite/HostedModeClassRewriter.java:60: static class RewriterOracle { Precisely. http://gwt-code-reviews.appspot.com/473801/diff/5001/6006 File dev/core/src/com/google/gwt/dev/shell/rewrite/RewriteJsoCasts.java (right): http://gwt-code-reviews.appspot.com/473801/diff/5001/6006#newcode57 dev/core/src/com/google/gwt/dev/shell/rewrite/RewriteJsoCasts.java:57: private class MyMethodAdapter extends AnalyzerAdapter { Addressed these issues and added tests. http://gwt-code-reviews.appspot.com/473801/diff/5001/6006#newcode128 dev/core/src/com/google/gwt/dev/shell/rewrite/RewriteJsoCasts.java:128: && rewriterOracle.isJsoSubtype(t.getInternalName())) { Done. http://gwt-code-reviews.appspot.com/473801/diff/5001/6006#newcode136 dev/core/src/com/google/gwt/dev/shell/rewrite/RewriteJsoCasts.java:136: && rewriterOracle.isJsoSubtype(t.getElementType().getInternalName())) { Wouldn't this get weird with dispatching to methods that want a JSO array? We can't just erase all subtypes, since that would break method overloading. http://gwt-code-reviews.appspot.com/473801/diff/5001/6006#newcode148 dev/core/src/com/google/gwt/dev/shell/rewrite/RewriteJsoCasts.java:148: Method m = new Method(name, desc); Done. http://gwt-code-reviews.appspot.com/473801/diff/5001/6006#newcode272 dev/core/src/com/google/gwt/dev/shell/rewrite/RewriteJsoCasts.java:272: * Casts to Object or JavaScriptObject should result in the canonical object Changed so that only Object is canonical. http://gwt-code-reviews.appspot.com/473801/diff/5001/6006#newcode356 dev/core/src/com/google/gwt/dev/shell/rewrite/RewriteJsoCasts.java:356: } else if (rewriterOracle.isJsoSubtype(internalName)) { Renamed to isJsoOrSubtype(). http://gwt-code-reviews.appspot.com/473801/diff/5001/6007 File dev/core/src/com/google/gwt/dev/shell/rewrite/RewriteObjectComparisons.java (right): http://gwt-code-reviews.appspot.com/473801/diff/5001/6007#newcode39 dev/core/src/com/google/gwt/dev/shell/rewrite/RewriteObjectComparisons.java:39: public void visitJumpInsn(int opcode, Label label) { Done. http://gwt-code-reviews.appspot.com/473801/diff/5001/6010 File dev/core/src/com/google/gwt/dev/shell/rewrite/SingleJsoImplSupport.java (right): http://gwt-code-reviews.appspot.com/473801/diff/5001/6010#newcode46 dev/core/src/com/google/gwt/dev/shell/rewrite/SingleJsoImplSupport.java:46: if (o == null) { Done. http://gwt-code-reviews.appspot.com/473801/diff/5001/6010#newcode50 dev/core/src/com/google/gwt/dev/shell/rewrite/SingleJsoImplSupport.java:50: if (Object.class == jsoOrIntfType) { Done. http://gwt-code-reviews.appspot.com/473801/diff/5001/6010#newcode54 dev/core/src/com/google/gwt/dev/shell/rewrite/SingleJsoImplSupport.java:54: if (jsoOrIntfType.isInstance(o)) { Done. http://gwt-code-reviews.appspot.com/473801/diff/5001/6010#newcode63 dev/core/src/com/google/gwt/dev/shell/rewrite/SingleJsoImplSupport.java:63: return cast(o, jsoOrIntfType, targetJsoType); Done. http://gwt-code-reviews.appspot.com/473801/diff/5001/6010#newcode97 dev/core/src/com/google/gwt/dev/shell/rewrite/SingleJsoImplSupport.java:97: throw new ClassCastException( On 2010/05/21 23:00:04, scottb wrote:
Can't you just return and let the caller throw the CCE?
The CCE describes a more specific error situation than the callsite would describe. It's this error case that makes me want to re-add the optional @SingleJsoImpl annotation to prime the implementation type. http://gwt-code-reviews.appspot.com/473801/diff/5001/6010#newcode121 dev/core/src/com/google/gwt/dev/shell/rewrite/SingleJsoImplSupport.java:121: assert intf != null : "No interface type for instanceof check"; Done. http://gwt-code-reviews.appspot.com/473801/diff/5001/6011 File dev/core/src/com/google/gwt/dev/shell/rewrite/WriteJsoImpl.java (right): http://gwt-code-reviews.appspot.com/473801/diff/5001/6011#newcode45 dev/core/src/com/google/gwt/dev/shell/rewrite/WriteJsoImpl.java:45: * <li>The zero-arg constructor is made public and makes the JavaScriptObject Done. http://gwt-code-reviews.appspot.com/473801/diff/5001/6011#newcode92 dev/core/src/com/google/gwt/dev/shell/rewrite/WriteJsoImpl.java:92: // Write the zero-arg constructor Done. http://gwt-code-reviews.appspot.com/473801/diff/5001/6011#newcode105 dev/core/src/com/google/gwt/dev/shell/rewrite/WriteJsoImpl.java:105: mv.visitVarInsn(Opcodes.ALOAD, 0); Done. http://gwt-code-reviews.appspot.com/473801/diff/5001/6011#newcode118 dev/core/src/com/google/gwt/dev/shell/rewrite/WriteJsoImpl.java:118: desc = "(L" + JAVASCRIPTOBJECT_DESC + ";)V"; Done. http://gwt-code-reviews.appspot.com/473801/diff/5001/6011#newcode244 dev/core/src/com/google/gwt/dev/shell/rewrite/WriteJsoImpl.java:244: // make the constructor public Done. http://gwt-code-reviews.appspot.com/473801/diff/5001/6011#newcode267 dev/core/src/com/google/gwt/dev/shell/rewrite/WriteJsoImpl.java:267: mv.visitLocalVariable("this", "L" + getOriginalName() + ";", null, It's just for debugging. It helped me to be able to chase down some CCE's. http://gwt-code-reviews.appspot.com/473801/diff/5001/6011#newcode282 dev/core/src/com/google/gwt/dev/shell/rewrite/WriteJsoImpl.java:282: * public static JsoSubclass rewrap$(JavaScriptObject jso) { The extra wrapper is now avoided. http://gwt-code-reviews.appspot.com/473801/diff/5001/6011#newcode286 dev/core/src/com/google/gwt/dev/shell/rewrite/WriteJsoImpl.java:286: * JavaScriptObjet canonical = jso.canonical; Done. http://gwt-code-reviews.appspot.com/473801/diff/5001/6011#newcode297 dev/core/src/com/google/gwt/dev/shell/rewrite/WriteJsoImpl.java:297: mv.visitVarInsn(Opcodes.ALOAD, 0); I need to push a null onto the stack or do a checkcast in order for the verifier to allow the JavaScriptObject parameter to be returned as the JsoSubclass return type. http://gwt-code-reviews.appspot.com/473801/diff/5001/6011#newcode375 dev/core/src/com/google/gwt/dev/shell/rewrite/WriteJsoImpl.java:375: private WriteJsoImpl(ClassVisitor cv) { Done. http://gwt-code-reviews.appspot.com/473801/diff/5001/6011#newcode397 dev/core/src/com/google/gwt/dev/shell/rewrite/WriteJsoImpl.java:397: protected String getOriginalName() { Done. http://gwt-code-reviews.appspot.com/473801/diff/5001/6012 File dev/core/src/com/google/gwt/dev/shell/rewrite/WriteSingleJsoSupportCode.java (right): http://gwt-code-reviews.appspot.com/473801/diff/5001/6012#newcode64 dev/core/src/com/google/gwt/dev/shell/rewrite/WriteSingleJsoSupportCode.java:64: * o = JavaScriptObject.rewrap$((JavaScriptObject o); Done. http://gwt-code-reviews.appspot.com/473801/diff/5001/6012#newcode115 dev/core/src/com/google/gwt/dev/shell/rewrite/WriteSingleJsoSupportCode.java:115: toInternalName(SingleJsoImplSupport.class.getName()), "cast", Done. http://gwt-code-reviews.appspot.com/473801/diff/5001/6012#newcode135 dev/core/src/com/google/gwt/dev/shell/rewrite/WriteSingleJsoSupportCode.java:135: mv.visitCode(); Done. http://gwt-code-reviews.appspot.com/473801/diff/5001/6012#newcode158 dev/core/src/com/google/gwt/dev/shell/rewrite/WriteSingleJsoSupportCode.java:158: new Object[0]); Done. http://gwt-code-reviews.appspot.com/473801/diff/5001/6012#newcode242 dev/core/src/com/google/gwt/dev/shell/rewrite/WriteSingleJsoSupportCode.java:242: mv.visitFrame(Opcodes.F_NEW, 0, new Object[0], 0, new Object[0]); The initial frame isn't necessary. I think this was a copy-and-paste job at one point. Removed. http://gwt-code-reviews.appspot.com/473801/diff/5001/6012#newcode289 dev/core/src/com/google/gwt/dev/shell/rewrite/WriteSingleJsoSupportCode.java:289: * its interfaces. Done. http://gwt-code-reviews.appspot.com/473801/diff/5001/6015 File user/src/com/google/gwt/user/client/DOM.java (right): http://gwt-code-reviews.appspot.com/473801/diff/5001/6015#newcode750 user/src/com/google/gwt/user/client/DOM.java:750: return (Element) Document.get().getElementById(id); It's possible that getElementById() returns null, so the call to cast() would throw an NPE. I suspect there's probably a good bit of code out there that inadvertently makes use of the fact that calling instance methods on a null JSO doesn't fail. This code will start breaking unless some additional compatibility dispatch logic is added. http://gwt-code-reviews.appspot.com/473801/diff/5001/6016 File user/src/com/google/gwt/user/client/ui/RootPanel.java (right): http://gwt-code-reviews.appspot.com/473801/diff/5001/6016#newcode286 user/src/com/google/gwt/user/client/ui/RootPanel.java:286: element = element.getParentElement(); Yes. http://gwt-code-reviews.appspot.com/473801/diff/5001/6019 File user/test/com/google/gwt/dev/jjs/test/SingleJsoImplTest.java (right): http://gwt-code-reviews.appspot.com/473801/diff/5001/6019#newcode960 user/test/com/google/gwt/dev/jjs/test/SingleJsoImplTest.java:960: public void XXXtestCreatedWithCast() { I've introduced an optional @SingleJsoIImpl() annotation to make this use-case work. It also serves as a sanity-check for library writers to ensure that consumers don't attempt to implement their own JSO version of the interface. This could help in the design of lightweight collections for GWT, if the public-facing API is mostly interface-based. If you don't like the annotation or want to discuss it separately from this patch, I can rebase my client to pull it out. http://gwt-code-reviews.appspot.com/473801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
