I agree with Lex about the node-building functions. While those few can be technically static, I don't see how making them so improves any dependencies anywhere. (I.E. I don't see any cases where we no longer have to pass around a JProgram). It also seems like a weird dichotomy for some of the node builders to be static and some to be instance.
Otherwise, LGTM given consideration of my two other comments. http://gwt-code-reviews.appspot.com/304801/diff/1/4 File dev/core/src/com/google/gwt/dev/jjs/impl/BuildTypeMap.java (right): http://gwt-code-reviews.appspot.com/304801/diff/1/4#newcode771 dev/core/src/com/google/gwt/dev/jjs/impl/BuildTypeMap.java:771: char[][] name = typeDeclaration.binding.compoundName; It looks like some changes here were lost. This should look like: String name = dotify(typeDeclaration.binding.compoundName); SourceTypeBinding binding = typeDeclaration.binding; if (binding instanceof LocalTypeBinding) { char[] localName = binding.constantPoolName(); if (localName == null) { /* * Weird case: if JDT determines that this local class is totally * uninstantiable, it won't bother allocating a local name. */ return false; } name = new String(localName).replace('/', '.'); } And then all of the following instances of dotify(name) are replaced with just, name. http://gwt-code-reviews.appspot.com/304801/diff/1/4#newcode888 dev/core/src/com/google/gwt/dev/jjs/impl/BuildTypeMap.java:888: static String dotify(char[][] name) { At the time I wrote this, BuildTypeMap was the only place that used dotify, so it made sense to make it a private member there. I see that there are now some new uses in GenerateJavaAST, so maybe JProgram would be a better home for it now - or maybe even a new helper class. I'm pretty ambivalent about it, but I thought I'd bring it up in case you had any opinion. http://gwt-code-reviews.appspot.com/304801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors To unsubscribe, reply using "remove me" as the subject.
