Okay, I found the invalid super-pointer behavior.

It's a lot better with Bert & Tobias' fix, but still there to a smaller degree..

Object compile: 'test  ^''Hello from Object'''.
(Object subclass: #MyClass instanceVariableNames: ''
classVariableNames: '' poolDictionaries: '' category: 'Test') compile:
'test  self halt.  ^ super test, ''my super call lands on myself!'''.
(Smalltalk classNamed: #MyClass) rename: 'MyNewAbstraction'.
((Smalltalk classNamed: #MyNewAbstraction) subclass: #MyClass
instanceVariableNames: '' classVariableNames: '' poolDictionaries: ''
category: 'Test').
(Smalltalk classNamed: #MyClass) new test

Do-it on the above.  When the debugger appears, simply highlight the
code "super test" in the debugger and press "Print It".

I expected to see 'Hello from Object' but it seems to be calculating
super from the receiver class rather than the method's home class...

WITHOUT Bert and Tobias' fix, the above results in the super call
truly landing on itself, and if you take the halt out, the
tight-recursion I mentioned from teh first paragraph of this thread.
WITH their fix, the problem only manifests in the debugger as
demonstrated.

See, I'm not crazy!  :)


On Tue, Jan 20, 2015 at 6:05 PM, Nicolas Cellier
<nicolas.cellier.aka.nice at gmail.com> wrote:
>
>
> 2015-01-20 23:56 GMT+01:00 Chris Muller <asqueaker at gmail.com>:
>>
>> Hi Eliot, there were TWO problems, Bert and Tobias have fixed one of
>> them.  The second one is still unfixed in trunk.  When you run this
>> script and arrive at the debugger, step INTO:
>>
>> Object compile: 'test  ^''Hello from Object'''.
>> (Object subclass: #MyClass instanceVariableNames: ''
>> classVariableNames: '' poolDictionaries: '' category: 'Test') compile:
>> 'test  ^ super halt test, ''my super call lands on itself!'''.
>> (Smalltalk classNamed: #MyClass) rename: 'MyNewAbstraction'.
>> (Smalltalk classNamed: #MyNewAbstraction) new test
>>
>
> But it behaves as it should...
> You first send halt to super.
> If you resume it just answer self.
>
> Then you send test to self (it's not a super send and never been)
>
>
>>
>> then be sure to step INTO the super call to #test.  See how it lands
>> you in teh same method?  THAT bug exists back at least as far back as
>> 4.2, so I don't think its Environments related.
>>
>> I took a look at it and presented my findings in the other email
>> thread with subject "The Inbox: Environments-cmm.52.mcz", -- that
>> screenshot showing that the bytecodes are teh same and the literal
>> binding looks correct.
>>
>> So I have no idea why its not calling super correctly...
>>
>>
>> On Tue, Jan 20, 2015 at 2:38 PM, Eliot Miranda <eliot.miranda at gmail.com>
>> wrote:
>> >
>> > Ugh, I'm behind the times, Tobias you've already fixed it; thanks!
>> > Tobias, I like your use of becomeForward:; it is more robust.  But I *think*
>> > the undeclared removal is misplaced.  Don't you think that that code belongs
>> > in the BindingPolicy?
>> >
>> > On Tue, Jan 20, 2015 at 12:33 PM, Eliot Miranda
>> > <eliot.miranda at gmail.com> wrote:
>> >>
>> >>
>> >>
>> >> On Tue, Jan 20, 2015 at 12:26 PM, Eliot Miranda
>> >> <eliot.miranda at gmail.com> wrote:
>> >>>
>> >>> Hi Chris, Hi Colin,
>> >>>
>> >>>
>> >>> On Thu, Jan 15, 2015 at 2:48 PM, Chris Muller <ma.chris.m at gmail.com>
>> >>> wrote:
>> >>>>
>> >>>> I did a minor class-hierarchy refactoring today which resulted in a
>> >>>> an
>> >>>> invalid super pointer, resulting in a confusing image lock up due to
>> >>>> tight recursion.  The script, below, demonstrates the issue in trunk:
>> >>>>
>> >>>> Object compile: 'test  ^''Hello from Object'''.
>> >>>> (Object subclass: #MyClass instanceVariableNames: ''
>> >>>> classVariableNames: '' poolDictionaries: '' category: 'Test')
>> >>>> compile:
>> >>>> 'test  ^ super halt test, ''my super call lands on myself!'''.
>> >>>> (Smalltalk classNamed: #MyClass) rename: 'MyNewAbstraction'.
>> >>>> ((Smalltalk classNamed: #MyNewAbstraction) subclass: #MyClass
>> >>>> instanceVariableNames: '' classVariableNames: '' poolDictionaries: ''
>> >>>> category: 'Test').
>> >>>> (Smalltalk classNamed: #MyNewAbstraction) new test
>> >>>>
>> >>>> But an existing method in the original MyClass had a call to super
>> >>>> which now lands in itself, resulting in a tight recursion.
>> >>>>
>> >>>> The same script without creating the new subclass results in an
>> >>>> instant VM crash (both interpreter 2357 and cog 3205):
>> >>>>
>> >>>> Object compile: 'test  ^''Hello from Object'''.
>> >>>> (Object subclass: #MyClass instanceVariableNames: ''
>> >>>> classVariableNames: '' poolDictionaries: '' category: 'Test')
>> >>>> compile:
>> >>>> 'test  ^ super halt test, ''my super call lands on itself!'''.
>> >>>> (Smalltalk classNamed: #MyClass) rename: 'MyNewAbstraction'.
>> >>>> (Smalltalk classNamed: #MyNewAbstraction) new test
>> >>>>
>> >>>> I even tried recompiling my entire class hierarchy which was renamed,
>> >>>> but it didn't help.
>> >>>>
>> >>>> How can this be?  I _KNOW_ I've used rename class many times in the
>> >>>> past...  A regression?
>> >>>
>> >>>
>> >>> This is indeed an Environments regression.  The issue is where to fix
>> >>> the problem.  One way to look at it is that
>> >>> Environment>>renameClass:from:to: does not update bindings in the renamed
>> >>> class's methods.
>> >>>
>> >>> Here's what the old code did:
>> >>>
>> >>> SystemDictionary>>renameClass: aClass from: oldName to: newName
>> >>> "Rename the class, aClass, to have the title newName."
>> >>>
>> >>> | oldref category |
>> >>> category := SystemOrganization categoryOfElement: oldName.
>> >>> self organization classify: newName under: category suppressIfDefault:
>> >>> true.
>> >>> self organization removeElement: oldName.
>> >>> oldref := self associationAt: oldName.
>> >>> self removeKey: oldName.
>> >>> ** oldref key: newName.
>> >>> self add: oldref.  "Old association preserves old refs"
>> >>> Smalltalk renamedClass: aClass from: oldName to: newName.
>> >>> self flushClassNameCache.
>> >>> SystemChangeNotifier uniqueInstance classRenamed: aClass from: oldName
>> >>> to: newName inCategory: category
>> >>>
>> >>> The starred line changes the key on the old class's binding, meaning
>> >>> that all the methods whose methodClassAssociation is that binding get
>> >>> automatically updated (note that class methods have a methodClassAssociation
>> >>> that has no key and just points to the class object).
>> >>>
>> >>> The new code simply forgets to do this:
>> >>>
>> >>> Environment>>renameClass: aClass from: oldName to: newName
>> >>> "Rename the class, aClass, to have the title newName."
>> >>>
>> >>> | binding category |
>> >>> category := self organization categoryOfElement: oldName.
>> >>> self organization classify: newName under: category suppressIfDefault:
>> >>> true.
>> >>> self organization removeElement: oldName.
>> >>> binding := self declarationOf: oldName.
>> >>> declarations removeKey: oldName.
>> >>> self binding: binding removedFrom: self.
>> >>> binding := newName => aClass.
>> >>> declarations add: binding.
>> >>> self binding: binding addedTo: self.
>> >>> Smalltalk renamedClass: aClass from: oldName to: newName.
>> >>> SystemChangeNotifier uniqueInstance
>> >>> classRenamed: aClass
>> >>> from: oldName
>> >>> to: newName
>> >>> inCategory: category
>> >>>
>> >>>
>> >>> But should the change go in Environment>>renameClass:from:to: or in
>> >>> the BindingPolicy?  i.e. it could be implemented either in
>> >>> Environment>>renameClass:from:to:, or in Environment>>showBinding:.  I guess
>> >>> the former, but I'm cc'ing Colin, because know it?, he wrote it!
>> >>
>> >>
>> >> and if the fix should indeed go in Environment>>renameClass:from:to: ,
>> >> it is as simple as
>> >>
>> >> Environment>>renameClass: aClass from: oldName to: newName
>> >> "Rename the class, aClass, to have the title newName."
>> >>
>> >> | binding category |
>> >> category := self organization categoryOfElement: oldName.
>> >> self organization classify: newName under: category suppressIfDefault:
>> >> true.
>> >> self organization removeElement: oldName.
>> >> binding := self declarationOf: oldName.
>> >> declarations removeKey: oldName.
>> >> self binding: binding removedFrom: self.
>> >> binding := newName => aClass.
>> >> declarations add: binding.
>> >> self binding: binding addedTo: self.
>> >>
>> >> ** aClass updateMethodBindingsTo: binding.
>> >> Smalltalk renamedClass: aClass from: oldName to: newName.
>> >> SystemChangeNotifier uniqueInstance
>> >> classRenamed: aClass
>> >> from: oldName
>> >> to: newName
>> >> inCategory: category
>> >>
>> >> I'll make this change and we can alter later if Colin thinks it's in
>> >> the wrong place.
>> >>
>> >>>
>> >>>
>> >>> In any case there needs to be a test that checks that a method's
>> >>> methodClassAssociation is correctly updated after a rename.  I bet Colin
>> >>> would have not made this slip had there been a test.
>> >>>
>> >>>
>> >>> Here's the code I used to test this:
>> >>>
>> >>> | mca1 mca2 |
>> >>> Object compile: 'test  ^''Hello from Object'''.
>> >>> (Object
>> >>> subclass: #MyClass instanceVariableNames: ''
>> >>> classVariableNames: ''
>> >>> poolDictionaries: ''
>> >>> category: 'Test')
>> >>> compile: 'test  ^ super halt test, ''my super call lands on
>> >>> itself!'''.
>> >>> mca1 := ((Smalltalk classNamed: #MyClass)>>#test)
>> >>> methodClassAssociation.
>> >>> (Smalltalk classNamed: #MyClass)
>> >>> rename: 'MyNewAbstraction'.
>> >>> mca2 := ((Smalltalk classNamed: #MyNewAbstraction)>>#test)
>> >>> methodClassAssociation.
>> >>> { mca1. mca2. mca1 == mca2. mca1 == (Smalltalk bindingOf:
>> >>> #MyNewAbstraction). mca2 == (Smalltalk bindingOf: #MyNewAbstraction)}
>> >>>
>> >>> It answers { #MyClass=>nil . #MyClass=>nil . true . false . false}
>> >>> i.e. the binding isn't updated, but changes from #MyClass=>MyClass to
>> >>> #MyClass=>nil in binding:removedFrom:.
>> >>>
>> >>>
>> >>> --
>> >>> HTH,
>> >>> Eliot
>> >>
>> >>
>> >>
>> >>
>> >> --
>> >> best,
>> >> Eliot
>> >
>> >
>> >
>> >
>> > --
>> > best,
>> > Eliot
>> >
>> >
>> >
>>
>
>
>
>