The Inbox: Kernel-fbs.732.mcz

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

The Inbox: Kernel-fbs.732.mcz

commits-2
Frank Shearar uploaded a new version of Kernel to project The Inbox:
http://source.squeak.org/inbox/Kernel-fbs.732.mcz

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

Name: Kernel-fbs.732
Author: fbs
Time: 27 January 2013, 11:23:27.822 am
UUID: b9ad1255-fa5b-4480-ae3f-923fd0969ca8
Ancestors: Kernel-nice.731

#subclassResponsibility in-Debugger method creation #1 of 4: Raise a SubclassResponsibility when something sends #subclassResponsibility, recording the offending class, the selector, and the arguments to the selector. (The Debugger uses this information.)

=============== Diff against Kernel-nice.731 ===============

Item was changed:
  ----- Method: Object>>subclassResponsibility (in category 'error handling') -----
  subclassResponsibility
  "This message sets up a framework for the behavior of the class' subclasses.
  Announce that the subclass should have implemented this message."
+ | exception args senderCtxt |
+ "We must assign to a local variable so that the Debugger can access the exception through thisParticularContext tempAt: 1. This also means that the exception local must be the first local declared."
+ senderCtxt := thisContext sender.
+ "Copy the temps out of senderCtxt"
+ args := OrderedCollection new.
+ 1 to: senderCtxt selector numArgs do:
+ [:idx | args addLast: (senderCtxt tempAt: idx)].
+ exception := SubclassResponsibilityError
+ class: self class
+ selector: senderCtxt selector
+ arguments: args.
+ exception signal.!
-
- self error: 'My subclass should have overridden ', thisContext sender selector printString!


Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: Kernel-fbs.732.mcz

Frank Shearar-3
On 27 January 2013 11:23,  <[hidden email]> wrote:

> Frank Shearar uploaded a new version of Kernel to project The Inbox:
> http://source.squeak.org/inbox/Kernel-fbs.732.mcz
>
> ==================== Summary ====================
>
> Name: Kernel-fbs.732
> Author: fbs
> Time: 27 January 2013, 11:23:27.822 am
> UUID: b9ad1255-fa5b-4480-ae3f-923fd0969ca8
> Ancestors: Kernel-nice.731
>
> #subclassResponsibility in-Debugger method creation #1 of 4: Raise a SubclassResponsibility when something sends #subclassResponsibility, recording the offending class, the selector, and the arguments to the selector. (The Debugger uses this information.)

Sigh. Actually, #2 of 4.

Is there an easier way of submitting cross-package changes? I guess
Pharo's slices?

frank

Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: Kernel-fbs.732.mcz

Eliot Miranda-2
In reply to this post by commits-2
Hi Frank,

    IMO the exception is overkill.  The context can be found from the sender of the signalling context for error.  Once the context is found its selector and arguments are available.  Even if you keep the exception you shouldn't need to load it with all that state (ugly as hell).  It can all be discovered by introspection in the debugger.  And a better place for the introspection would be something like ContextPart>invocationMessage that would answer a MessageSend for a context.  Note that createMethod is driven by a Message.



On Sun, Jan 27, 2013 at 3:23 AM, <[hidden email]> wrote:
Frank Shearar uploaded a new version of Kernel to project The Inbox:
http://source.squeak.org/inbox/Kernel-fbs.732.mcz

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

Name: Kernel-fbs.732
Author: fbs
Time: 27 January 2013, 11:23:27.822 am
UUID: b9ad1255-fa5b-4480-ae3f-923fd0969ca8
Ancestors: Kernel-nice.731

#subclassResponsibility in-Debugger method creation #1 of 4: Raise a SubclassResponsibility when something sends #subclassResponsibility, recording the offending class, the selector, and the arguments to the selector. (The Debugger uses this information.)

=============== Diff against Kernel-nice.731 ===============

Item was changed:
  ----- Method: Object>>subclassResponsibility (in category 'error handling') -----
  subclassResponsibility
        "This message sets up a framework for the behavior of the class' subclasses.
        Announce that the subclass should have implemented this message."
+       | exception args senderCtxt |
+       "We must assign to a local variable so that the Debugger can access the exception through thisParticularContext tempAt: 1. This also means that the exception local must be the first local declared."
+       senderCtxt := thisContext sender.
+        "Copy the temps out of senderCtxt"
+       args := OrderedCollection new.
+       1 to: senderCtxt selector numArgs do:
+               [:idx | args addLast: (senderCtxt tempAt: idx)].
+       exception := SubclassResponsibilityError
+               class: self class
+               selector: senderCtxt selector
+               arguments: args.
+       exception signal.!
-
-       self error: 'My subclass should have overridden ', thisContext sender selector printString!





--
best,
Eliot


Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: Kernel-fbs.732.mcz

Frank Shearar-3
Hi Eliot,

Thanks for the review. So in particular you dislike the needless
copying of state into the exception, rather than the exception itself?
I'd like to keep the distinguishing nature of
SubclassResponsibilityError, but I'm happy to lose the state storage.

In writing ContextPart >> #invocationMessage I'd still have to copy
state, unless you can suggest something cleaner?

invocationMessage
  | selector args |
  selector := self method.
  args := selector numArgs > 0
    ifTrue: ["copy stuff from using #tempAt:"]
    ifFalse: [#()].
  ^ MessageSend receiver self receiver selector: selector arguments: args.

We _could_ special-case MNU, and say

invocationMessage
  | selector args |
  (self selector == #doesNotUnderstand:) ifTrue: [^ self tempAt: 1].
  "Do what the first implementation does."

which would mimic what Debugger >> buildNotifierWith:label:message: does.

Then #createMethod would say

createMethod
        | msg chosenClass |
        self halt.
        msg := self contextStackTop invocationMessage.
        chosenClass := self
                askForSuperclassOf: msg receiver class
                toImplement: msg selector
                ifCancel: [^self].
        self implement: msg message inClass: chosenClass.

frank

On 28 January 2013 23:58, Eliot Miranda <[hidden email]> wrote:

> Hi Frank,
>
>     IMO the exception is overkill.  The context can be found from the sender
> of the signalling context for error.  Once the context is found its selector
> and arguments are available.  Even if you keep the exception you shouldn't
> need to load it with all that state (ugly as hell).  It can all be
> discovered by introspection in the debugger.  And a better place for the
> introspection would be something like ContextPart>invocationMessage that
> would answer a MessageSend for a context.  Note that createMethod is driven
> by a Message.
>
> 2¢
>
> On Sun, Jan 27, 2013 at 3:23 AM, <[hidden email]> wrote:
>>
>> Frank Shearar uploaded a new version of Kernel to project The Inbox:
>> http://source.squeak.org/inbox/Kernel-fbs.732.mcz
>>
>> ==================== Summary ====================
>>
>> Name: Kernel-fbs.732
>> Author: fbs
>> Time: 27 January 2013, 11:23:27.822 am
>> UUID: b9ad1255-fa5b-4480-ae3f-923fd0969ca8
>> Ancestors: Kernel-nice.731
>>
>> #subclassResponsibility in-Debugger method creation #1 of 4: Raise a
>> SubclassResponsibility when something sends #subclassResponsibility,
>> recording the offending class, the selector, and the arguments to the
>> selector. (The Debugger uses this information.)
>>
>> =============== Diff against Kernel-nice.731 ===============
>>
>> Item was changed:
>>   ----- Method: Object>>subclassResponsibility (in category 'error
>> handling') -----
>>   subclassResponsibility
>>         "This message sets up a framework for the behavior of the class'
>> subclasses.
>>         Announce that the subclass should have implemented this message."
>> +       | exception args senderCtxt |
>> +       "We must assign to a local variable so that the Debugger can
>> access the exception through thisParticularContext tempAt: 1. This also
>> means that the exception local must be the first local declared."
>> +       senderCtxt := thisContext sender.
>> +        "Copy the temps out of senderCtxt"
>> +       args := OrderedCollection new.
>> +       1 to: senderCtxt selector numArgs do:
>> +               [:idx | args addLast: (senderCtxt tempAt: idx)].
>> +       exception := SubclassResponsibilityError
>> +               class: self class
>> +               selector: senderCtxt selector
>> +               arguments: args.
>> +       exception signal.!
>> -
>> -       self error: 'My subclass should have overridden ', thisContext
>> sender selector printString!
>>
>>
>
>
>
> --
> best,
> Eliot