On 29 December 2013 23:33, Levente Uzonyi leves@elte.hu wrote:

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.

Right.

...

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.

Er, of course, because had I been thinking properly, I'd have realised that the receiver is an Environment, not an IdentityDictionary.

...

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