[squeak-dev] The Inbox: Environments-ul.43.mcz

Levente Uzonyi leves at elte.hu
Sun Dec 29 23:33:49 UTC 2013


On Sun, 29 Dec 2013, Frank Shearar wrote:

> "Radical" would be the #becomeForwards: calls?

No. Radical in the sense that those three methods are not just 
forwarders anymore. A bug in any of them can have bad consequences.

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

No, code which uses #removeKey:ifAbsent: also updates Undeclared.

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

The postscript also fixes the exports and cleans the imports.

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

We definitely need more tests for Environments.


Levente

>
> 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.
>>> - '!
>>>
>>>
>>>
>>
>
>


More information about the Squeak-dev mailing list