Mostly ready to sign-off, a couple comments inline, and a couple
questions.

Can you explain the high-level of why the change everywhere away from
HasEnclosingType to JNode?  Is it just code cleanup, or does it have
some functional component?

I assume also that, once the optimizations start, the expectation is
that the behavior will be the same as before?


http://gwt-code-reviews.appspot.com/1375801/diff/4002/dev/core/src/com/google/gwt/dev/jjs/ArtificialRescueRecorder.java
File dev/core/src/com/google/gwt/dev/jjs/ArtificialRescueRecorder.java
(right):

http://gwt-code-reviews.appspot.com/1375801/diff/4002/dev/core/src/com/google/gwt/dev/jjs/ArtificialRescueRecorder.java#newcode112
dev/core/src/com/google/gwt/dev/jjs/ArtificialRescueRecorder.java:112:
if (!(node instanceof JType)) {
Can you clarify this comment here?  Should it not be the other way
around (e.g. "We haven't already added the type directly, so we do here"
(in so many words)?).

http://gwt-code-reviews.appspot.com/1375801/diff/4002/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaAST.java
File dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaAST.java
(right):

http://gwt-code-reviews.appspot.com/1375801/diff/4002/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaAST.java#newcode3035
dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaAST.java:3035: }
I'm not sure I understand why we need this new case (did we handle name
refs to class literals/types differently before?)

http://gwt-code-reviews.appspot.com/1375801/diff/4002/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaAST.java#newcode3049
dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaAST.java:3049:
throw new InternalCompilerException(node,
Should the message here be updated to "...other than a field or method
or class literal"?

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

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

Reply via email to