LGTM, aside from "magic"
http://gwt-code-reviews.appspot.com/1451804/diff/1/dev/core/src/com/google/gwt/dev/jjs/ast/JGwtCreate.java File dev/core/src/com/google/gwt/dev/jjs/ast/JGwtCreate.java (right): http://gwt-code-reviews.appspot.com/1451804/diff/1/dev/core/src/com/google/gwt/dev/jjs/ast/JGwtCreate.java#newcode58 dev/core/src/com/google/gwt/dev/jjs/ast/JGwtCreate.java:58: // TODO: replace with BinaryName.toSourceName(type.getName())? On 2011/06/01 23:02:29, scottb wrote:
I'd rather not including anything in this change that I can avoid, the
TODO was
already there, I'm just making the method accessible.
(snicker) http://gwt-code-reviews.appspot.com/1451804/diff/1/dev/core/src/com/google/gwt/dev/jjs/impl/UnifyAst.java File dev/core/src/com/google/gwt/dev/jjs/impl/UnifyAst.java (right): http://gwt-code-reviews.appspot.com/1451804/diff/1/dev/core/src/com/google/gwt/dev/jjs/impl/UnifyAst.java#newcode465 dev/core/src/com/google/gwt/dev/jjs/impl/UnifyAst.java:465: private static final Set<String> MAGIC_METHOD_CALLS = new LinkedHashSet<String>(Arrays.asList( On 2011/06/01 23:02:29, scottb wrote:
I suppose it's six of one, half a dozen of the other. I guess I tend
to think
of "built-ins" as the code gen types in com.google.gwt.lang. I
loosely use the
term 'magic' for things that the compiler has to have truly
specialized support
for.
I feel strongly that "magic" is the wrong terminology. I see the term "magic" used throughout the code to indicate that "something unexpected" is going on. IMHO, this is like saying this is something that can't be understood or is difficult to understand. But here that isn't really the case. I suggest 'built-in' because we aren't inventing something new here, it is a commonly understood concept. http://www.google.com/search?q=built-in+programming http://gwt-code-reviews.appspot.com/1451804/diff/1/dev/core/src/com/google/gwt/dev/jjs/impl/UnifyAst.java#newcode504 dev/core/src/com/google/gwt/dev/jjs/impl/UnifyAst.java:504: JDeclaredType type = program.getFromTypeMap(rootType); On 2011/06/01 23:02:29, scottb wrote:
The incoming root types are by source name, whereas searchForType is
by binary
name. Updated a couple of variable names to make this more clear.
clearer now, thanks. http://gwt-code-reviews.appspot.com/1451804/diff/1055/dev/core/src/com/google/gwt/dev/jjs/impl/UnifyAst.java#newcode478 dev/core/src/com/google/gwt/dev/jjs/impl/UnifyAst.java:478: private final JsProgram jsProgram; thanks. http://gwt-code-reviews.appspot.com/1451804/diff/1055/dev/core/src/com/google/gwt/dev/jjs/impl/UnifyAst.java#newcode606 dev/core/src/com/google/gwt/dev/jjs/impl/UnifyAst.java:606: } Why not create a constant JMethodBody.EMPTY for this case? Might shave a bit of memory. http://gwt-code-reviews.appspot.com/1451804/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
