http://gwt-code-reviews.appspot.com/1373805/diff/1/dev/core/src/com/google/gwt/dev/CompileModule.java
File dev/core/src/com/google/gwt/dev/CompileModule.java (right):

http://gwt-code-reviews.appspot.com/1373805/diff/1/dev/core/src/com/google/gwt/dev/CompileModule.java#newcode2
dev/core/src/com/google/gwt/dev/CompileModule.java:2: * Copyright 2010
Google Inc.
date

http://gwt-code-reviews.appspot.com/1373805/diff/1/dev/core/src/com/google/gwt/dev/CompileModule.java#newcode61
dev/core/src/com/google/gwt/dev/CompileModule.java:61: * Compiles a GWT
module.
Right now, this class looks like a unit test...  Is that what it will
remain or is this just the current state in a mid-stream review?

http://gwt-code-reviews.appspot.com/1373805/diff/1/dev/core/src/com/google/gwt/dev/jjs/impl/GwtAstBuilder.java
File dev/core/src/com/google/gwt/dev/jjs/impl/GwtAstBuilder.java
(right):

http://gwt-code-reviews.appspot.com/1373805/diff/1/dev/core/src/com/google/gwt/dev/jjs/impl/GwtAstBuilder.java#newcode213
dev/core/src/com/google/gwt/dev/jjs/impl/GwtAstBuilder.java:213: *
contain unresolved references.
Looks like this whole file is a refactoring of GenerateJavaAST?   How
long are we planning to live with 2 copies?

Can you give us any pointers as to what your design goals are? I see you
moved a lot of the AST construction from processXXX to endVisit methods.
 What's the motivation there?

http://gwt-code-reviews.appspot.com/1373805/diff/1/dev/core/src/com/google/gwt/dev/jjs/impl/GwtAstBuilder.java#newcode258
dev/core/src/com/google/gwt/dev/jjs/impl/GwtAstBuilder.java:258: if
(field.isCompileTimeConstant()) {
why did you remove assert() from here?  Just old  debugging?

http://gwt-code-reviews.appspot.com/1373805/diff/1/dev/core/src/com/google/gwt/dev/jjs/impl/GwtAstBuilder.java#newcode354
dev/core/src/com/google/gwt/dev/jjs/impl/GwtAstBuilder.java:354: --i;
um, this is weird.  change the for expression or use a while loop.

http://gwt-code-reviews.appspot.com/1373805/diff/1/dev/core/src/com/google/gwt/dev/jjs/impl/GwtAstBuilder.java#newcode415
dev/core/src/com/google/gwt/dev/jjs/impl/GwtAstBuilder.java:415: int
binOp = (x.bits & ASTNode.OperatorMASK) >> ASTNode.OperatorSHIFT;
if we are going to maintain 2 version of the AST build, methods like
these would be a good candidate to move to common code.

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

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

Reply via email to