The Inbox: Kernel-cmm.831.mcz
Locked
 
|
A new version of Kernel was added to project The Inbox:
http://source.squeak.org/inbox/Kernel-cmm.831.mcz ==================== Summary ==================== Name: Kernel-cmm.831 Author: cmm Time: 8 January 2014, 10:24:39.553 am UUID: 7cc78def-6be9-4aec-b2f1-08d8f7e66910 Ancestors: Kernel-fbs.829 - Factor the actual removal of individual classes into primRemoveFromSystem: to make Frank happy. - SystemChangeNotifier, no more notifications for Obsolete classes please. - Because we're modifying a collection while enumerating it, enumerate a copy of it instead. =============== Diff against Kernel-fbs.829 =============== Item was added: + ----- Method: Class>>primRemoveFromSystem: (in category 'initialize-release') ----- + primRemoveFromSystem: logged + "Forget the receiver from the Smalltalk global dictionary. Any existing + instances will refer to an obsolete version of the receiver." + + "keep the class name and category for triggering the system change message. If we wait to long, then we get obsolete information which is not what we want." + + "tell class to deactivate and unload itself-- two separate events in the module system" + self deactivate; unload. + self superclass ifNotNil: + ["If we have no superclass there's nothing to be remembered" + self superclass addObsoleteSubclass: self]. + self environment forgetClass: self logged: logged. + SystemChangeNotifier uniqueInstance noMoreNotificationsFor: self. + self obsolete.! Item was changed: ----- Method: Class>>removeFromSystem: (in category 'initialize-release') ----- + removeFromSystem: logged - removeFromSystem: logged "Forget the receiver from the Smalltalk global dictionary. Any existing instances will refer to an obsolete version of the receiver." - "keep the class name and category for triggering the system change message. If we wait to long, then we get obsolete information which is not what we want." - "tell class to deactivate and unload itself-- two separate events in the module system" + subclasses ifNotNil: + [ subclasses copy do: + [ : each | each removeFromSystem: logged ] ]. + self primRemoveFromSystem: logged! - self deactivate; unload. - self superclass ifNotNil: - ["If we have no superclass there's nothing to be remembered" - self superclass addObsoleteSubclass: self]. - self environment forgetClass: self logged: logged. - self obsolete.! |
Re: The Inbox: Kernel-cmm.831.mcz
|
|
On 8 January 2014 16:24, <[hidden email]> wrote:
> A new version of Kernel was added to project The Inbox: > http://source.squeak.org/inbox/Kernel-cmm.831.mcz > > ==================== Summary ==================== > > Name: Kernel-cmm.831 > Author: cmm > Time: 8 January 2014, 10:24:39.553 am > UUID: 7cc78def-6be9-4aec-b2f1-08d8f7e66910 > Ancestors: Kernel-fbs.829 > > - Factor the actual removal of individual classes into primRemoveFromSystem: to make Frank happy. > - SystemChangeNotifier, no more notifications for Obsolete classes please. > - Because we're modifying a collection while enumerating it, enumerate a copy of it instead. > > =============== Diff against Kernel-fbs.829 =============== That's pretty much the opposite of what I was saying. #removeFromSystem: is something wired fairly deeply into the guts of the system. But OK, try it out with unloading MC packages. Let's see if it blows up. (I anticipate seeing a bunch of unexpected behaviour resulting from MCClassDefinitions having their classes ripped out from under them, and possibly weird behaviour from classes being #unload-ed multiple times during the unload process. Specifically, given Object subclass: #Foo. Foo subclass: #Bar. Bar subclass: #Baz. I expect to see Foo unloaded once, Bar unloaded twice, and Bax unloaded three times. That does not seem right to me. But I repeat: I think it's a great idea to have cmd-X say "yep, that class is gone" and "oh hey, do you want to remove the class and all these here subclasses?" (a bit like like removing a method does) in the tools. frank |
Re: The Inbox: Kernel-cmm.831.mcz
|
|
> That's pretty much the opposite of what I was saying.
> #removeFromSystem: is something wired fairly deeply into the guts of > the system. Not very deep, and there aren't that many senders. They could be changed to use the new #primRemoveFromSystem:; but how much would you like to bet they don't expect there to be subclasses. IOW, eToys or Kedama generating classes dynamically, and then later removing those classes as part of an app function; they are not creating subclasses of *those*. I don't think any sender of removeFromSystem: wants to keep subclasses. If they do, I'll be interested to understand why. It's wrong to have a method which leaves the domain (in this case, the class model) in an invalid state be named what the first thought someone would have, looking for such API. #removeFromSystem: is the correct name for the behavior users expect. #prim is the appropriate prefix for the behavior users don't expect; that potentially leaves the domain in a partial or invalid state. The #prim method should be private. > But OK, try it out with unloading MC packages. Let's see > if it blows up. (I anticipate seeing a bunch of unexpected behaviour > resulting from MCClassDefinitions having their classes ripped out from > under them, and possibly weird behaviour from classes being #unload-ed > multiple times during the unload process. > > Specifically, given > > Object subclass: #Foo. > Foo subclass: #Bar. > Bar subclass: #Baz. Sure, please try it out and if anything blows up or is otherwise wrong in some way, we'll either fix or abandon this idea. > I expect to see Foo unloaded once, Bar unloaded twice, and Bax > unloaded three times. That does not seem right to me. I agree it would be wrong if it were true, but I'm not seeing how it is. What do you mean "unloaded three times"? Are you saying you think #unload will be called three times on Baz? If that's true, you'll get no argument from me about that needing fixed. > But I repeat: I think it's a great idea to have cmd-X say "yep, that > class is gone" and "oh hey, do you want to remove the class and all > these here subclasses?" (a bit like like removing a method does) in > the tools. There is already a short and sweet message warning about there being subclasses. Your proposal to list every subclass would turn that into a large and elaborate confirmation pop-up. It would be presenting information that was directly under it in the original HierarchyBrowser, and better viewed there anyway. So, the user who didn't know what they're doing will most likely just press "No" and see it there. Pop-ups are evil. Until we can eliminate them entirely, I prefer to keep them as low-profile as possible. |
Re: The Inbox: Kernel-cmm.831.mcz
|
|
On 8 January 2014 20:16, Chris Muller <[hidden email]> wrote:
>> That's pretty much the opposite of what I was saying. >> #removeFromSystem: is something wired fairly deeply into the guts of >> the system. > > Not very deep, and there aren't that many senders. They could be > changed to use the new #primRemoveFromSystem:; but how much would you > like to bet they don't expect there to be subclasses. IOW, eToys or > Kedama generating classes dynamically, and then later removing those > classes as part of an app function; they are not creating subclasses > of *those*. I don't think any sender of removeFromSystem: wants to > keep subclasses. If they do, I'll be interested to understand why. > > It's wrong to have a method which leaves the domain (in this case, the > class model) in an invalid state be named what the first thought > someone would have, looking for such API. #removeFromSystem: is the > correct name for the behavior users expect. #prim is the appropriate > prefix for the behavior users don't expect; that potentially leaves > the domain in a partial or invalid state. The #prim method should be > private. I can see your argument. I'm not 100% convinced yet, but perhaps my hindbrain will be persuaded by the time we get to the start of 4.6. >> But OK, try it out with unloading MC packages. Let's see >> if it blows up. (I anticipate seeing a bunch of unexpected behaviour >> resulting from MCClassDefinitions having their classes ripped out from >> under them, and possibly weird behaviour from classes being #unload-ed >> multiple times during the unload process. >> >> Specifically, given >> >> Object subclass: #Foo. >> Foo subclass: #Bar. >> Bar subclass: #Baz. > > Sure, please try it out and if anything blows up or is otherwise wrong > in some way, we'll either fix or abandon this idea. > >> I expect to see Foo unloaded once, Bar unloaded twice, and Bax >> unloaded three times. That does not seem right to me. > > I agree it would be wrong if it were true, but I'm not seeing how it > is. What do you mean "unloaded three times"? Are you saying you > think #unload will be called three times on Baz? If that's true, > you'll get no argument from me about that needing fixed. I responded too quickly to the mail. You're right, the class won't receive multiple #unload sends because of the guard. I still don't like the multiple #removeFromSystem: calls subclasses would get during package removal. Seems messy. >> But I repeat: I think it's a great idea to have cmd-X say "yep, that >> class is gone" and "oh hey, do you want to remove the class and all >> these here subclasses?" (a bit like like removing a method does) in >> the tools. > > There is already a short and sweet message warning about there being > subclasses. Your proposal to list every subclass would turn that into > a large and elaborate confirmation pop-up. It would be presenting > information that was directly under it in the original > HierarchyBrowser, and better viewed there anyway. So, the user who > didn't know what they're doing will most likely just press "No" and > see it there. My main concern is just someone doing something apparently innocuous, and not realising they're about to obsolete half the classes in their image. I don't seriously propose that anyone would cmd-X ProtoObject, but think of that as the worst-case-taken-to-ridiculous-extremes scenario, > Pop-ups are evil. Until we can eliminate them entirely, I prefer to > keep them as low-profile as possible. I _like_ the remove-selector popup, but then (a) "don't mode me in" and (b) you're more experienced in the UX thing, so I'll accept this :) frank |
Re: The Inbox: Kernel-cmm.831.mcz
|
|
In reply to this post by commits-2
Hi Chris,
On Wed, Jan 8, 2014 at 8:24 AM, <[hidden email]> wrote: A new version of Kernel was added to project The Inbox: The convention is to use prim or primitive for VM things, and basic for image things. So basicRemoveSelector: and removeSelector: etc. So this is better-named basicRemoveFromSystem: [ irrespective of whether you decide to keep this code or not ;-) ].
HTH, Eliot + "Forget the receiver from the Smalltalk global dictionary. Any existing best, Eliot
|
«
Return to Squeak - Dev
|
1 view|%1 views
| Free forum by Nabble | Edit this page |