<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">2015-01-20 23:56 GMT+01:00 Chris Muller <span dir="ltr"><<a href="mailto:asqueaker@gmail.com" target="_blank">asqueaker@gmail.com</a>></span>:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Eliot, there were TWO problems, Bert and Tobias have fixed one of<br>
them. The second one is still unfixed in trunk. When you run this<br>
script and arrive at the debugger, step INTO:<br>
<br>
Object compile: 'test ^''Hello from Object'''.<br>
(Object subclass: #MyClass instanceVariableNames: ''<br>
classVariableNames: '' poolDictionaries: '' category: 'Test') compile:<br>
'test ^ super halt test, ''my super call lands on itself!'''.<br>
(Smalltalk classNamed: #MyClass) rename: 'MyNewAbstraction'.<br>
(Smalltalk classNamed: #MyNewAbstraction) new test<br>
<br></blockquote><div><br></div><div>But it behaves as it should...<br></div><div>You first send halt to super.<br></div><div>If you resume it just answer self.<br><br></div><div>Then you send test to self (it's not a super send and never been)<br><br> <br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
then be sure to step INTO the super call to #test. See how it lands<br>
you in teh same method? THAT bug exists back at least as far back as<br>
4.2, so I don't think its Environments related.<br>
<br>
I took a look at it and presented my findings in the other email<br>
thread with subject "The Inbox: Environments-cmm.52.mcz", -- that<br>
screenshot showing that the bytecodes are teh same and the literal<br>
binding looks correct.<br>
<br>
So I have no idea why its not calling super correctly...<br>
<br>
<br>
On Tue, Jan 20, 2015 at 2:38 PM, Eliot Miranda <<a href="mailto:eliot.miranda@gmail.com">eliot.miranda@gmail.com</a>> wrote:<br>
><br>
> 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?<br>
><br>
> On Tue, Jan 20, 2015 at 12:33 PM, Eliot Miranda <<a href="mailto:eliot.miranda@gmail.com">eliot.miranda@gmail.com</a>> wrote:<br>
>><br>
>><br>
>><br>
>> On Tue, Jan 20, 2015 at 12:26 PM, Eliot Miranda <<a href="mailto:eliot.miranda@gmail.com">eliot.miranda@gmail.com</a>> wrote:<br>
>>><br>
>>> Hi Chris, Hi Colin,<br>
>>><br>
>>><br>
>>> On Thu, Jan 15, 2015 at 2:48 PM, Chris Muller <<a href="mailto:ma.chris.m@gmail.com">ma.chris.m@gmail.com</a>> wrote:<br>
>>>><br>
>>>> I did a minor class-hierarchy refactoring today which resulted in a an<br>
>>>> invalid super pointer, resulting in a confusing image lock up due to<br>
>>>> tight recursion. The script, below, demonstrates the issue in trunk:<br>
>>>><br>
>>>> Object compile: 'test ^''Hello from Object'''.<br>
>>>> (Object subclass: #MyClass instanceVariableNames: ''<br>
>>>> classVariableNames: '' poolDictionaries: '' category: 'Test') compile:<br>
>>>> 'test ^ super halt test, ''my super call lands on myself!'''.<br>
>>>> (Smalltalk classNamed: #MyClass) rename: 'MyNewAbstraction'.<br>
>>>> ((Smalltalk classNamed: #MyNewAbstraction) subclass: #MyClass<br>
>>>> instanceVariableNames: '' classVariableNames: '' poolDictionaries: ''<br>
>>>> category: 'Test').<br>
>>>> (Smalltalk classNamed: #MyNewAbstraction) new test<br>
>>>><br>
>>>> But an existing method in the original MyClass had a call to super<br>
>>>> which now lands in itself, resulting in a tight recursion.<br>
>>>><br>
>>>> The same script without creating the new subclass results in an<br>
>>>> instant VM crash (both interpreter 2357 and cog 3205):<br>
>>>><br>
>>>> Object compile: 'test ^''Hello from Object'''.<br>
>>>> (Object subclass: #MyClass instanceVariableNames: ''<br>
>>>> classVariableNames: '' poolDictionaries: '' category: 'Test') compile:<br>
>>>> 'test ^ super halt test, ''my super call lands on itself!'''.<br>
>>>> (Smalltalk classNamed: #MyClass) rename: 'MyNewAbstraction'.<br>
>>>> (Smalltalk classNamed: #MyNewAbstraction) new test<br>
>>>><br>
>>>> I even tried recompiling my entire class hierarchy which was renamed,<br>
>>>> but it didn't help.<br>
>>>><br>
>>>> How can this be? I _KNOW_ I've used rename class many times in the<br>
>>>> past... A regression?<br>
>>><br>
>>><br>
>>> 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.<br>
>>><br>
>>> Here's what the old code did:<br>
>>><br>
>>> SystemDictionary>>renameClass: aClass from: oldName to: newName<br>
>>> "Rename the class, aClass, to have the title newName."<br>
>>><br>
>>> | oldref category |<br>
>>> category := SystemOrganization categoryOfElement: oldName.<br>
>>> self organization classify: newName under: category suppressIfDefault: true.<br>
>>> self organization removeElement: oldName.<br>
>>> oldref := self associationAt: oldName.<br>
>>> self removeKey: oldName.<br>
>>> ** oldref key: newName.<br>
>>> self add: oldref. "Old association preserves old refs"<br>
>>> Smalltalk renamedClass: aClass from: oldName to: newName.<br>
>>> self flushClassNameCache.<br>
>>> SystemChangeNotifier uniqueInstance classRenamed: aClass from: oldName to: newName inCategory: category<br>
>>><br>
>>> 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).<br>
>>><br>
>>> The new code simply forgets to do this:<br>
>>><br>
>>> Environment>>renameClass: aClass from: oldName to: newName<br>
>>> "Rename the class, aClass, to have the title newName."<br>
>>><br>
>>> | binding category |<br>
>>> category := self organization categoryOfElement: oldName.<br>
>>> self organization classify: newName under: category suppressIfDefault: true.<br>
>>> self organization removeElement: oldName.<br>
>>> binding := self declarationOf: oldName.<br>
>>> declarations removeKey: oldName.<br>
>>> self binding: binding removedFrom: self.<br>
>>> binding := newName => aClass.<br>
>>> declarations add: binding.<br>
>>> self binding: binding addedTo: self.<br>
>>> Smalltalk renamedClass: aClass from: oldName to: newName.<br>
>>> SystemChangeNotifier uniqueInstance<br>
>>> classRenamed: aClass<br>
>>> from: oldName<br>
>>> to: newName<br>
>>> inCategory: category<br>
>>><br>
>>><br>
>>> 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!<br>
>><br>
>><br>
>> and if the fix should indeed go in Environment>>renameClass:from:to: , it is as simple as<br>
>><br>
>> Environment>>renameClass: aClass from: oldName to: newName<br>
>> "Rename the class, aClass, to have the title newName."<br>
>><br>
>> | binding category |<br>
>> category := self organization categoryOfElement: oldName.<br>
>> self organization classify: newName under: category suppressIfDefault: true.<br>
>> self organization removeElement: oldName.<br>
>> binding := self declarationOf: oldName.<br>
>> declarations removeKey: oldName.<br>
>> self binding: binding removedFrom: self.<br>
>> binding := newName => aClass.<br>
>> declarations add: binding.<br>
>> self binding: binding addedTo: self.<br>
>><br>
>> ** aClass updateMethodBindingsTo: binding.<br>
>> Smalltalk renamedClass: aClass from: oldName to: newName.<br>
>> SystemChangeNotifier uniqueInstance<br>
>> classRenamed: aClass<br>
>> from: oldName<br>
>> to: newName<br>
>> inCategory: category<br>
>><br>
>> I'll make this change and we can alter later if Colin thinks it's in the wrong place.<br>
>><br>
>>><br>
>>><br>
>>> 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.<br>
>>><br>
>>><br>
>>> Here's the code I used to test this:<br>
>>><br>
>>> | mca1 mca2 |<br>
>>> Object compile: 'test ^''Hello from Object'''.<br>
>>> (Object<br>
>>> subclass: #MyClass instanceVariableNames: ''<br>
>>> classVariableNames: ''<br>
>>> poolDictionaries: ''<br>
>>> category: 'Test')<br>
>>> compile: 'test ^ super halt test, ''my super call lands on itself!'''.<br>
>>> mca1 := ((Smalltalk classNamed: #MyClass)>>#test) methodClassAssociation.<br>
>>> (Smalltalk classNamed: #MyClass)<br>
>>> rename: 'MyNewAbstraction'.<br>
>>> mca2 := ((Smalltalk classNamed: #MyNewAbstraction)>>#test) methodClassAssociation.<br>
>>> { mca1. mca2. mca1 == mca2. mca1 == (Smalltalk bindingOf: #MyNewAbstraction). mca2 == (Smalltalk bindingOf: #MyNewAbstraction)}<br>
>>><br>
>>> It answers { #MyClass=>nil . #MyClass=>nil . true . false . false}<br>
>>> i.e. the binding isn't updated, but changes from #MyClass=>MyClass to #MyClass=>nil in binding:removedFrom:.<br>
>>><br>
>>><br>
>>> --<br>
>>> HTH,<br>
>>> Eliot<br>
>><br>
>><br>
>><br>
<span class="HOEnZb"><font color="#888888">>><br>
>> --<br>
>> best,<br>
>> Eliot<br>
><br>
><br>
><br>
><br>
> --<br>
> best,<br>
> Eliot<br>
><br>
><br>
><br>
<br>
</font></span></blockquote></div><br></div></div>