"Radical" would be the #becomeForwards: calls?

If I read it right, this commit "just" updates the various caches
(references, exports), and updates undeclared, when something's added
to the Environment. It then also ensures that _removed_ bindings are
removed from the various caches. Should #removeKey:ifAbsent: possibly
update undeclared? Couldn't you still have CompiledMethods floating
around holding references to a class you just deleted from the
Environment?

Lastly, the postscript runs over all globals, finds those globals with
more than one binding, and forcibly unifies them all (through
#becomeForward:) to point to whatever Smalltalk globals points to.

I'd be interested in seeing tests exercising these changes directly,
but I realise that partly they will be, by virtue of making the
current test suite work.

frank

On 29 December 2013 21:28, Levente Uzonyi <leves at elte.hu> wrote:
> I've uploaded this version to the Inbox to let you review it before I push
> it to the Trunk. It fixes the most urgent issues of Environments, but it
> also includes some "radical" changes, and a "high impact" postscript.
>
>
> Levente
>
>
> On Sun, 29 Dec 2013, commits at source.squeak.org wrote:
>
>> A new version of Environments was added to project The Inbox:
>> http://source.squeak.org/inbox/Environments-ul.43.mcz
>>
>> ==================== Summary ====================
>>
>> Name: Environments-ul.43
>> Author: ul
>> Time: 29 December 2013, 10:04:32.454 pm
>> UUID: a91a7c62-f608-48aa-b296-2d541c0083f5
>> Ancestors: Environments-ul.42
>>
>> Fixed a typo in the postscript.
>>
>> =============== Diff against Environments-ul.40 ===============
>>
>> Item was changed:
>>  ----- Method: Environment>>at:ifAbsentPut: (in category 'emulating')
>> -----
>>  at: aSymbol ifAbsentPut: aBlock
>> +
>> +       ^declarations
>> -       ^ declarations
>>                 at: aSymbol
>> +               ifAbsent: [ self at: aSymbol put: aBlock value ]!
>> -               ifAbsentPut: aBlock!
>>
>> Item was changed:
>>  ----- Method: Environment>>at:put: (in category 'emulating') -----
>>  at: aSymbol put: anObject
>> +
>> +       | binding newBinding |
>> +       newBinding := aSymbol => anObject.
>> +       binding := declarations associationAt: aSymbol ifAbsent: [ nil ].
>> +       binding ifNotNil: [
>> +               binding class == newBinding class
>> +                       ifTrue: [ binding value: anObject ]
>> +                       ifFalse: [ binding becomeForward: newBinding ].
>> +               ^anObject ].
>> +       binding := undeclared associationAt: aSymbol ifAbsent: [ nil ].
>> +       binding
>> +               ifNil: [ binding := newBinding ]
>> +               ifNotNil: [
>> +                       undeclared removeKey: aSymbol.
>> +                       binding class == newBinding class
>> +                               ifTrue: [ binding value: anObject ]
>> +                               ifFalse: [ binding becomeForward:
>> newBinding ] ].
>> +       declarations add: binding.
>> +       references add: binding.
>> +       exports bind: binding. "Do we really want this?"
>> +       ^anObject
>> -       | binding |
>> -       (declarations includesKey: aSymbol)
>> -               ifTrue: [declarations at: aSymbol put: anObject]
>> -               ifFalse:
>> -                       [(undeclared includesKey: aSymbol)
>> -                               ifTrue:
>> -                                       [declarations declare: aSymbol
>> from: undeclared.
>> -                                       declarations at: aSymbol put:
>> anObject]
>> -                               ifFalse:
>> -                                       [binding := aSymbol => anObject.
>> -                                       declarations add: binding.
>> -                                       exports bind: binding]].
>> -       ^ anObject
>>  !
>>
>> Item was changed:
>>  ----- Method: Environment>>removeKey:ifAbsent: (in category 'emulating')
>> -----
>>  removeKey: key ifAbsent: aBlock
>> +
>> +       (declarations includesKey: key) ifFalse: [ ^aBlock value ].
>> +       references removeKey: key ifAbsent: [].
>> +       public removeKey: key ifAbsent: [].
>> +       ^declarations removeKey: key!
>> -       self flag: #review.
>> -       ^ declarations removeKey: key ifAbsent: aBlock!
>>
>> Item was changed:
>> + (PackageInfo named: 'Environments') postscript: '|
>> globalsWithMultipleBindings |
>> + "Fix exports."
>> + Smalltalk globals exports instVarNamed: #namespace put: Smalltalk
>> globals public.
>> + "Fix imports."
>> + (Smalltalk globals instVarNamed: #imports) instVarNamed: #next put: nil.
>> + "Unify globals."
>> + globalsWithMultipleBindings := Dictionary new.
>> + Binding allSubInstances do: [ :binding |
>> +       (globalsWithMultipleBindings at: binding key ifAbsentPut: [
>> IdentitySet new ]) add: binding ].
>> + globalsWithMultipleBindings := globalsWithMultipleBindings select: [
>> :each | each size > 1 ].
>> + globalsWithMultipleBindings keysAndValuesDo: [ :name :set |
>> +       | realBinding |
>> +       realBinding := Smalltalk associationAt: name ifAbsent: [ nil ].
>> +       realBinding ifNotNil: [
>> +               set do: [ :each |
>> +                       each == realBinding ifFalse: [ each becomeForward:
>> realBinding ] ] ] ]'!
>> - (PackageInfo named: 'Environments') postscript: '"below, add code to be
>> run after the loading of this package"
>> -
>> - | allAliases toBeRecompiled undesirableAliases |
>> -
>> - "Collect the CompiledMethods pointing to an Alias"
>> - allAliases := Alias allInstances.
>> - toBeRecompiled := CompiledMethod allInstances select: [:c | c
>> isInstalled and: [allAliases anySatisfy: [:a | c pointsTo: a]]].
>> -
>> - "Collect the Aliases pointing to some class binding in the same
>> Environment with same name"
>> - undesirableAliases := (Smalltalk globals instVarNamed: ''references'')
>> associations select: [:e |
>> -       e class = Alias and: [e key = e source key
>> -               and: [(Smalltalk globals instVarNamed: ''declarations'')
>> associations includes: e source]]].
>> -
>> - "Replace the undesirable Aliases with their source binding"
>> - undesirableAliases do: [:a | a becomeForward: a source].
>> -
>> - "Rehash the references because they pointed to those Aliases - hope
>> there''s nothing else to rehash"
>> - (Smalltalk globals instVarNamed: ''references'') rehash.
>> -
>> - "Recompile the CompiledMethod that used to point to an Alias, because
>> the bytecodes do change"
>> - Symbol rehash.
>> - toBeRecompiled do: [:c | c methodClass recompile: c selector].
>> -
>> - allAliases := toBeRecompiled := undesirableAliases := nil.
>> - Smalltalk garbageCollect.
>> - Alias allInstances size.
>> - '!
>>
>>
>>
>