[Pharo-project] Big problem with the implementation of traits

Adrian Lienhard adi at netstyle.ch
Sun Jan 18 13:51:32 EST 2009


Hi Alex

Thanks for taking a look.

On Jan 18, 2009, at 19:14 , Alexandre Bergel wrote:

> I went through your changes:
> 	- You new version of ClassBuilder>>validateClassvars:from:forSuper:
> makes fully sense to me, class variables of metaclasses should not
> matter. It is right to do (oldClass allSubclasses reject: #isMeta). I
> read the description of http://code.google.com/p/pharo/issues/detail?id=246
>  , but I haven't seen this bug. (Object allSubclasses reject:
> #isMeta) select: [:cls | cls classVarNames ~= cls class classVarNames]
> is empty in my image

This issue, which I fixed with the change you mention above, is not  
related to traits but I needed to fix it because else I could not load  
my fixes back into an image as ClassBuilder was broken in the edge  
case of the class Behavior. The issue is that it checks whether there  
exists no subclass of the class to be changed (in this case Behavior)  
that has the same classVarNames. Now in the case of Behavior, its  
metaclass, Behavior class, *is* a subclass of Behavior. With the  
change introduced by #246, Metaclasses answer to #classVarNames,  
leading to a false warning in ClassBuilder.

> Before your change, 87 out of 88 tests on traits are green. The only
> failing one is testLocalMethodWithSameCodeInTrait. Are there some
> compiled method sharing still? Or each compiled method is copied?

This is unrelated to sharing compiled methods. This test simply checks  
whether there exists a method in a class that has the same  
implementation as a method that would be obtained from a trait this  
class uses. Hence, this points out issues with clients of traits, not  
with the traits implementation.

> Same ratio of green tests after having loaded your change.
>
> That strange, Traits-adrian_lienhard.258 removes only two test methods
> (testTraitsUsersSanity and testUsersWithClassChanges). These two
> methods do not exist in my image. The comment said this fix the issue
> 443 (http://code.google.com/p/pharo/issues/detail?id=443). I do not
> see how you change is related to this bug.

Traits-adrian_lienhard.258 should *add* the two tests you mention  
above. They document the problem and should run green after loading  
the Kernel package and after executing the script.

Adrian

>
>
> Hope it helps,
> Alexandre
>
>
>
> On 18 Jan 2009, at 18:27, Adrian Lienhard wrote:
>
>> I've published a fix to the inbox.  Since the bug was related to some
>> implementation details of the ClassBuilder and hence non-trivial, I'd
>> appreciate if somebody could try it out and verify that traits still
>> work as expected. I've added tests that document the bug and they run
>> green now (with the other 80 traits tests).
>> For details see http://code.google.com/p/pharo/issues/detail?id=443
>> Adrian
>>
>> On Jan 17, 2009, at 15:29 , Adrian Lienhard wrote:
>>
>>> Well, if you just want to make your numbers be right, that's easy:
>>>
>>> Smalltalk allTraits do: [ :each | each instVarNamed: 'users' put:
>>> IdentitySet new ].
>>> Smalltalk allClassesAndTraits do: [ :each |
>>> 	each hasTraitComposition ifTrue: [ each setTraitComposition: each
>>> traitComposition ] ].
>>>
>>> This recreates all users sets.
>>>
>>> Adrian
>>>
>>> On Jan 15, 2009, at 18:20 , Damien Cassou wrote:
>>>
>>>> On Thu, Jan 15, 2009 at 6:18 PM, Adrian Lienhard <adi at netstyle.ch>
>>>> wrote:
>>>>> BTW, why is this so pressing now? This bug has existed for four
>>>>> years.
>>>>
>>>> We are writing an article and we have lots of metrics automatically
>>>> calculated (and the LaTeX tables are also automatically generated).
>>>> Since the article and the metrics are about trait users...
>>>>
>>>> -- 
>>>> Damien Cassou
>>>> http://damiencassou.seasidehosting.st
>>>>
>>>> _______________________________________________
>>>> Pharo-project mailing list
>>>> Pharo-project at lists.gforge.inria.fr
>>>> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
>>>
>>>
>>> _______________________________________________
>>> Pharo-project mailing list
>>> Pharo-project at lists.gforge.inria.fr
>>> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
>>
>>
>> _______________________________________________
>> Pharo-project mailing list
>> Pharo-project at lists.gforge.inria.fr
>> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
>>
>
> -- 
> _,.;:~^~:;._,.;:~^~:;._,.;:~^~:;._,.;:~^~:;._,.;:
> Alexandre Bergel  http://www.bergel.eu
> ^~:;._,.;:~^~:;._,.;:~^~:;._,.;:~^~:;._,.;:~^~:;.
>
>
>
>
>
>
> _______________________________________________
> Pharo-project mailing list
> Pharo-project at lists.gforge.inria.fr
> http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project





More information about the Pharo-dev mailing list