The Trunk: Environments-cwp.49.mcz

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

The Trunk: Environments-cwp.49.mcz

commits-2
Colin Putney uploaded a new version of Environments to project The Trunk:
http://source.squeak.org/trunk/Environments-cwp.49.mcz

==================== Summary ====================

Name: Environments-cwp.49
Author: cwp
Time: 29 March 2014, 9:51:18.227 pm
UUID: 5f13ecd1-6908-4d47-ae16-deb2308cd0dc
Ancestors: Environments-cwp.48

Make sure globals are properly destroyed along with their environment.

=============== Diff against Environments-cwp.48 ===============

Item was changed:
+ ----- Method: BindingPolicy>>environment (in category 'accessing') -----
- ----- Method: BindingPolicy>>environment (in category 'as yet unclassified') -----
  environment
  ^ environment!

Item was changed:
  ----- Method: Environment>>destroy (in category 'initialize-release') -----
  destroy
 
  self allClasses do: [:ea | ea removeFromSystem].
+ declarations keys do: [:ea | self unbind: ea].
  policies do: [:ea | ea removeObserver: self].
  observers do: [:ea | ea stopObserving: self].!

Item was changed:
  ----- Method: Environment>>exportSelf (in category 'configuring') -----
  exportSelf
  | policy |
  policy := BindingPolicy
  environment: self
  policy: (AllNamePolicy new)
  addSelector: #notifyObserversOfBindingAdded:
+ removeSelector: #notifyObserversOfBindingRemoved:.
- removeSelector: #notifyObserversOfBindingAdded:.
  policies := policies copyWith: policy!

Item was changed:
  ----- Method: Environment>>forgetClass:logged: (in category 'classes and traits') -----
  forgetClass: aClass logged: aBool
+ (self hasBindingOf: aClass name) ifFalse: [ ^ self ].
- | binding |
- self flag: #review.
- "The class might not bound to its name"
-
  aBool ifTrue:
  [SystemChangeNotifier uniqueInstance
  classRemoved: aClass fromCategory: aClass category].
+ self organization removeElement: aClass name.
+ Smalltalk removeFromStartUpList: aClass.
+ Smalltalk removeFromShutDownList: aClass.
+ self unbind: aClass name!
-
- binding := declarations bindingOf: aClass name.
- binding ifNotNil:
- [self organization removeElement: aClass name.
- Smalltalk removeFromStartUpList: aClass.
- Smalltalk removeFromShutDownList: aClass.
-
- undeclared declare: aClass name from: declarations.
- declarations removeKey: aClass name ifAbsent: [].
- [undeclared at: aClass name put: nil]
- on: AttemptToWriteReadOnlyGlobal
- do: [:n | n resume: true].
- self binding: binding removedFrom: self]
- !

Item was added:
+ ----- Method: Environment>>hasBindingOf: (in category 'binding') -----
+ hasBindingOf: aSymbol
+ ^ declarations includesKey: aSymbol!

Item was changed:
  ----- Method: Environment>>hideBinding: (in category 'binding') -----
  hideBinding: aBinding
+ self undeclare: aBinding key from: bindings!
- bindings removeKey: aBinding key!

Item was added:
+ ----- Method: Environment>>unbind: (in category 'binding') -----
+ unbind: aSymbol
+ | binding |
+ binding := declarations bindingOf: aSymbol ifAbsent: [^ self].
+ undeclared declare: aSymbol from: declarations.
+ declarations removeKey: aSymbol ifAbsent: [  ].
+ [ undeclared at: aSymbol put: nil ]
+ on: AttemptToWriteReadOnlyGlobal
+ do: [ :n | n resume: true ].
+ self binding: binding removedFrom: self!


Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Environments-cwp.49.mcz

Balázs Kósi
Hi Colin,

This breaks renaming of classes. Try:

Object subclass: #Bar instanceVariableNames: '' classVariableNames: '' poolDictionaries: '' category: 'Foo'.
Bar rename: #Baz.

and then the binding for Baz will point to nil.

The problem stems from the change of Environment>>hideBinding: because

self undeclare: aBinding key from: bindings

makes the binding #Bar => Baz to become a binding pointing to nil, and when we get back to Environment>>renameClass:from:to:
it uses that same binding object and just changes the binding key to the new name, so at the end we have a binding #Baz => nil.

Balazs


Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Environments-cwp.49.mcz

Colin Putney-3



On Mon, Mar 31, 2014 at 2:21 AM, Balázs Kósi <[hidden email]> wrote:
 
self undeclare: aBinding key from: bindings

makes the binding #Bar => Baz to become a binding pointing to nil, and when we get back to Environment>>renameClass:from:to:
it uses that same binding object and just changes the binding key to the new name, so at the end we have a binding #Baz => nil

Good catch Balázs, thanks!

I've committed a test and fix to trunk. 

The fix is to create an entirely new binding for the new name, rather than reusing the old binding and just chaining the key. That's a change in behaviour, but I think it's correct. That way methods that refer to the old name will now have an undeclared binding with a nil value, and the new name will correctly refer to the renamed class. 

This avoids leaving methods in an inconsistent state where their source code doesn't match their compiled state. Methods that refer to the old name will have an undefined binding, while methods that refer to the new name will be bound to the correct class. That's how it should work at the low level—the tools should be responsible for fixing up the source code to use the new name and recompiling.

Colin