The Trunk: Environments-cmm.53.mcz
Locked
 
|
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. 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. Smalltalk renamedClass: aClass from: oldName to: newName. SystemChangeNotifier uniqueInstance classRenamed: aClass from: oldName to: newName inCategory: category! |
Re: The Trunk: Environments-cmm.53.mcz
|
|
I'm moving this to trunk now because I've just noticed a painful side-effect of this bug [1]. After renaming a class, the ClassBindings in the methods of the renamed class will be corrupted. Know that if you try to save the package with the renamed class, Monticello will \silently exclude/ methods of the renamed class (because MC will only save methods with #isLocalSelector). I didn't realize that until I found myself in my newly-built image with several methods missing from my MC package. Thankfully I still had the old image to develop and run this script [2] to identify and replace the incorrect ClassBindings in the CM's of the Class which was renamed. If anyone else has used rename on the class menu, I hope this script will help you fix your image. If you had already saved a package after a class rename, be sure to select "check all packages for changes" from the MC working copy menu after running the script. - Chris [1] -- We have two bugs. "this bug" refers to the one fixed here in #renameClass:from:to:. The other one is the invalid super pointer and still unfixed. That problem is easy to reproduce by that script in my original email ("invalid super pointer"), and we really could use a CM-structure / Bytecode expert to take a look, as it is also a pretty nasty bug. Thanks! [2] | notAtHome | notAtHome := Array streamContents: [ : stream | Smalltalk allClasses do: [ : eachClass | eachClass methodDictionary valuesDo: [ : eachCm | eachCm methodHome = eachClass ifFalse: [ stream nextPut: eachCm -> eachClass ]]] ]. notAtHome ifNotEmpty: [ Warning signal: 'The following methods were found to have a methodHome that was not their containing Class. They will be rehomed. ', (notAtHome) ]. notAtHome do: [ : eachCmAndCorrectHome | eachCmAndCorrectHome key literalAt: eachCmAndCorrectHome key numLiterals put: (Smalltalk environment bindingOf: eachCmAndCorrectHome value name) ] On Sun, Jan 18, 2015 at 5:44 PM, <[hidden email]> wrote: Chris Muller uploaded a new version of Environments to project The Trunk: |
Re: The Trunk: Environments-cmm.53.mcz
|
|
In reply to this post by commits-2
On Sun, 18 Jan 2015, [hidden email] 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. > 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. This problem is shadowed in case of Smalltalk - which imports itself - by the direct manipulation of the bindings dictionary. Levente > > Smalltalk renamedClass: aClass from: oldName to: newName. > SystemChangeNotifier uniqueInstance > classRenamed: aClass > from: oldName > to: newName > inCategory: category! > > > |
Re: The Trunk: Environments-cmm.53.mcz
|
|
On Sun, Jan 18, 2015 at 9:13 PM, Levente Uzonyi <[hidden email]> wrote: On Sun, 18 Jan 2015, [hidden email] wrote: Okay, we should change it back to declarationOf: then. I simply put it back like it was prior to cwp.50. declarations removeKey: oldName. 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.
|
«
Return to Squeak - Dev
|
1 view|%1 views
| Free forum by Nabble | Edit this page |