http://gwt-code-reviews.appspot.com/702801/diff/7001/8001 File dev/core/src/com/google/gwt/dev/jjs/ast/JArrayLength.java (right):
http://gwt-code-reviews.appspot.com/702801/diff/7001/8001#newcode26 dev/core/src/com/google/gwt/dev/jjs/ast/JArrayLength.java:26: * This can only be null if the referenced field is static. On 2010/07/19 23:40:01, scottb wrote:
Remove stale copied comment.
Done. http://gwt-code-reviews.appspot.com/702801/diff/7001/8001#newcode40 dev/core/src/com/google/gwt/dev/jjs/ast/JArrayLength.java:40: @Override On 2010/07/19 23:40:01, scottb wrote:
@Override illegal in 1.5 for interface method.
Tricky. I used JFieldRef as a starting point, and it uses @override, but it inherits an implementation from its super class. So in that case, it's technically overriding it's super class method. It'd be nice to upgrade to 1.6 and use -target 1.5, but I can see how that'd make things more tricky in terms of keeping runtime compatibility. http://gwt-code-reviews.appspot.com/702801/diff/7001/8001#newcode52 dev/core/src/com/google/gwt/dev/jjs/ast/JArrayLength.java:52: if (instance != null) { On 2010/07/19 23:40:01, scottb wrote:
Should never be null.
Done. http://gwt-code-reviews.appspot.com/702801/diff/7001/8004 File dev/core/src/com/google/gwt/dev/jjs/impl/CompoundAssignmentNormalizer.java (right): http://gwt-code-reviews.appspot.com/702801/diff/7001/8004#newcode88 dev/core/src/com/google/gwt/dev/jjs/impl/CompoundAssignmentNormalizer.java:88: public boolean visit(JArrayLength x, Context ctx) { On 2010/07/19 23:40:01, scottb wrote:
This isn't necessary... this visitor is only ever run on LVALUE
expressions,
and array.length can never be an LVALUE.
Done. http://gwt-code-reviews.appspot.com/702801/diff/7001/8005 File dev/core/src/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzer.java (right): http://gwt-code-reviews.appspot.com/702801/diff/7001/8005#newcode457 dev/core/src/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzer.java:457: } else if (x instanceof JArrayLength) { On 2010/07/19 23:40:01, scottb wrote:
I don't understand this change. This is another context where array
length
should never be encountered because we're in an LVALUE context here.
I was being conservative in guaranteeing that if JArrayLengths flowed through the same code that JFieldRefs did, it would be treated as before. I was leery of quirks in the GWT AST that show up during the optimization phases. http://gwt-code-reviews.appspot.com/702801/diff/7001/8006 File dev/core/src/com/google/gwt/dev/jjs/impl/ExpressionAnalyzer.java (right): http://gwt-code-reviews.appspot.com/702801/diff/7001/8006#newcode99 dev/core/src/com/google/gwt/dev/jjs/impl/ExpressionAnalyzer.java:99: accessesFieldNonFinal = true; On 2010/07/19 23:40:01, scottb wrote:
Definitely remove this line, array.length is immutable.
Ok. Thrown off by the non-final/volatile definition of length in the intrinsic Array.
We also need canThrowException = true in case the instance happens to
be null. Done. http://gwt-code-reviews.appspot.com/702801/diff/7001/8008 File dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java (right): http://gwt-code-reviews.appspot.com/702801/diff/7001/8008#newcode257 dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java:257: arrayLengthField.setObfuscatable(false); On 2010/07/19 23:40:01, scottb wrote:
On review, I think I gave you bad advice to make it a sibling of
nullMethodName.
Making it a sibling of GenerateJavaScriptVisitor.prototype looks
better. Also
suggest just shorten it to "arrayLength" since "field" is not really a
term used
in JS.
Done. http://gwt-code-reviews.appspot.com/702801/diff/7001/8008#newcode540 dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java:540: JsNameRef ref = new JsNameRef(x.getSourceInfo(), arrayLengthField); On 2010/07/19 23:40:01, scottb wrote:
JsNameRef ref = arrayLengthField.makeRef(x.getSourceInfo());
Done. http://gwt-code-reviews.appspot.com/702801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
