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: /**
On 2011/06/06 22:59:05, jbrosenberg wrote:
What does "possibly winding its back" mean?
I'm not sure. I assume it means it has to go back and compile another
source unit if we don't have the type available.
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: /*
On 2011/06/06 22:59:05, jbrosenberg wrote:
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).
Updated.
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");
On 2011/06/06 22:59:05, jbrosenberg wrote:
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.
I think there is more to it than just getting rid of String. It
wouldn't be String by default. If we took another JMethodCall
constructor, we'd get a MethodCall that returned type "Object". At the
time of normalizing, all tightening is over, so I'm not sure why it
matters, but "Object" is a lot less tight than String. Since the method
will never turn any instantiated object, it just returns null. I guess
there's just no way to declare that in Java, so we override the return
type with JNullType.
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:
/*
On 2011/06/06 22:59:05, jbrosenberg wrote:
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."
Done.
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: /*
On 2011/06/06 22:59:05, jbrosenberg wrote:
"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)
Done.
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()) {
On 2011/06/06 22:59:05, jbrosenberg wrote:
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}.
Done.
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: /**
On 2011/06/06 22:59:05, jbrosenberg wrote:
Did you mean to remove "magic" here?
"magic number" actually means something, so I left it.
http://en.wikipedia.org/wiki/Magic_number_(programming)#Unnamed_numerical_constants
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: /**
On 2011/06/06 22:59:05, jbrosenberg wrote:
How about "Default value used to indicate no value was suplied..."
It really isn't a default value, it gets swapped out with something else
if not filled in. how about placeholder?
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: /**
On 2011/06/06 22:59:05, jbrosenberg wrote:
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).
Done. I was thing about this field and how it is kind of a waste of
space to set it on every object we create. I was wondering if we could
create a prototype for all JS objects that came from Java with this
field set.
http://gwt-code-reviews.appspot.com/1453804/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
