I have reviewed the JJS parts and the JSORestrictionsChecker part.
That leaves hosted mode (dev/shell).

I like the new rules that allow Java implementations to also exist,
albeit at a performance penalty.

I have one question about the new checking rules: is the check at
JSORestrictionsChecker lines 125-137 necessary?  It actually seems
fine to me if an interface extends another interface that extends
SingleJsoImpl.  This is particularly interesting in the case where the
interface actually won't have a JSO implementation, but only Java
implementations.  With the checking rules in the current patch, such
an interface would have to explicitly mark itself as SingleJsoImpl
even though it wouldn't be implemented by even one JSO.

More generally, marker interfaces would naturally be inherited along
with everything else that is inherited, and this doesn't seem like a
place to make an exception.

The comment of JSORestrictionsChecker still says that no method may
override another.  This should be updated to reflect the SingleJsoImpl
exception.  Ideally, the restrictions would all be documented in one
place.  One way to do that would be to move SingleJsoImpl's comments
over to JSORestrictionsChecker.  Or better, perhaps they should all be
moved to JavaScriptObject, so that they are easily accessible to
people using GWT.

Did you try making JSORestrictionsChecker and
CompilationUnitInvalidator being instantiable?  This would remove a
few lines of code for defining and passing around the state objects.

Does JCastOperation.allowJso simplify things any over simply having
CastNormalizer compute the same information?  That is, couldn't we
move the current test in JSONormalizer into CastNormalizer, and leave
JCastOperation as it stands?  Likewise for JInstanceOf.

If I'm not mistaken, the dualImpl information in JTypeOracle is
computed only before optimization.  It would be a good TODO in
setInstantiatedTypes to say that the info should be recomputed.  It's
entirely possible that there will be Java implementations of a JSO
before optimizations, but that the optimizer will remove the non-Java
ones and thus allow a better implementation.

That's all.  The implementation looks great and the test cases are
remarkably thorough.


Lex

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

Reply via email to