Mostly LGTM, just one concern.

http://gwt-code-reviews.appspot.com/125803/diff/1/3
File dev/core/src/com/google/gwt/dev/jjs/ast/JProgram.java (right):

http://gwt-code-reviews.appspot.com/125803/diff/1/3#newcode480
Line 480: }
Might add these to INDEX_TYPES and nest inside that block, as in
createClass(), and avoid the two string compares in the general case.

http://gwt-code-reviews.appspot.com/125803/diff/1/3#newcode1032
Line 1032: allArrayTypes.add(arrayType);
I know the current type hierarchy doesn't currently lend itself to this,
but it kind of feels like we ought to add the two interfaces to the
array type here, instead of hard coding the cast rules elsewhere.  Guess
it's a judgement call, but it feels kind of weird to have to encode
special logic.

For example, imagine
1) A slightly smarter implementation of
JTypeOracle.canTheoerticallyCast() where one interface can be
theoretically cast to another IFF there is some concrete instantiable
type that implements both.
2) A program where an array type is the *only* instantiable object that
implements both Clonable and Serializable.

In that case, JTypeOracle.canTheoerticallyCast(Clonable, Serializable)
ought to return true; but would return false without additional
special-case logic.

Just something to think about.

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

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

Reply via email to