@spoon: question for you embedded in comments. @tobyr: mostly LG, just a few things.
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. Remove stale copied comment. http://gwt-code-reviews.appspot.com/702801/diff/7001/8001#newcode40 dev/core/src/com/google/gwt/dev/jjs/ast/JArrayLength.java:40: @Override @Override illegal in 1.5 for interface method. 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) { Should never be null. 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) { This isn't necessary... this visitor is only ever run on LVALUE expressions, and array.length can never be an LVALUE. 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) { 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. 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; Definitely remove this line, array.length is immutable. We also need canThrowException = true in case the instance happens to be null. http://gwt-code-reviews.appspot.com/702801/diff/7001/8006#newcode163 dev/core/src/com/google/gwt/dev/jjs/impl/ExpressionAnalyzer.java:163: canThrowException = true; @spoon: speaking of canThrowException, we can now make this slightly better by checking for a JNonNullType instance? 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 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. 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); JsNameRef ref = arrayLengthField.makeRef(x.getSourceInfo()); http://gwt-code-reviews.appspot.com/702801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
