On Mon, Aug 15, 2011 at 9:58 AM,  <stephen.haber...@gmail.com> wrote:

> If you mean the "Roll seed-function optimization forward again" commit
> [1] (or others before it), yeah, I was hoping that would fix it, but it
> did not.

It surprises me that CFA doesn't take care of this and that it needs
to be done as a fix up. One of the changes I made to CFA was to rescue
indirectly referenced class literals (Object.getClass()) of
instantiable subtypes of Object, if the getClass() method is live. You
may want to look at the method CFA.maybeRescueClassLiteral().
Basically, as CFA  is visiting class literals, it will either rescue
them immediatelly (if the type is instantiable and getClass() is
live), or it will add them to a list of things to be rescued later
when getClass() is invoked.

The thing is, if B is a subtype of A, and B is rescued, then A is also
rescued. So if B's class literal is rescued, A's should as well. Even
if getClass() is never invoked, and all you do is refer to B.class,
the fact is, the class literal holder will look something like this:

class ClassLiteralHolder {
   static final Class<Foo> A_classLit = createForClass(..., null);
   static final Class<Foo> B_classLit = createForClass(..., A_classLit);
}

So I'd expect that rescuing B_classLit would rescue A_classLit.   I
added Lex who might be able to shed more light on this. I'm not
opposed to doing it in the fixup, the CodeSplitter is not really a
code base I look at very much, but I wonder if we're not missing a
deeper bug, I would hate to have a never ending stream of 'fixups'
that we have to keep extending. :(


> [1]
> https://github.com/stephenh/scalagwt-gwt/commit/8b04f896d509bb71e59d2acb92e48e68a154bd1f)
>
>> Do you have the repo code?
>
> Yep:
>
> https://github.com/stephenh/scalagwt-sample/commit/6cc1b70ae18c31063f71ce27fc110646c65c73c0
>
> You shouldn't need that whole repo, just the classes in that commit
> (Foo, FragmentA, FragmentB, etc.).
>
> There is also some discussion on the scalagwt list about the bug [2].
> Note that I misused the "L(...)" notation in a few places, so read those
> parts liberally.
>
> [2]: https://groups.google.com/d/msg/scalagwt/cVuvYEkJ5sU/XbfbOWUW2p8J
>
>
> http://gwt-code-reviews.appspot.com/1513803/
>
> --
> http://groups.google.com/group/Google-Web-Toolkit-Contributors
>

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

Reply via email to