On Tue, Feb 17, 2009 at 5:46 PM, BobV <b...@google.com> wrote:
> On Wed, Feb 18, 2009 at 8:50 AM, Joel Webber <j...@google.com> wrote:
> > ControlFlowAnalyzer:
> > - Just to make sure I understand this correctly, can you explain why we
> had
> > to move the class-literal rescue code to JVariable?
>
> Because of ::class references; access to class literals from JSNI is
> handled as though it were a field reference. I could change
> JsniMethodBody.traverse() to examine it's field references and
> construct JClassLiterals to pass into JVisitor.accept() if you think
> it would be more clear as to what's gong on.
>
Got it, that makes sense.
> > JProgram/CompilingClassLoader:
> > - It feels like there's got to be some way to merge the implementations
> of
> > getTypeFromJsniRef() and getClassFromBinaryName(). Feel free to tell me
> it's
> > not worth it, though.
>
> They operate on totally different data-types. getTypeFromJsniRef()
> needs access to JProgram fields, but getClassFromBinaryName() doesn't
> seem appropriate to add to JProgram.
>
Fair enough. They look so close if you squint a little bit, but it's not
worth convoluting the code to try and shove them together. It's not like
someone's going to invent a new primitive Java type anytime soon.
> JsInliner:
> > - (861) Why the comment about clinits being folded into the JsProgram
> body?
> > I think I'm missing something here -- are we calling clinits in a context
> > where they might get inlined directly into the outer scope?
>
> ClassLiteralHolder's clinit is effectively inlined into the JProgram
> in GenerateJavaScriptAST.generateClassLiterals.
>
Understood.
> In the jjs tests, is there any eay way to share all those class name tests
> > (i.e. startsWith("Class$")) without getting into heavy refactoring? It
> seems
> > like we're only going to run into this sort of thing more as we start
> > implementing optional optimizations (e.g. disable cast-checking), and it
> > would be good to start a sensible pattern for these kinds of tests.
>
> I agree that there should be some common way of querying wether or not
> a particular optimization is applied for those tests that test for
> specific optimizations. Implement System.getProperties() to return
> compiler flags and module properties and a utility class to interpret
> them?
>
Mostly just an observation. Something like that could work. Now's probably
not the time, but I wanted to put the idea out there so we didn't forget
about it.
A few more comments follow:
SerializerBase contains the comment "Relies on monotonic behavior of
hashcodes in web mode". I see no problem with this (and I've wanted to do so
for JRE stuff at times), but I don't think we've ever explicitly stated this
as a requirement of System.identityHashCode(). We should, at a minimum,
document this requirement in our implementation of
identityHashCode()/Impl.getHashCode(), so we don't inadvertently break it.
SerializerBase.MethodMap is a JSO whose methods are non-final. Do you know
why this is working? Do we only run JSO constraint-checking in hosted mode?
Nits:ClientSerializationStreamWriter:57,82,96: JSNI reformat cruft.
StandardSerializationPolicy:55: Double semicolons.
ServerSerializationStreamWriter:412: Comment reformatting cruft
(right-shifted notes).
All that said, once you feel comfortable with it go ahead and commit away.
I've run it against some pretty big projects without incident thus far,
so... Good work!
--~--~---------~--~----~------------~-------~--~----~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~----------~----~----~----~------~----~------~--~---
