Issue 7352 in pharo: Changing a superclass removes the Class from the superclass' subclasses

Status: HorribleErrorBlockingE…erything
Owner: [hidden email]
Labels: Type-Bug Milestone-2.0 Importance-High

New issue 7352 by [hidden email]: Changing a superclass removes the  
Class from the superclass' subclasses
http://code.google.com/p/pharo/issues/detail?id=7352

Object subclass: #Foo
        instanceVariableNames: ''
        classVariableNames: ''
        poolDictionaries: ''
        category: 'Refactoring-Critics'.

Foo subclass: #RBLintRule
        instanceVariableNames: ''
        classVariableNames: ''
        poolDictionaries: ''
        category: 'Refactoring-Critics'.

ClassHierarchyTest debug: #testSubclasses

_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Updates:
        Cc: [hidden email]

Comment #1 on issue 7352 by [hidden email]: Changing a superclass  
removes the Class from the superclass' subclasses
http://code.google.com/p/pharo/issues/detail?id=7352

(No comment was entered for this change.)


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Updates:
        Cc: [hidden email] [hidden email]

Comment #2 on issue 7352 by [hidden email]: Changing a superclass  
removes the Class from the superclass' subclasses
http://code.google.com/p/pharo/issues/detail?id=7352

(No comment was entered for this change.)


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Comment #3 on issue 7352 by [hidden email]: Changing a superclass  
removes the Class from the superclass' subclasses
http://code.google.com/p/pharo/issues/detail?id=7352

see also the failing test:
https://ci.inria.fr/pharo/job/Pharo-2.0-Tests/lastCompletedBuild/label=win/testReport/KernelTests.Classes/ClassHierarchyTest/testSubclasses/


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Comment #4 on issue 7352 by [hidden email]: Changing a superclass  
removes the Class from the superclass' subclasses
http://code.google.com/p/pharo/issues/detail?id=7352

This again is a side effect of

  Issue 7240




_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Comment #5 on issue 7352 by [hidden email]: Changing a superclass  
removes the Class from the superclass' subclasses
http://code.google.com/p/pharo/issues/detail?id=7352

JuanPi and alejandro are working on it and this is really strange. Only two  
classes.


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Comment #6 on issue 7352 by [hidden email]: Changing a superclass  
removes the Class from the superclass' subclasses
http://code.google.com/p/pharo/issues/detail?id=7352

Marcus we checked 7240 and the change is

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 shallowCopy].

        newClass
                superclass: newSuper
                methodDictionary: MethodDictionary new
                format: newFormat;
                setInstVarNames: instVars.

        oldClass ifNotNil:[
                newClass organization: oldClass organization.
                newClass isMeta not ifTrue: [ newClass environment at: newClass name put:  
newClass ].
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
                 "Recompile the new class"
                oldClass hasMethods
                        ifTrue:[newClass compileAllFrom: oldClass].
                self recordClass: oldClass replacedBy: newClass.
        ].

        (oldClass == nil or:[oldClass isObsolete not])
                ifTrue:[newSuper addSubclass: newClass]
                ifFalse:[newSuper addObsoleteSubclass: newClass].

        ^newClass


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Comment #7 on issue 7352 by [hidden email]: Changing a superclass  
removes the Class from the superclass' subclasses
http://code.google.com/p/pharo/issues/detail?id=7352

I do not understand how it could work before and I do not see how the  
change could modif the subclass' superclass.


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Comment #8 on issue 7352 by [hidden email]: Changing a superclass  
removes the Class from the superclass' subclasses
http://code.google.com/p/pharo/issues/detail?id=7352

But we checked and when we removed this line
newClass isMeta not ifTrue: [ newClass environment at: newClass name put:  
newClass ].
we do not get the current bug anymore.


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Comment #9 on issue 7352 by [hidden email]: Changing a superclass  
removes the Class from the superclass' subclasses
http://code.google.com/p/pharo/issues/detail?id=7352

This is my fault. I will not touch the class builder again because its a  
total mess covered by only 9 tests.

For issue 7240, I installed the new class in its environment before all the  
methods from the old class gets recompiled to avoid having a nil key in the  
last literal of these methods.
The subclasses are now considered obsolete because the last test of the  
method checks:
(oldClass == nil or:[oldClass isObsolete not])
whereas it should check:
(oldClass == nil or:[newClass isObsolete not])
With this correction, *all* tests are green again but it does not mean a  
lot in this case, so beware...



_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Comment #10 on issue 7352 by [hidden email]: Changing a superclass  
removes the Class from the superclass' subclasses
http://code.google.com/p/pharo/issues/detail?id=7352

By *all* tests I mean that the only ones that are failing are not related


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Updates:
        Cc: [hidden email]

Comment #11 on issue 7352 by [hidden email]: Changing a superclass  
removes the Class from the superclass' subclasses
http://code.google.com/p/pharo/issues/detail?id=7352

(No comment was entered for this change.)


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Comment #12 on issue 7352 by [hidden email]: Changing a superclass  
removes the Class from the superclass' subclasses
http://code.google.com/p/pharo/issues/detail?id=7352

Ok thanks camille.
I will give a try later.





_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Updates:
        Status: WorkNeeded-Conclusion
        Labels: -Type-Bug Type-HorribleBugBlockingEverything

Comment #13 on issue 7352 by [hidden email]: Changing a superclass  
removes the Class from the superclass' subclasses
http://code.google.com/p/pharo/issues/detail?id=7352

(No comment was entered for this change.)


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Updates:
        Labels: -Type-HorribleBugBlockingEverything  
Type-HorribleErrorBlockingEverything

Comment #14 on issue 7352 by [hidden email]: Changing a superclass  
removes the Class from the superclass' subclasses
http://code.google.com/p/pharo/issues/detail?id=7352

(No comment was entered for this change.)


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Comment #15 on issue 7352 by [hidden email]: Changing a superclass  
removes the Class from the superclass' subclasses
http://code.google.com/p/pharo/issues/detail?id=7352

the problem with making the horrible error a lable is that this now got  
completely lost and nobody will look at it for weeks.


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Comment #16 on issue 7352 by [hidden email]: Changing a superclass  
removes the Class from the superclass' subclasses
http://code.google.com/p/pharo/issues/detail?id=7352

Issue 7340 has been merged into this issue.


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Updates:
        Status: FixReviewNeeded

Comment #17 on issue 7352 by [hidden email]: Changing a superclass  
removes the Class from the superclass' subclasses
http://code.google.com/p/pharo/issues/detail?id=7352

lets try the change

Name:  
SLICE-Issue-7352-Changing-a-superclass-removes-the-Class-from-the-superclass-subclasses-MarcusDenker.1
Author: MarcusDenker
Time: 1 February 2013, 11:11:53.987 am
UUID: 449ee33b-4e92-46ee-acff-6fb27f8fca53
Ancestors:
Dependencies: Kernel-MarcusDenker.1294

Issue 7352: Changing a superclass removes the Class from the superclass'  
subclasses


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Comment #18 on issue 7352 by [hidden email]: Changing a superclass  
removes the Class from the superclass' subclasses
http://code.google.com/p/pharo/issues/detail?id=7352

@marcus: why does the fact that it's a label make it not visible? it's just  
as easy to filter for label as it is for status, no? The old way, as soon  
as someone looked at it, the fact that it was horrible was lost because the  
status was changed to WorkNeeded, FixReviewNeeded, etc. Is it just a  
question of updating our workflow, or something deeper?


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Comment #19 on issue 7352 by [hidden email]: Changing a superclass  
removes the Class from the superclass' subclasses
http://code.google.com/p/pharo/issues/detail?id=7352

If "ClassHierarchyTest debug: #testSubclasses" is run in a fresh 20503,  
there are 6 assertion failures already in the image.

When evaluating:
Object subclass: #Foo
        instanceVariableNames: ''
        classVariableNames: ''
        poolDictionaries: ''
        category: 'Refactoring-Critics'.

Foo subclass: #RBLintRule
        instanceVariableNames: ''
        classVariableNames: ''
        poolDictionaries: ''
        category: 'Refactoring-Critics'.

The second statement, which changes the superclass, takes about 10 seconds.

Without the fix, if #testSubclasses is run again, there are many more  
assert failures. After the fix is applied, running #testSubclasses produces  
only the original 6 failures. The superclass change still takes 10  
seconds...


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker