> 5176: All this does is tweak the debug output of the Java AST. JSNI methods > have proper indentation in an AST dump now (I was using AST dumps to verify > incremental correctness.)
LGTM. > 5177: Fairly self-explanatory; added the state to JReferenceType, but kept > it in JTypeOracle at first to assert there was no difference. Aside: I'm > kind of displeased with the type hierarchy of JReferenceType. It really > feels like JClassType and JInterfaceType should share a common ancestor that > JArrayType and JNullType do not. If this were the structure, I'd have put > abstract methods into JReferenceType and put the statefulness in that common > ancestor. LGTM. I see what you mean about JArrayType being an immediate sibling of JClassType; it doesn't really make sense to ask an array type if it has a clinit. > 5178: Also tightened up the recursive method slightly, and managing the > "computed" set better. This works because once a class transitions from > hasClinit -> !hasClinit, there's no possible way it can ever go back. Small problem: I believe line 644-646 in the patched version is intended to test "target", not "type". If that sounds right, then the rest LGTM. Otherwise, let's discuss how this is supposed to work. By the way, this algorithm could be sped up if, it mattered for performance. Instead of repeatedly recursing for each type, start by marking classes where hasLiveCode() as having clinits. Then, propagate clinit-ness backwards along the getClinitTargets() graph. Any class not reached does not needs its clinit. The advantage probably doesn't matter in this case in practice, but I mention it because this funny algorithm pattern keeps coming up. > 5179: I should have actually squashed this tiny fix into a previous commit; > apparently sometimes people were calling hasClinit with nulls. LGTM. It looks like things would be simpler if we gave those last straggling fields and methods an actual enclosing class, but now's not the time. > 5180: Note the assertion in JFieldRef... I added this because I ran into a > null JFieldRef.enclosingType() late in the compile. This assertion > uncovered a major problem... JsniFieldRefs always had a null enclosing type. > I didn't check, but it's possible this could have been resulting in a > failure to clinit a class when one of its fields is first accessed from > JSNI. The fix is in GenerateJavaAST in this patch. LGTM. > 5181: Should be self-explanatory. LGTM. -Lex --~--~---------~--~----~------------~-------~--~----~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~----------~----~----~----~------~----~------~--~---
