icode reader

[Lukas Rytz]

hi,

this is just an update on the one aspect of the inlining story that i'm working on. insights /

comments welcome.

in separate compilation, the icodeReader fails to load the code of methods in trait

implementation classes (A$class)

there's quite a mess when it comes to the symbol for such a class (only in separate

compilation). there are four (!)

 - one ClassSymbol created during Erasure / AddInterfaces

 - one ClassSymbol created by the ClassPath, because the "A$class.class" file exists

 - one ModuleSymbol, also created due to the classfile

 - one ModuleClassSymbol, also created due to the classfile

i observed at least two issues:

1. AddInterfaces "unlinks" the existing ClassSymbol from the "owner.info.decls". The

   intention is good, but i think it's not good enough.

   a. it should also unlink the symbols for module and the moduleClass.

   b. i think we have a general problem when it comes to un-linking: it removes a symbol

       only from the scope of the current type, but not all types in the typeHistory

1.b. seems to be a real problem because the supposedly unlinked symbol comes back /

      is found during ICodeReader

2. ClassfileParser is confused because the (bytecode) methods in implementation classes

    static, so it thinks the symbol for the method found in the moduleClass.

maybe fixing 2. is enough, but everything seems quite brittle...

lukas

[Paul Phillips]

On Saturday, August 11, 2012, Lukas Rytz wrote:

this is just an update on the one aspect of the inlining story that i'm working on. insights /

comments welcome.

I realize I am something of a broken record on this point, but even a broken record is right twice a day (or something like that) so:

Huge amounts of compiler code exist to roll back, restore, reset, or otherwise undo actions taken earlier in the compiler run.  I understand it is usually more difficult to rethink something when the code is 5+ years old and will be full of hidden mysteries for anyone who tries to change it.  But every time someone opts for the localized undo, unlink, un-whatever, the total picture becomes that much more difficult to follow.

The multiplicity of bogus symbols and the difficulty you describe here looks like a great example. Your observation that

 | b. i think we have a general problem when it comes to un-linking: it removes a symbol

 |   only from the scope of the current type, but not all types in the typeHistory

is also a perfect illustration of why it should be the last of last resorts to try to undo anything when operating in an environment where nearly everything is mutable.

I acknowledge that it is within the realm of possibility that some or all of these symbols have to be created; I'm just saying that someone should have to be extremely convincing that it is necessary - no appeals to authority or to secret knowledge allowed - creating bogus symbols which one plans to unlink later should be presumptively the wrong way to do it.

[Lukas Rytz]

Agree with all.

One thing we could do short term: instead of removing a symbol from scopes, mark the symbol

itself as dead. Then we just have to skip all dead symbols in "member" / "decl" lookups.

[Paul Phillips]

On Saturday, August 11, 2012, Lukas Rytz wrote:

 Then we just have to skip all dead symbols in "member" / "decl" lookups.

In fact we could opportunistically unlink them whenever they are encountered during such lookups.

[Lukas Rytz]

sure. so you think it's a good idea?

[Paul Phillips]

On Saturday, August 11, 2012, Lukas Rytz wrote:

sure. so you think it's a good idea?

I think it is likely to help with the immediate problem.  But:

 - if we spend a flag on this I will be sad, and flags tend to survive a long time so it'll be hard to undo this

 - if we have a cache of dead symbols, it will slow down findMember, possibly unacceptably so

 - I would like to look harder at how we might avoid having to distinguish valid symbols from invalid ones before starting to tag some as valid and some as invalid

But in the absence of better ideas, I think it is better than the status quo.

[Lukas Rytz]

On Sat, Aug 11, 2012 at 6:55 PM, Paul Phillips <pa...@improving.org> wrote:

On Saturday, August 11, 2012, Lukas Rytz wrote:

sure. so you think it's a good idea?

I think it is likely to help with the immediate problem.  But:

 - if we spend a flag on this I will be sad, and flags tend to survive a long time so it'll be hard to undo this

 - if we have a cache of dead symbols, it will slow down findMember, possibly unacceptably so

I was thinking of a symbol attachment. But I think they are not part of the 2.10 branch.

Performance might indeed be an issue though..

 

 - I would like to look harder at how we might avoid having to distinguish valid symbols from invalid ones before starting to tag some as valid and some as invalid

right..

[Vlad Ureche]

On Sat, Aug 11, 2012 at 6:47 PM, Paul Phillips <pa...@improving.org> wrote:

In fact we could opportunistically unlink them whenever they are encountered during such lookups.

One problem I faced and I think won't be solved by this is related to point 1b in Lukas' email:

If typer doesn't access a trait's .info and you need to access it late, during GenICode, the implementation class symbol is created but the classfileParser symbol is still visible as .implClass, as flatten already duplicated the scope earlier on, when the implementation class wasn't there.
For more details: https://github.com/VladUreche/scala/commit/f008880256dbf6719e08da0ad8fbf8c62d9fd76a

How would your solution fix that? .implClass would return noSymbol?

Cheers,
Vlad

[Eugene Burmako]

FYI, attachments are a part of what's going to become 2.10.0-final [1]

[1] https://github.com/scala/scala/blob/2.10.x/src/reflect/scala/reflect/macros/Universe.scala#L25

[martin odersky]

Unlinking needs to be done very carefully. In the end it is often better to re-use an existing class symbol.

I think what happened was that, initially, $class symbol were never read by the classfile parser, so they did not show up. That was changed at some point. I don't know why for sure (specialization maybe?). Then probably the unlinking was done in AddInterfaces as an easy fix. But it might have been better to re-use the classfile symbol.

Cheers

 - Martin

--
Martin Odersky
Prof., EPFL and Chairman, Typesafe
PSED, 1015 Lausanne, Switzerland
Tel. EPFL: +41 21 693 6863
Tel. Typesafe: +41 21 691 4967

[Paul Phillips]

On Sat, Aug 11, 2012 at 10:00 AM, Vlad Ureche <vlad....@gmail.com> wrote:

If typer doesn't access a trait's .info and you need to access it late, during GenICode, the implementation class symbol is created but the classfileParser symbol is still visible as .implClass, as flatten already duplicated the scope earlier on, when the implementation class wasn't there.
For more details: https://github.com/VladUreche/scala/commit/f008880256dbf6719e08da0ad8fbf8c62d9fd76a

How would your solution fix that? .implClass would return noSymbol?

Here's what I would do from first principles, someone can tell me why it won't work:

I'd never create any "derived" symbol except through the symbol from which it is derived.  That means module symbols, module class symbols, and implementation class symbols would all be "slaves" to the class symbol master.  If the only way to obtain those symbols is to go through the class, then you cannot end up with an inconsistent view unless you create two class symbols.  You also don't have to go through these huge gyrations when something changes (e.g. look at the dance which takes place when a trait is renamed and the impl class has to be found so it can be renamed in the same way.)

On the specific issue of finding an impl class, is there any reason we can't create the impl class symbol immediately when we create the trait symbol, and create a strong, fixed linkage between them?

These have been on my list of things to try out for a while.