Issue with traits

Create a trait:

Trait named: #ABBA
        uses: {}
        category: 'ABBA'.

- add a method #foo to ABBA trait

Then create 2 classes which using it:

Object subclass: #ABBAB
        uses: ABBA
        instanceVariableNames: ''
        classVariableNames: ''
        poolDictionaries: ''
        category: 'ABBA'.

Object subclass: #ABBAC
        uses: ABBA
        instanceVariableNames: ''
        classVariableNames: ''
        poolDictionaries: ''
        category: 'ABBA'.


now what i found interesting:

(ABBAB methodDict at: #foo) == (ABBAC methodDict at: #foo)

but as side effect, we got this one:

(ABBAB methodDict at: #foo) methodClass  ==> ABBAC

which i find a bit disturbing :(

I'm not sure if given issue could have a major impact somewhere...
But i would just #clone the trait method before installing it..
Yes, it takes more space, but it makes each of such methods unrelated.

And i strongly assume that these methods should be unrelated, and
moreover, recompiled for each class separately, because
global var in one class could be a class var in another one, so a
trait method, like:

foo
  ^ Bar

will behave depending on class environment.


--
Best regards,
Igor Stasenko AKA sig.

Igor Stasenko wrote:
> now what i found interesting:
>
> (ABBAB methodDict at: #foo) == (ABBAC methodDict at: #foo)
>
> but as side effect, we got this one:
>
> (ABBAB methodDict at: #foo) methodClass  ==> ABBAC
>
> which i find a bit disturbing :(

Yes, seems wrong. VERY wrong. Try this:

Object subclass: #BaseA

BaseA>>foo
   ^42

Object subclass: #BaseB

BaseB>>foo
   ^13

And then create subclasses of BaseA and BaseB which use your trait and
have the trait implementation use "^super foo". One of two things should
happen:

a) They both return the same (incorrect) value (which would be expected
if methodClass is the same). In this case it's just plain broken.

b) They both return different values, in which case there is code that
handles super sends specifically and the sharing is simply an
unnecessary optimization.

I hope it's the latter case which would mean you only need to throw out
the test for super sends and do whatever that code was doing to begin
with ;-)

Cheers,
   - Andreas

2009/10/21 Andreas Raab <[hidden email]>:
Ouch... so VM behavior depends on correct methodClass slot. Thanks for
reminding!
I remember that methodClass plays important role (and its important to
have method and its
methodClass being in sync with class where the method installed), but
forgot the details :)

That could lead to even worse results, if you use super sends, and then
in base class , accessing the instance variables in base method. It
could corrupt the memory & crash the VM.

I hope this is already fixed, because the image where i checked it
is quite old one - 3.10.2

> Cheers,
>  - Andreas


--
Best regards,
Igor Stasenko AKA sig.

Igor Stasenko wrote:
Yes, that's why I returned a literal (so the experiment wouldn't crash
you ;-)

> I hope this is already fixed, because the image where i checked it
> is quite old one - 3.10.2

I doubt it. Traits are just another abandoned experiment in Squeak.

Cheers,
   - Andreas

2009/10/21 Andreas Raab <[hidden email]>:
Just tried in Pharo, there is a copy.

(ABBAB methodDict at: #foo) == (ABBAC methodDict at: #foo). -> false

(ABBAB methodDict at: #foo) methodClass. -> ABBAB

At least Traits are maintained there, and pharo changes should be picked.

Nicolas

> Cheers,
>  - Andreas
>
>

I've fixed this issue in Pharo a couple of months ago. CompiledMethods  
are not shared anymore, so methodClass returns the expected class. At  
the time I implemented traits (2004?), there were no method properties  
and hence sharing compiled methods worked -- of course except for  
super sends in which case the method was always recompiled (so the  
answer to Andreas' question below is b)).

Cheers,
Adrian


On Oct 21, 2009, at 08:42 , Andreas Raab wrote:

Nicolas Cellier wrote:
> Just tried in Pharo, there is a copy.
>
> (ABBAB methodDict at: #foo) == (ABBAC methodDict at: #foo). -> false
>
> (ABBAB methodDict at: #foo) methodClass. -> ABBAB
>
> At least Traits are maintained there, and pharo changes should be picked.

If you feel comfortable doing this, please do. The reason I'm calling it
an abandoned experiment is that unless I'm mistaken, there is nobody
left in the Squeak community who could fix any issues arising here. That
means that although we might be able to mechanically pick some fixes
from Pharo, there is no ownership of the code itself which means that if
there is ever a real issue we'll be left without help. Which is a risk
that I'm generally not comfortable in taking and less so if the kernel
of the system is involved. We need to own that code if we want to
develop it further.

Cheers,
   - Andreas

On 21.10.2009, at 09:41, Adrian Lienhard wrote:

> I've fixed this issue in Pharo a couple of months ago. CompiledMethods
> are not shared anymore, so methodClass returns the expected class. At
> the time I implemented traits (2004?), there were no method properties
> and hence sharing compiled methods worked -- of course except for
> super sends in which case the method was always recompiled (so the
> answer to Andreas' question below is b)).
>

We did a small paper for this years ESUG that describes the changes.
It will be online at esug.org soon with all the other papers.

If somebody wants a copy, I can send it.

        Marcus

--
Marcus Denker  --  [hidden email]
http://www.2denker.de

2denker UG (haftungsbeschränkt)
Sitz Köln
Amtsgericht - Registergericht - Köln, HRB 65065
Geschäftsführer: Christian Denker und Dr. Marcus Denker

--
Cédrick

Why not share the code?  Just use a "cell" to hold the class and have
methodClass accessor do the indirection.

If you have a bit in the method header, the VM could use it as a 0/1 index
into the code to call to do (or not do) the extra indirection.  Just think of
the a trait method as a closure which is parameterized on the class.

Am I missing something deep here?

-KenD

Ken.Dickey wrote:
> Why not share the code?  Just use a "cell" to hold the class and have
> methodClass accessor do the indirection.
>
> If you have a bit in the method header, the VM could use it as a 0/1 index
> into the code to call to do (or not do) the extra indirection.  Just think of
> the a trait method as a closure which is parameterized on the class.
>
> Am I missing something deep here?

Mostly that it's generally hard to change the VM and asking for extra
header bits is generally frowned upon because they are in short supply.
So in short, yes what you are proposing would be one possible solution
but by and large it will be *much* simpler to just not share trait
methods (which is a fix that can be implemented without VM mods).

Cheers,
   - Andreas

On 21.10.2009, at 22:28, Ken.Dickey wrote:

> Why not share the code?  Just use a "cell" to hold the class and have
> methodClass accessor do the indirection.
>
But based on which info should this indirection be made?

The idea with a simple backpointer was done because it's already there  
in case of super-sends.
So you would need to do something special there again (the last  
literal *has to be* a class
for those methds).

> If you have a bit in the method header, the VM could use it as a 0/1  
> index
> into the code to call to do (or not do) the extra indirection.

If there is something that is a scarce resource, than it's bits in the  
method header. And
I don't get it: there is one header per object, not one per reference.  
Thus
the bit could not be diffrerent per *use* of the compiledMethod.

>  Just think of
> the a trait method as a closure which is parameterized on the class.
>
> Am I missing something deep here?
>

You overlook complexity ;-)

On 21.10.2009, at 11:13, Marcus Denker wrote:

Here is the pdf:

http://marcusdenker.de/publications/Duca09bTraitsAPI.pdf

        Marcus



--
Marcus Denker  --  [hidden email]
http://www.2denker.de

Marcus Denker wrote:
>> We did a small paper for this years ESUG that describes the changes.
>> It will be online at esug.org soon with all the other papers.
>
> Here is the pdf:
>
> http://marcusdenker.de/publications/Duca09bTraitsAPI.pdf

Thanks though it seems there isn't much detail on the sharing issue; it
appears that all that's being said is the following (for those of you
who haven't read the paper):

        5.2 Non Sharing
        The new implementation copies the method instances and
        their source code on a per use basis (for each class or trait).
        This way a compiled method has a unique selector and a
        unique class to which it belongs. The source is also not
        shared anymore since with aliases the selector of a method
        can differ from its original (in the old implementation, the
        source was patched before shown to the user). [...]

Cheers,
   - Andreas