This certainly sounds like a good change to me, especially since it is supported by unit tests.
Dave
On Sun, Jan 22, 2017 at 01:19:18AM +0100, Jakob Reschke wrote:
Hi all,
I stumbled on something with Environment imports and would like to hear other opinions about the issue.
Currently, environments do not care all that much about other environments declaring the same name. If an environment T imports from environments A, B, C, the binding of #x in T will be replaced with the binding of #x that was added last in any of the environments A, B, C, or T.
Example:
env1 := Environment new. env2 := Environment new. env1 exportSelf. env2 importSelf. env2 import: env1. env2 bind: #Griffle to: 2. env2 bindingOf: #Griffle. "#Griffle=>2" env1 bind: #Griffle to: 1. env2 bindingOf: #Griffle. "#Griffle=>1"Note that the imported environment hides the local binding of env2.
If the environments already come with bindings at "import time", the import order matters:
env1 := Environment new. env2 := Environment new. target := Environment new. env1 exportSelf. env2 exportSelf. env1 bind: #Griffle to: 1. env2 bind: #Griffle to: 2. target import: env1; import: env2. target bindingOf: #Griffle. "#Griffle=>2" "...but when done the other way around (start over up to the imports)..." target import: env2; import: env1. target bindingOf: #Griffle. "#Griffle=>1"Now the funniest part: Let's assume target imported env1 last, so the binding comes from env1 and is #Griffle=>1. Guess what happens when env2 unbinds its #Griffle:
env2 unbind: #Griffle. target bindingOf: #Griffle. "nil"So an environment can unbind something in another environment that was actually imported from a third environment. I consider this a bug and have a fix proposal for it, which I plan to put into the inbox soon. This behavior can have a really nasty consequence, which you can read up in the PS at the bottom if you like.
Another peculiarity is that #importSelf does not overwrite existing bindings (if you imported something else before), but #import: and friends happily do overwrite bindings from self.
What do you think about that reckless overwriting of bindings in general? I would like at least the "own" bindings (declarations) of an environment to have precedence over imported bindings. So you do not surprisingly lose your class binding when another class with the same name enters the original Smalltalk environment or is removed from it:
env := Environment withName: #Demo. env importSelf; import: Smalltalk globals. classfactory := ClassFactoryForTestCase new. [class := classfactory newClass] on: CurrentEnvironment do: [:e |e resume: env]. (env valueOf: class name) environment. "Demo" "put the CurrentEnvironment handling around the following instead to test the removing case" Object subclass: class name instanceVariableNames: '' classVariableNames: '' poolDictionaries: '' category: 'CategoryForTestToBeDeleted-Default'. (env valueOf: class name) environment. "Smalltalk" (Smalltalk at: class name) removeFromSystem. env bindingOf: class name. "nil" classfactory cleanUp.
Kind regards, Jakob
PS. Here the nasty story about the "bug" from above: It "literally destroys Smalltalk" when you attempt to #destroy a named environment (created with Environment withName: or named:) that imports "Smalltalk globals" and imports self. Every named environment has an own #Smalltalk declaration with an own SmalltalkImage instance that answers the named environment itself from "Smalltalk globals". Destroy unbinds all declarations, including this #Smalltalk. Because #importSelf does not overwrite existing bindings, the effective #Smalltalk binding is the one from the original Smalltalk environment, #Smalltalk=>Smalltalk (instead of #Smalltalk=>TheOtherEnvironmentsSmalltalk). Because the environment imports itself, it propagates the unbind: #Smalltalk to itself and eventually, as per the above bug, modifies the #Smalltalk=>Smalltalk binding with a nice becomeForward:. So the whole image has just lost its binding for #Smalltalk, nothing happens anymore, great.
The code for the adventurous: env := Environment withName: #Demo. (env at: #Smalltalk) globals. "Demo" env importSelf. (env valueOf: #Smalltalk) globals. "Demo" env import: Smalltalk globals. (env valueOf: #Smalltalk) globals. "Smalltalk" env destroy.