On Sun, Jan 18, 2015 at 9:13 PM, Levente Uzonyi <leves at elte.hu> wrote:

> On Sun, 18 Jan 2015, commits at source.squeak.org wrote:
>
>  Chris Muller uploaded a new version of Environments to project The Trunk:
>> http://source.squeak.org/trunk/Environments-cmm.53.mcz
>>
>> ==================== Summary ====================
>>
>> Name: Environments-cmm.53
>> Author: cmm
>> Time: 16 January 2015, 2:54:24.767 pm
>> UUID: 69b32fc7-98f1-4c19-9976-a38106a9ee63
>> Ancestors: Environments-cmm.52
>>
>> - Only change key when it is NOT in the 'bindings' Dictionary, otherwise
>> it would be in a state inconsistent with its new hash.
>>
>> =============== Diff against Environments-cmm.51 ===============
>>
>> Item was changed:
>>  ----- Method: Environment>>renameClass:from:to: (in category 'classes
>> and traits') -----
>>  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 associationAt: oldName.
>> -       binding := self declarationOf: oldName.
>>
>
> The Dictionary API shouldn't be used from new code, especially not from
> the Environments code itself.
> What was wrong with #declarationOf:? As I see the only difference is that
> it'll return nil instead of raising an error (#associationAt:). But that's
> not a problem, because the next line will raise an error anyway.


Okay, we should change it back to declarationOf: then.  I simply put it
back like it was prior to cwp.50.

         declarations removeKey: oldName.
>> +       bindings removeKey: oldName.
>> + "     self binding: binding removedFrom: self."
>> -       self binding: binding removedFrom: self.
>>
>> +       binding key: newName.
>> +       declarations add: binding.
>> +       bindings add: binding.
>> + "     self binding: binding addedTo: self."
>> -       binding := newName => aClass.
>> -       declarations add: binding.
>> -       self binding: binding addedTo: self.
>>
>
> I suspect that the removal of the #binding:removedFrom: and the
> #binding:addedTo: sends will break Environments, because another
> environment which imports this environment will not be notified about
> changes which happen here.
>

The calls ot those are what had rename-class so severely broken.  I tried
to understand a proper fix but #binding:removedFrom: does a #perform on its
'removeSelector', (#hideBinding: in this case), which ends up doing a
becomeForward: on the binding from the looked-up namespace to the one in
the the receiver Environments 'bindings'.  That kind of code is way too
smart for me, and since #hideBinding: is the removeSelector used by all of
Environments main "import" API, I decided to make this temporary fix
isolated to the renameClass function, to minimize the impact on
Environments.

In a nutshell, cwp.50 was attempting to fix rename class, but it made it
worse, so this reversion lets us go back and "try again".

This problem is shadowed in case of Smalltalk - which imports itself - by
> the direct manipulation of the bindings dictionary.


I don't care what solution we have as long as regular development works.



>
> Levente
>
>
>
>>         Smalltalk renamedClass: aClass from: oldName to: newName.
>>         SystemChangeNotifier uniqueInstance
>>                 classRenamed: aClass
>>                 from: oldName
>>                 to: newName
>>                 inCategory: category!
>>
>>
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20150119/1e77f094/attachment.htm