Mostly nits.
http://gwt-code-reviews.appspot.com/34822/diff/1/2 File dev/core/src/com/google/gwt/dev/jjs/ast/JClassLiteral.java (right): http://gwt-code-reviews.appspot.com/34822/diff/1/2#newcode68 Line 68: if (type instanceof JClassType && !(type instanceof JArrayType)) { JArrayType no longer extends JClassType, so the second test is not necessary. http://gwt-code-reviews.appspot.com/34822/diff/1/3 File dev/core/src/com/google/gwt/dev/jjs/ast/JNameOf.java (right): http://gwt-code-reviews.appspot.com/34822/diff/1/3#newcode48 Line 48: visitor.visit(this, ctx); Do: if (visitor.visit(this, ctx)) { // Intentionally not visiting referenced node } That way, if something we want to visit in the future gets added, we won't forget to conditionally visit based on the result of the visit() call. http://gwt-code-reviews.appspot.com/34822/diff/1/7 File dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java (right): http://gwt-code-reviews.appspot.com/34822/diff/1/7#newcode1044 Line 1044: push(jsProgram.getNullLiteral()); Is this a valid state? Does this actually happen in practice? http://gwt-code-reviews.appspot.com/34822/diff/1/9 File dev/core/src/com/google/gwt/dev/jjs/impl/ToStringGenerationVisitor.java (right): http://gwt-code-reviews.appspot.com/34822/diff/1/9#newcode635 Line 635: printStringLiteral(x.getNode().getName()); More information might be useful for debugging, to distinguish this node from an actual string literal. But your call. http://gwt-code-reviews.appspot.com/34822/diff/1/12 File dev/core/src/com/google/gwt/dev/js/ast/JsNameOf.java (right): http://gwt-code-reviews.appspot.com/34822/diff/1/12#newcode56 Line 56: visitor.visit(this, ctx); Same here, do: if (visitor.visit(this, ctx)) { } http://gwt-code-reviews.appspot.com/34822/diff/1/14 File user/super/com/google/gwt/emul/java/lang/Class.java (right): http://gwt-code-reviews.appspot.com/34822/diff/1/14#newcode107 Line 107: static void setName(Class<?> clazz, String packageName, String className, Can you confirm this inlines/optimizes correctly in all cases? I notice on the second fork that the arg evaluation is non-linear. http://gwt-code-reviews.appspot.com/34822/diff/1/14#newcode151 Line 151: }-*/; Diff cheese. http://gwt-code-reviews.appspot.com/34822/diff/1/14#newcode176 Line 176: public boolean isClassMetadataEnabled() { Why is this public? Should be private and static, right? http://gwt-code-reviews.appspot.com/34822 --~--~---------~--~----~------------~-------~--~----~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~----------~----~----~----~------~----~------~--~---
