http://gwt-code-reviews.appspot.com/1470803/diff/19/dev/core/src/com/google/gwt/dev/jjs/ast/AccessModifiers.java
File dev/core/src/com/google/gwt/dev/jjs/ast/AccessModifiers.java
(right):

http://gwt-code-reviews.appspot.com/1470803/diff/19/dev/core/src/com/google/gwt/dev/jjs/ast/AccessModifiers.java#newcode21
dev/core/src/com/google/gwt/dev/jjs/ast/AccessModifiers.java:21: public
enum AccessModifiers {
On 2011/07/06 17:00:29, zundel wrote:
Note, using <value>.ordinal() means that any changing in order is
going to screw
up the serialized tree.  I suggest we either do something to protect
against
that in JMethod, or add a dire warning here against reordring the
values (e.g.
sorting them alphabetcially)

Done.

http://gwt-code-reviews.appspot.com/1470803/diff/19/dev/core/src/com/google/gwt/dev/jjs/ast/AccessModifiers.java#newcode21
dev/core/src/com/google/gwt/dev/jjs/ast/AccessModifiers.java:21: public
enum AccessModifiers {
On 2011/07/06 17:31:33, jbrosenberg wrote:
I'll second Eric's concern here.  But that not-withstanding, I'd
suggest the
name of this enum should be singular, e.g., "AccessModifier".  Since
you can't
compose multiple modifiers in a single instance, it appears misleading
when you
declare it as a param to a method, e.g. myFunc(AccessModifiers
access)....

Done.

http://gwt-code-reviews.appspot.com/1470803/diff/19/dev/core/src/com/google/gwt/dev/jjs/ast/JConstructor.java
File dev/core/src/com/google/gwt/dev/jjs/ast/JConstructor.java (right):

http://gwt-code-reviews.appspot.com/1470803/diff/19/dev/core/src/com/google/gwt/dev/jjs/ast/JConstructor.java#newcode53
dev/core/src/com/google/gwt/dev/jjs/ast/JConstructor.java:53: // Access
only matters for virtual methods, just use public.
I wanted to leave open the possibility of extending it to track other
methods eventually, which makes adding additional access modifiers kind
of sour tasting.

http://gwt-code-reviews.appspot.com/1470803/diff/19/dev/core/src/com/google/gwt/dev/jjs/ast/JMethod.java
File dev/core/src/com/google/gwt/dev/jjs/ast/JMethod.java (right):

http://gwt-code-reviews.appspot.com/1470803/diff/19/dev/core/src/com/google/gwt/dev/jjs/ast/JMethod.java#newcode140
dev/core/src/com/google/gwt/dev/jjs/ast/JMethod.java:140: this.access =
access.ordinal();
What I ended up doing is asserting explicit ordinal values in
AccessModifier to guarantee stable values.  See updated patch.  Added
additional API seemed more dangerous, as it would be very easy to mix up
use of ordinal() and the new api accidentially.

http://gwt-code-reviews.appspot.com/1470803/diff/19/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/1470803/diff/19/dev/core/src/com/google/gwt/dev/jjs/impl/GwtAstBuilder.java#newcode2824
dev/core/src/com/google/gwt/dev/jjs/impl/GwtAstBuilder.java:2824: static
AccessModifiers getAccessModifier(MethodBinding b) {
On 2011/07/06 17:00:29, zundel wrote:
Make this  method AccessModifiers.fromMethodBinding()?

Done.

http://gwt-code-reviews.appspot.com/1470803/diff/19/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/1470803/diff/19/dev/core/src/com/google/gwt/dev/jjs/impl/UnifyAst.java#newcode728
dev/core/src/com/google/gwt/dev/jjs/impl/UnifyAst.java:728: for (JMethod
upref : collected.get(method.getSignature())) {
On 2011/07/06 17:31:33, jbrosenberg wrote:
debug cruft?

Done.

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

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

Reply via email to