OMG....  :$  LOL!!  I'm sooo embarassed..!

I never write "super someMethodOtherThanTheOneImIn" but I sure did
that with "super halt" didn't I?  Duh.

Now I'm wondering what my recursion was that locked up because it
didn't have a halt in the loop..  Ugg, sorry, if I find this bug again
I'll take better care to recreate it.  :)

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
>> >
>> >
>> >
>>
>
>
>
>