LGTM, but it'd be good if you could add some comments to record some of
the ideas we discussed.


http://gwt-code-reviews.appspot.com/1428808/diff/1/dev/core/test/com/google/gwt/dev/jjs/impl/EnumOrdinalizerTest.java
File dev/core/test/com/google/gwt/dev/jjs/impl/EnumOrdinalizerTest.java
(right):

http://gwt-code-reviews.appspot.com/1428808/diff/1/dev/core/test/com/google/gwt/dev/jjs/impl/EnumOrdinalizerTest.java#newcode273
dev/core/test/com/google/gwt/dev/jjs/impl/EnumOrdinalizerTest.java:273:
"String y = fruit.staticField;");
Thing is, I don't believe the generated code actually checks the NPE.
It only evaluates 'fruit' for side effects, and 'fruit' would eventually
get optimized out with a full optimization.  You can't simply convert to
Fruit because it could be a side-effecty qualifier like
maybeGetFruit().staticField.

http://gwt-code-reviews.appspot.com/1428808/diff/1/dev/core/test/com/google/gwt/dev/jjs/impl/EnumOrdinalizerTest.java#newcode456
dev/core/test/com/google/gwt/dev/jjs/impl/EnumOrdinalizerTest.java:456:
"}");
Okay, looking at this formulation, it makes me question that this should
even blacklist Fruit.  As long as CastNormalizer does something smart, I
can't imagine that this instanceof would necessitate blacklisting: if
the test could possibly succeed, it would have implied an upcast
elsewhere.  Maybe CastNormalizer could treat instanceof OrdinalizedEnum
specially and always make the result trivially true or trivially false?

http://gwt-code-reviews.appspot.com/1428808/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

Reply via email to