[gwt-contrib] Re: Adds ability to query the generator context whether a rebind rule exists for a given type (issue1450806)
http://gwt-code-reviews.appspot.com/1450806/diff/3002/dev/core/src/com/google/gwt/core/ext/GeneratorContext.java File dev/core/src/com/google/gwt/core/ext/GeneratorContext.java (right): http://gwt-code-reviews.appspot.com/1450806/diff/3002/dev/core/src/com/google/gwt/core/ext/GeneratorContext.java#newcode34 dev/core/src/com/google/gwt/core/ext/GeneratorContext.java:34: * likely to succeed. Why is the argument a string rather than, say, JClassType? Wouldn't the second sentence would be more accurate as: is likely to succeed for a type with no default constructor. (Any concrete type with a zero args constructor can be instantiated via GWT.create().) Also, it isn't GWT.create(sourceTypeName), the argument is a class literal. http://gwt-code-reviews.appspot.com/1450806/diff/3002/dev/core/src/com/google/gwt/dev/javac/StandardGeneratorContext.java File dev/core/src/com/google/gwt/dev/javac/StandardGeneratorContext.java (right): http://gwt-code-reviews.appspot.com/1450806/diff/3002/dev/core/src/com/google/gwt/dev/javac/StandardGeneratorContext.java#newcode373 dev/core/src/com/google/gwt/dev/javac/StandardGeneratorContext.java:373: return false; When will there be no resolver? Should the GeneratorContext javadoc say something about this? http://gwt-code-reviews.appspot.com/1450806/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Adds ability to query the generator context whether a rebind rule exists for a given type (issue1450806)
http://gwt-code-reviews.appspot.com/1450806/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Adds ability to query the generator context whether a rebind rule exists for a given type (issue1450806)
http://gwt-code-reviews.appspot.com/1450806/diff/3002/dev/core/src/com/google/gwt/core/ext/GeneratorContext.java File dev/core/src/com/google/gwt/core/ext/GeneratorContext.java (right): http://gwt-code-reviews.appspot.com/1450806/diff/3002/dev/core/src/com/google/gwt/core/ext/GeneratorContext.java#newcode34 dev/core/src/com/google/gwt/core/ext/GeneratorContext.java:34: * likely to succeed. On 2011/05/31 16:27:01, rjrjr wrote: Why is the argument a string rather than, say, JClassType? Since this method will be called from within a generator, there may not be a valid JClassType from type oracle (but we can check for the availability of a rebind rule, by sourceTypeName, which is how they are expressed in rebind rules). The actual type might come into existence after the current (and other) generators complete. Wouldn't the second sentence would be more accurate as: is likely to succeed for a type with no default constructor. (Any concrete type with a zero args constructor can be instantiated via GWT.create().) Can't really check whether a concrete type with a zero args constructor might be available, since it might only come into being after other generators run. Also, it isn't GWT.create(sourceTypeName), the argument is a class literal. Good point. I've removed references to GWT.create() altogether in the javadoc. http://gwt-code-reviews.appspot.com/1450806/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Adds ability to query the generator context whether a rebind rule exists for a given type (issue1450806)
LGTM On Tue, May 31, 2011 at 1:39 PM, jbrosenb...@google.com wrote: http://gwt-code-reviews.appspot.com/1450806/diff/3002/dev/core/src/com/google/gwt/core/ext/GeneratorContext.java File dev/core/src/com/google/gwt/core/ext/GeneratorContext.java (right): http://gwt-code-reviews.appspot.com/1450806/diff/3002/dev/core/src/com/google/gwt/core/ext/GeneratorContext.java#newcode34 dev/core/src/com/google/gwt/core/ext/GeneratorContext.java:34: * likely to succeed. On 2011/05/31 16:27:01, rjrjr wrote: Why is the argument a string rather than, say, JClassType? Since this method will be called from within a generator, there may not be a valid JClassType from type oracle (but we can check for the availability of a rebind rule, by sourceTypeName, which is how they are expressed in rebind rules). The actual type might come into existence after the current (and other) generators complete. Wouldn't the second sentence would be more accurate as: is likely to succeed for a type with no default constructor. (Any concrete type with a zero args constructor can be instantiated via GWT.create().) Can't really check whether a concrete type with a zero args constructor might be available, since it might only come into being after other generators run. Also, it isn't GWT.create(sourceTypeName), the argument is a class literal. Good point. I've removed references to GWT.create() altogether in the javadoc. http://gwt-code-reviews.appspot.com/1450806/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Adds ability to query the generator context whether a rebind rule exists for a given type (issue1450806)
LGTM w/ nit question. http://gwt-code-reviews.appspot.com/1450806/diff/1/dev/core/src/com/google/gwt/core/ext/GeneratorContext.java File dev/core/src/com/google/gwt/core/ext/GeneratorContext.java (right): http://gwt-code-reviews.appspot.com/1450806/diff/1/dev/core/src/com/google/gwt/core/ext/GeneratorContext.java#newcode38 dev/core/src/com/google/gwt/core/ext/GeneratorContext.java:38: boolean checkRebindRuleAvailable(String typeName); sourceTypeName if that's what it is (e.g. com.example.Foo.Bar vs. com.example.Foo$Bar http://gwt-code-reviews.appspot.com/1450806/diff/1/dev/core/src/com/google/gwt/dev/javac/rebind/RebindRuleResolver.java File dev/core/src/com/google/gwt/dev/javac/rebind/RebindRuleResolver.java (right): http://gwt-code-reviews.appspot.com/1450806/diff/1/dev/core/src/com/google/gwt/dev/javac/rebind/RebindRuleResolver.java#newcode22 dev/core/src/com/google/gwt/dev/javac/rebind/RebindRuleResolver.java:22: boolean checkRebindRuleResolvable(String typeName); Why not just toss this onto RebindOracle interface? http://gwt-code-reviews.appspot.com/1450806/diff/1/dev/core/src/com/google/gwt/dev/shell/StandardRebindOracle.java File dev/core/src/com/google/gwt/dev/shell/StandardRebindOracle.java (left): http://gwt-code-reviews.appspot.com/1450806/diff/1/dev/core/src/com/google/gwt/dev/shell/StandardRebindOracle.java#oldcode109 dev/core/src/com/google/gwt/dev/shell/StandardRebindOracle.java:109: usedTypeNames.add(typeName); You might ask Bruce about this. Or at try compiling a project where you have: when-type-assignable A replace-with B when-type-assignable B replace-with A And see what happens. http://gwt-code-reviews.appspot.com/1450806/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Adds ability to query the generator context whether a rebind rule exists for a given type (issue1450806)
http://gwt-code-reviews.appspot.com/1450806/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Adds ability to query the generator context whether a rebind rule exists for a given type (issue1450806)
http://gwt-code-reviews.appspot.com/1450806/diff/1/dev/core/src/com/google/gwt/core/ext/GeneratorContext.java File dev/core/src/com/google/gwt/core/ext/GeneratorContext.java (right): http://gwt-code-reviews.appspot.com/1450806/diff/1/dev/core/src/com/google/gwt/core/ext/GeneratorContext.java#newcode38 dev/core/src/com/google/gwt/core/ext/GeneratorContext.java:38: boolean checkRebindRuleAvailable(String typeName); On 2011/05/27 13:39:20, scottb wrote: sourceTypeName if that's what it is (e.g. com.example.Foo.Bar vs. com.example.Foo$Bar Done. Here and elsewhere that it's implemented. http://gwt-code-reviews.appspot.com/1450806/diff/1/dev/core/src/com/google/gwt/dev/javac/rebind/RebindRuleResolver.java File dev/core/src/com/google/gwt/dev/javac/rebind/RebindRuleResolver.java (right): http://gwt-code-reviews.appspot.com/1450806/diff/1/dev/core/src/com/google/gwt/dev/javac/rebind/RebindRuleResolver.java#newcode22 dev/core/src/com/google/gwt/dev/javac/rebind/RebindRuleResolver.java:22: boolean checkRebindRuleResolvable(String typeName); Well, I guess I don't want to expose the full RebindOracle interface to the StandardGeneratorContext (e.g. generators shouldn't be able to call rebind() directly). Also, other things implement the RebindOracle interface, such as ModuleSpaceHost (and by extension ShellModuleSpaceHost). It doesn't seem sensible for those to implement checkRebindRuleResolvable(), etc. No? http://gwt-code-reviews.appspot.com/1450806/diff/1/dev/core/src/com/google/gwt/dev/shell/StandardRebindOracle.java File dev/core/src/com/google/gwt/dev/shell/StandardRebindOracle.java (left): http://gwt-code-reviews.appspot.com/1450806/diff/1/dev/core/src/com/google/gwt/dev/shell/StandardRebindOracle.java#oldcode109 dev/core/src/com/google/gwt/dev/shell/StandardRebindOracle.java:109: usedTypeNames.add(typeName); I haven't been able to break things with setting up circular dependencies. Seems to be passing all the tests I've tried with it, etc. http://gwt-code-reviews.appspot.com/1450806/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Adds ability to query the generator context whether a rebind rule exists for a given type (issue1450806)
LGTM http://gwt-code-reviews.appspot.com/1450806/diff/1/dev/core/src/com/google/gwt/dev/javac/rebind/RebindRuleResolver.java File dev/core/src/com/google/gwt/dev/javac/rebind/RebindRuleResolver.java (right): http://gwt-code-reviews.appspot.com/1450806/diff/1/dev/core/src/com/google/gwt/dev/javac/rebind/RebindRuleResolver.java#newcode22 dev/core/src/com/google/gwt/dev/javac/rebind/RebindRuleResolver.java:22: boolean checkRebindRuleResolvable(String typeName); On 2011/05/27 23:11:20, jbrosenberg wrote: Well, I guess I don't want to expose the full RebindOracle interface to the StandardGeneratorContext (e.g. generators shouldn't be able to call rebind() directly). Also, other things implement the RebindOracle interface, such as ModuleSpaceHost (and by extension ShellModuleSpaceHost). It doesn't seem sensible for those to implement checkRebindRuleResolvable(), etc. No? What I'd meant was not exposing this to users, but at a first glance it seemed like more of the unnecessary abstraction that's plagued this part of the system (for example, there's effectively only one real implementor of RebindOracle and RebindPermutationOracle). But now that I look closer, there's really no other easy way to do what you want. Oh well, just another reason to come through here sometime holistically and make it all more sane. http://gwt-code-reviews.appspot.com/1450806/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Adds ability to query the generator context whether a rebind rule exists for a given type (issue1450806)
http://gwt-code-reviews.appspot.com/1450806/diff/1/dev/core/src/com/google/gwt/dev/shell/StandardRebindOracle.java File dev/core/src/com/google/gwt/dev/shell/StandardRebindOracle.java (left): http://gwt-code-reviews.appspot.com/1450806/diff/1/dev/core/src/com/google/gwt/dev/shell/StandardRebindOracle.java#oldcode52 dev/core/src/com/google/gwt/dev/shell/StandardRebindOracle.java:52: These 2 data structures appear to be relics from an older version of this class, I can't see how it's possible for them to retain state between successive calls to Rebinder.rebind() http://gwt-code-reviews.appspot.com/1450806/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
