http://gwt-code-reviews.appspot.com/1453804/diff/1017/dev/core/src/com/google/gwt/dev/jdt/AbstractCompiler.java File dev/core/src/com/google/gwt/dev/jdt/AbstractCompiler.java (right):
http://gwt-code-reviews.appspot.com/1453804/diff/1017/dev/core/src/com/google/gwt/dev/jdt/AbstractCompiler.java#newcode268 dev/core/src/com/google/gwt/dev/jdt/AbstractCompiler.java:268: /** What does "possibly winding its back" mean? http://gwt-code-reviews.appspot.com/1453804/diff/1017/dev/core/src/com/google/gwt/dev/jjs/impl/CastNormalizer.java File dev/core/src/com/google/gwt/dev/jjs/impl/CastNormalizer.java (right): http://gwt-code-reviews.appspot.com/1453804/diff/1017/dev/core/src/com/google/gwt/dev/jjs/impl/CastNormalizer.java#newcode434 dev/core/src/com/google/gwt/dev/jjs/impl/CastNormalizer.java:434: /* I like "Hack" less than "Magic". How about neither? Perhaps reference the previous code pass that inserted this null type cast, and then also reference Cast.throwClassCastExceptionUnlessNull. "If we have a null cast here, it indicates that the user tried a cast that couldn't possibly work (@see TypeTightener). Typically... <newline> We handle this by replacing the cast operation with a method call that will throw a ClassCastException, unless the argument is null. (@see Cast.throwClassCastExceptionUnlessNull). http://gwt-code-reviews.appspot.com/1453804/diff/1017/dev/core/src/com/google/gwt/dev/jjs/impl/CastNormalizer.java#newcode442 dev/core/src/com/google/gwt/dev/jjs/impl/CastNormalizer.java:442: JMethod method = program.getIndexedMethod("Cast.throwClassCastExceptionUnlessNull"); Is that the reason for overriding the type to null? I think it's more that we've replaced one expression with another, but it still needs to ensure the same semantics. So, casting a null object to any type still results in an expression of type JNullType. Object x = null; Object y = (String)x; This should assign null to y (and not throw an exception). So when it is replaced with: Object x = null; Object y = throwClassCastExceptionUnlessNull(x); it still needs to set y to null, and not to a value of type String. How about this comment instead: // override the type of the replacement method to the null type. http://gwt-code-reviews.appspot.com/1453804/diff/1017/dev/core/src/com/google/gwt/dev/jjs/impl/CastNormalizer.java#newcode472 dev/core/src/com/google/gwt/dev/jjs/impl/CastNormalizer.java:472: } This is the companion comment to the one I'm suggesting above http://gwt-code-reviews.appspot.com/1453804/diff/1017/dev/core/src/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzer.java File dev/core/src/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzer.java (right): http://gwt-code-reviews.appspot.com/1453804/diff/1017/dev/core/src/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzer.java#newcode370 dev/core/src/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzer.java:370: /* I don't think this is a "warning", is it? It should be an explanation. How about: "In order to preserve code-splitting, we don't allow code from the...." and then add something like: "The RunAsync.onSuccess() overridden methods are visited instead as top-level entry points for the purposes of control flow analysis." http://gwt-code-reviews.appspot.com/1453804/diff/1017/dev/core/src/com/google/gwt/dev/jjs/impl/ReplaceRunAsyncs.java File dev/core/src/com/google/gwt/dev/jjs/impl/ReplaceRunAsyncs.java (right): http://gwt-code-reviews.appspot.com/1453804/diff/1017/dev/core/src/com/google/gwt/dev/jjs/impl/ReplaceRunAsyncs.java#newcode65 dev/core/src/com/google/gwt/dev/jjs/impl/ReplaceRunAsyncs.java:65: /* "We don't optimize calls from AsyncFragmentLoader to runAsyncCallBack.onSuccess(), since this can defeat code splitting." (I don't think that last added part quite makes sense in a general way) http://gwt-code-reviews.appspot.com/1453804/diff/1017/dev/core/src/com/google/gwt/dev/jjs/impl/TypeTightener.java File dev/core/src/com/google/gwt/dev/jjs/impl/TypeTightener.java (right): http://gwt-code-reviews.appspot.com/1453804/diff/1017/dev/core/src/com/google/gwt/dev/jjs/impl/TypeTightener.java#newcode371 dev/core/src/com/google/gwt/dev/jjs/impl/TypeTightener.java:371: } else if (triviallyFalse && toType != program.getTypeNull()) { How about: This case requires special handling, since it is guaranteed to force a ClassCastException, except in the case of the cast expression evaluates to null. We insert a place-holder cast to the null type here, for subsequent handling in {@link CastNormalizer}. http://gwt-code-reviews.appspot.com/1453804/diff/1017/dev/core/test/com/google/gwt/dev/jjs/JavaAstConstructor.java File dev/core/test/com/google/gwt/dev/jjs/JavaAstConstructor.java (right): http://gwt-code-reviews.appspot.com/1453804/diff/1017/dev/core/test/com/google/gwt/dev/jjs/JavaAstConstructor.java#newcode238 dev/core/test/com/google/gwt/dev/jjs/JavaAstConstructor.java:238: /* Will these always be "re-written"? http://gwt-code-reviews.appspot.com/1453804/diff/1017/dev/core/test/com/google/gwt/lang/LongLibTestBase.java File dev/core/test/com/google/gwt/lang/LongLibTestBase.java (right): http://gwt-code-reviews.appspot.com/1453804/diff/1017/dev/core/test/com/google/gwt/lang/LongLibTestBase.java#newcode23 dev/core/test/com/google/gwt/lang/LongLibTestBase.java:23: /** Did you mean to remove "magic" here? http://gwt-code-reviews.appspot.com/1453804/diff/1017/user/src/com/google/gwt/i18n/client/LocalizableResource.java File user/src/com/google/gwt/i18n/client/LocalizableResource.java (right): http://gwt-code-reviews.appspot.com/1453804/diff/1017/user/src/com/google/gwt/i18n/client/LocalizableResource.java#newcode88 user/src/com/google/gwt/i18n/client/LocalizableResource.java:88: /** How about "Default value used to indicate no value was suplied..." http://gwt-code-reviews.appspot.com/1453804/diff/1017/user/super/com/google/gwt/emul/java/lang/Object.java File user/super/com/google/gwt/emul/java/lang/Object.java (right): http://gwt-code-reviews.appspot.com/1453804/diff/1017/user/super/com/google/gwt/emul/java/lang/Object.java#newcode45 user/super/com/google/gwt/emul/java/lang/Object.java:45: /** A special marker field used internally to the GWT compiler. For example, it is used for distinguishing whether an object is a Java object or a JavaScriptObject (@see com.google.gwt.lang.Cast). http://gwt-code-reviews.appspot.com/1453804/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
