[Pharo-project] Big problem with the implementation of traits
Hi, if you load the latest version of Collection package in PharoInbox and evaluate the following, you will see a problem: TCreationWithTest users ===> an IdentitySet(SetTest OrderedCollectionTest ArrayTest) TCreationWithTest users size =====> 5 Depending on your image, the results may vary but will probably still be incorrect. Bye -- Damien Cassou http://damiencassou.seasidehosting.st
Confirmed. I created http://code.google.com/p/pharo/issues/detail?id=443 Not sure how it is possible that this IdentitySet can get into that state. Any ideas? Adrian On Jan 15, 2009, at 11:01 , Damien Cassou wrote:
Hi,
if you load the latest version of Collection package in PharoInbox and evaluate the following, you will see a problem:
TCreationWithTest users ===> an IdentitySet(SetTest OrderedCollectionTest ArrayTest)
TCreationWithTest users size =====> 5
Depending on your image, the results may vary but will probably still be incorrect.
Bye
-- Damien Cassou http://damiencassou.seasidehosting.st
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
No we check and this is strange. What is the hash of a trait? Stef On Jan 15, 2009, at 12:08 PM, Adrian Lienhard wrote:
Confirmed. I created http://code.google.com/p/pharo/issues/detail?id=443 Not sure how it is possible that this IdentitySet can get into that state. Any ideas?
Adrian
On Jan 15, 2009, at 11:01 , Damien Cassou wrote:
Hi,
if you load the latest version of Collection package in PharoInbox and evaluate the following, you will see a problem:
TCreationWithTest users ===> an IdentitySet(SetTest OrderedCollectionTest ArrayTest)
TCreationWithTest users size =====> 5
Depending on your image, the results may vary but will probably still be incorrect.
Bye
-- Damien Cassou http://damiencassou.seasidehosting.st
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
On Jan 15, 2009, at 12:41 , Stéphane Ducasse wrote:
No we check and this is strange. What is the hash of a trait?
The hash of traits is not related to the problem. The problem is that classes show up multiple times in IdentitySets. I found the cause (its a bit complicated to explain because related to the ClassBuilder and become:). I can fix that during the weekend... Adrian
Stef
On Jan 15, 2009, at 12:08 PM, Adrian Lienhard wrote:
Confirmed. I created http://code.google.com/p/pharo/issues/detail?id=443 Not sure how it is possible that this IdentitySet can get into that state. Any ideas?
Adrian
On Jan 15, 2009, at 11:01 , Damien Cassou wrote:
Hi,
if you load the latest version of Collection package in PharoInbox and evaluate the following, you will see a problem:
TCreationWithTest users ===> an IdentitySet(SetTest OrderedCollectionTest ArrayTest)
TCreationWithTest users size =====> 5
Depending on your image, the results may vary but will probably still be incorrect.
Bye
-- Damien Cassou http://damiencassou.seasidehosting.st
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
On Thu, Jan 15, 2009 at 3:13 PM, Adrian Lienhard <adi@netstyle.ch> wrote:
The hash of traits is not related to the problem. The problem is that classes show up multiple times in IdentitySets.
I found the cause (its a bit complicated to explain because related to the ClassBuilder and become:). I can fix that during the weekend...
I've implemented a TraitBehavior>>nsUsers method which does nsUsers ^ self users asOrderedCollection asSet to work around the problem for the article. Can you confirm that this is a valid workaround? -- Damien Cassou http://damiencassou.seasidehosting.st
On Jan 15, 2009, at 15:43 , Damien Cassou wrote:
On Thu, Jan 15, 2009 at 3:13 PM, Adrian Lienhard <adi@netstyle.ch> wrote:
The hash of traits is not related to the problem. The problem is that classes show up multiple times in IdentitySets.
I found the cause (its a bit complicated to explain because related to the ClassBuilder and become:). I can fix that during the weekend...
I've implemented a TraitBehavior>>nsUsers method which does
nsUsers ^ self users asOrderedCollection asSet
to work around the problem for the article. Can you confirm that this is a valid workaround?
No. This will remove the duplicates, but in case traits are removed from classes you can still get wrong results if this happens for a class with a users set that is inconsistent. Adrian
On Thu, Jan 15, 2009 at 4:47 PM, Adrian Lienhard <adi@netstyle.ch> wrote:
No. This will remove the duplicates, but in case traits are removed from classes you can still get wrong results if this happens for a class with a users set that is inconsistent.
So, we really need your fix. -- Damien Cassou http://damiencassou.seasidehosting.st
ok, I'll see what I can do... BTW, why is this so pressing now? This bug has existed for four years. Adrian On Jan 15, 2009, at 18:11 , Damien Cassou wrote:
On Thu, Jan 15, 2009 at 4:47 PM, Adrian Lienhard <adi@netstyle.ch> wrote:
No. This will remove the duplicates, but in case traits are removed from classes you can still get wrong results if this happens for a class with a users set that is inconsistent.
So, we really need your fix.
-- Damien Cassou http://damiencassou.seasidehosting.st
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
On Thu, Jan 15, 2009 at 6:18 PM, Adrian Lienhard <adi@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
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@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@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
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@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@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
Hi Adrian, 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 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? 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. 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@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@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
-- _,.;:~^~:;._,.;:~^~:;._,.;:~^~:;._,.;:~^~:;._,.;: Alexandre Bergel http://www.bergel.eu ^~:;._,.;:~^~:;._,.;:~^~:;._,.;:~^~:;._,.;:~^~:;.
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
In one of the comment, Marcus said this is fixed already. Cheers, Alexandre
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? 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.
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@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@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
_______________________________________________ Pharo-project mailing list Pharo-project@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@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
-- _,.;:~^~:;._,.;:~^~:;._,.;:~^~:;._,.;:~^~:;._,.;: Alexandre Bergel http://www.bergel.eu ^~:;._,.;:~^~:;._,.;:~^~:;._,.;:~^~:;._,.;:~^~:;.
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@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@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
_______________________________________________ Pharo-project mailing list Pharo-project@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@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
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.
Sorry, I misread the merger. After having loaded Kernel-adrian_lienhard.252, testTraitsUsersSanity goes red (where testUsersWithClassChanges goes green). It seems that the following traits do not include their users: an IdentitySet(SetTest ArrayTest OrderedCollectionTest BagTest) Did I do something wrong? Cheers, Alexandre
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@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@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo- project
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
_______________________________________________ Pharo-project mailing list Pharo-project@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@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
-- _,.;:~^~:;._,.;:~^~:;._,.;:~^~:;._,.;:~^~:;._,.;: Alexandre Bergel http://www.bergel.eu ^~:;._,.;:~^~:;._,.;:~^~:;._,.;:~^~:;._,.;:~^~:;.
On Jan 18, 2009, at 20:07 , Alexandre Bergel wrote:
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.
Sorry, I misread the merger. After having loaded Kernel-adrian_lienhard.252, testTraitsUsersSanity goes red (where testUsersWithClassChanges goes green). It seems that the following traits do not include their users: an IdentitySet(SetTest ArrayTest OrderedCollectionTest BagTest)
Did I do something wrong?
maybe you have not loaded the script I posted as a comment on http://code.google.com/p/pharo/issues/detail?id=443 I'll add that in the change set of the update stream when I push this out. Adrian
Cheers, Alexandre
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@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@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo- project
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo- project
_______________________________________________ Pharo-project mailing list Pharo-project@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@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
_______________________________________________ Pharo-project mailing list Pharo-project@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@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
On Jan 18, 2009, at 7:29 PM, Alexandre Bergel wrote:
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
In one of the comment, Marcus said this is fixed already.
normally yes it was. I rely on the fix for moose extraction.
Cheers, Alexandre
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? 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.
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@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@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo- project
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
_______________________________________________ Pharo-project mailing list Pharo-project@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@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@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
I will try to check but in this moment this is crazy (was sick still a bit and 2 paper deadline + one deliverable) On Jan 18, 2009, at 9:05 PM, Adrian Lienhard wrote:
On Jan 18, 2009, at 20:07 , Alexandre Bergel wrote:
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.
Sorry, I misread the merger. After having loaded Kernel-adrian_lienhard.252, testTraitsUsersSanity goes red (where testUsersWithClassChanges goes green). It seems that the following traits do not include their users: an IdentitySet(SetTest ArrayTest OrderedCollectionTest BagTest)
Did I do something wrong?
maybe you have not loaded the script I posted as a comment on
http://code.google.com/p/pharo/issues/detail?id=443
I'll add that in the change set of the update stream when I push this out.
Adrian
Cheers, Alexandre
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@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@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo- project
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo- project
_______________________________________________ Pharo-project mailing list Pharo-project@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@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
_______________________________________________ Pharo-project mailing list Pharo-project@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@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
It works now! Cheers, Alexandre On 18 Jan 2009, at 21:05, Adrian Lienhard wrote:
On Jan 18, 2009, at 20:07 , Alexandre Bergel wrote:
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.
Sorry, I misread the merger. After having loaded Kernel-adrian_lienhard.252, testTraitsUsersSanity goes red (where testUsersWithClassChanges goes green). It seems that the following traits do not include their users: an IdentitySet(SetTest ArrayTest OrderedCollectionTest BagTest)
Did I do something wrong?
maybe you have not loaded the script I posted as a comment on
http://code.google.com/p/pharo/issues/detail?id=443
I'll add that in the change set of the update stream when I push this out.
Adrian
Cheers, Alexandre
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@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@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo- project
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo- project
_______________________________________________ Pharo-project mailing list Pharo-project@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@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
_______________________________________________ Pharo-project mailing list Pharo-project@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@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
-- _,.;:~^~:;._,.;:~^~:;._,.;:~^~:;._,.;:~^~:;._,.;: Alexandre Bergel http://www.bergel.eu ^~:;._,.;:~^~:;._,.;:~^~:;._,.;:~^~:;._,.;:~^~:;.
hi adrian I'm reading your fixes and I have one question in the following class: oldClass instanceVariableNames: instVarString unsafe: unsafe "This is the basic initialization message to change the definition of an existing Metaclass" | instVars newClass needNew copyOfOldClass copyOfOldTraitComposition copyOfOldClassTraitComposition | environ := oldClass environment. instVars := Scanner new scanFieldNames: instVarString. unsafe ifFalse:[ "Run validation checks so we know that we have a good chance for recompilation" (self validateInstvars: instVars from: oldClass forSuper: oldClass superclass) ifFalse:[^nil]. (self validateSubclassFormat: oldClass typeOfClass from: oldClass forSuper: oldClass superclass extra: instVars size) ifFalse:[^nil]]. "See if we need a new subclass or not" needNew := self needsSubclassOf: oldClass superclass type: oldClass typeOfClass instanceVariables: instVars from: oldClass. needNew ifNil:[^nil]. "some error" needNew ifFalse:[^oldClass]. "no new class needed" "Create the new class" copyOfOldClass := oldClass copy. oldClass hasTraitComposition ifTrue: [ copyOfOldTraitComposition := oldClass traitComposition copyTraitExpression ]. oldClass class hasTraitComposition ifTrue: [ copyOfOldClassTraitComposition := oldClass class traitComposition copyTraitExpression ]. newClass := self newSubclassOf: oldClass superclass type: oldClass typeOfClass instanceVariables: instVars from: oldClass. newClass := self recompile: false from: oldClass to: newClass mutate: false. "... set trait composition..." copyOfOldTraitComposition ifNotNil: [ newClass setTraitComposition: copyOfOldTraitComposition ]. copyOfOldClassTraitComposition ifNotNil: [ newClass class setTraitComposition: copyOfOldClassTraitComposition ]. self doneCompiling: newClass. SystemChangeNotifier uniqueInstance classDefinitionChangedFrom: copyOfOldClass to: newClass. ^newClass to get class: oldClass instanceVariableNames: instVarString unsafe: unsafe "This is the basic initialization message to change the definition of an existing Metaclass" | instVars newClass needNew copyOfOldClass | environ := oldClass environment. instVars := Scanner new scanFieldNames: instVarString. unsafe ifFalse:[ "Run validation checks so we know that we have a good chance for recompilation" (self validateInstvars: instVars from: oldClass forSuper: oldClass superclass) ifFalse:[^nil]. (self validateSubclassFormat: oldClass typeOfClass from: oldClass forSuper: oldClass superclass extra: instVars size) ifFalse:[^nil]]. "See if we need a new subclass or not" needNew := self needsSubclassOf: oldClass superclass type: oldClass typeOfClass instanceVariables: instVars from: oldClass. needNew ifNil:[^nil]. "some error" needNew ifFalse:[^oldClass]. "no new class needed" "Create the new class" copyOfOldClass := oldClass copy. newClass := self newSubclassOf: oldClass superclass type: oldClass typeOfClass instanceVariables: instVars from: oldClass. newClass := self recompile: false from: oldClass to: newClass mutate: false. self doneCompiling: newClass. SystemChangeNotifier uniqueInstance classDefinitionChangedFrom: copyOfOldClass to: newClass. ^newClass and change newclass newSubclassOf: newSuper type: type instanceVariables: instVars from: oldClass "Create a new subclass of the given superclass with the given specification." | newFormat newClass | "Compute the format of the new class" newFormat := self computeFormat: type instSize: instVars size forSuper: newSuper ccIndex: (oldClass ifNil:[0] ifNotNil:[oldClass indexIfCompact]). newFormat == nil ifTrue:[^nil]. (oldClass == nil or:[oldClass isMeta not]) ifTrue:[newClass := self privateNewSubclassOf: newSuper from: oldClass] ifFalse:[newClass := oldClass clone]. newClass superclass: newSuper methodDictionary: MethodDictionary new format: newFormat; setInstVarNames: instVars. oldClass ifNotNil:[ newClass organization: oldClass organization. "Recompile the new class" oldClass hasMethods ifTrue:[newClass compileAllFrom: oldClass]. oldClass hasTraitComposition ifTrue: [ newClass setTraitComposition: oldClass traitComposition copyTraitExpression ]. oldClass class hasTraitComposition ifTrue: [ newClass class setTraitComposition: oldClass class traitComposition copyTraitExpression ]. self recordClass: oldClass replacedBy: newClass. ]. (oldClass == nil or:[oldClass isObsolete not]) ifTrue:[newSuper addSubclass: newClass] ifFalse:[newSuper addObsoleteSubclass: newClass]. ^newClass Now I was wondering why you did not change newClass := self recompile: false from: oldClass to: newClass mutate: false. too Stef
On Jan 20, 2009, at 09:49 , Stéphane Ducasse wrote:
Now I was wondering why you did not change newClass := self recompile: false from: oldClass to: newClass mutate: false. too
Not sure if I understand what you mean. The message #recompile:from:to:mutate: is sent from the two methods I changed. The important change is that the transfer of the trait composition has to be done into the class returned by this method, not into the previous value of newClass (that is, before and and after the call, the variable newClass points to different instances). Adrian
My question was that you removed two traits block of code in one method and added one only to one method. so I was wondering why this was not to two methods (and recompile: false from: oldClass to: newClass was the one you did not change). On Jan 21, 2009, at 2:28 PM, Adrian Lienhard wrote:
On Jan 20, 2009, at 09:49 , Stéphane Ducasse wrote:
Now I was wondering why you did not change newClass := self recompile: false from: oldClass to: newClass mutate: false. too
Not sure if I understand what you mean.
The message #recompile:from:to:mutate: is sent from the two methods I changed. The important change is that the transfer of the trait composition has to be done into the class returned by this method, not into the previous value of newClass (that is, before and and after the call, the variable newClass points to different instances).
Adrian _______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
participants (4)
-
Adrian Lienhard -
Alexandre Bergel -
Damien Cassou -
Stéphane Ducasse