Hi Jaromir,

> On Sep 19, 2022, at 7:13 AM, Jaromir Matas <mail at jaromir.net> wrote:
> 
> 
> Hi Eliot, Christoph,
>  
> I have a question re: [Vm-dev] [OpenSmalltalk/opensmalltalk-vm] pushRcvr fails for context instances whose sender has been set to an Integer instance (Issue #654):
>  
> Why would one want to be able to send `privSender: 1` ? Is it useful for something? Otherwise I'm thinking why not block such an assignment inside #privSender e.g. like this:
>  
> Context>>privSender: aContextOrNil
>  
>         sender := aContextOrNil ifNotNil: [aContextOrNil asContext]

Well, I find the convention weak, but the implication of the priv prefix in privSender: et al is that these are private selectors, and hence only used within Context (clearly violated in a few places) and hence only correctly.

However, your idea is a good one; I’ll add it.

 
> As a side-effect this would allow e.g. blocks to be used as reasonable arguments too...

Which might simplify some debugger code.

But should the selector be simply sender: or setSender: ?  And what about swapSender: ?  I like sending isContext in several places to validate the argument.  I also find the sends to access sender in e.g. insertSender: unnecessary and complicating.

I like the idea of cleaning up this code. There’s lots of old code and some duplication.  Let’s keep polishing.

>  Thanks,
> Jaromir

Eliot
_,,,^..^,,,_ (phone)

> From: commits at source.squeak.org
> Sent: Monday, September 19, 2022 1:29
> To: squeak-dev at lists.squeakfoundation.org
> Subject: [squeak-dev] The Inbox: Kernel-eem.1489.mcz
>  
> A new version of Kernel was added to project The Inbox:
> http://source.squeak.org/inbox/Kernel-eem.1489.mcz
> 
> ==================== Summary ====================
> 
> Name: Kernel-eem.1489
> Author: eem
> Time: 18 September 2022, 4:29:27.672168 pm
> UUID: 825872cd-7b3f-48cb-b765-0fa7bb4b4f21
> Ancestors: Kernel-eem.1488
> 
> Ensure that a Context can only be instantiated via newForMethod:, and that the resulting Context has its method inst var initiialized.
> 
> =============== Diff against Kernel-eem.1488 ===============
> 
> Item was changed:
>   ----- Method: BlockClosure>>asContextWithSender: (in category 'private') -----
>   asContextWithSender: aContext
>          "Inner private support method for evaluation.  Do not use unless you know what you're doing."
>   
>          ^(Context newForMethod: outerContext method)
>                  setSender: aContext
>                  receiver: outerContext receiver
> -                method: outerContext method
>                  closure: self
>                  startpc: startpcOrMethod;
>                  privRefresh!
> 
> Item was changed:
>   ----- Method: BlockClosure>>simulateValueWithArguments:caller: (in category 'system simulation') -----
>   simulateValueWithArguments: anArray caller: aContext
>          "Simulate the valueWithArguments: primitive. Fail if anArray is not an array of the right arity."
>          | newContext sz |
>          newContext := (Context newForMethod: outerContext method)
>                                                  setSender: aContext
>                                                  receiver: outerContext receiver
> -                                                method: outerContext method
>                                                  closure: self
>                                                  startpc: startpcOrMethod.
>          ((newContext objectClass: anArray) ~~ Array
>           or: [numArgs ~= anArray size]) ifTrue:
>                  [^Context primitiveFailTokenFor: nil].
>          sz := self basicSize.
>          newContext stackp: sz + numArgs.
>          1 to: numArgs do:
>                  [:i| newContext at: i put: (anArray at: i)].
>          1 to: sz do:
>                  [:i| newContext at: i + numArgs put: (self at: i)].
>          ^newContext!
> 
> Item was changed:
>   ----- Method: Context class>>basicNew: (in category 'instance creation') -----
>   basicNew: size 
> +        self instanceCreationError!
> -        ^ (size = CompiledMethod smallFrameSize or: [ size = CompiledMethod fullFrameSize ])
> -                ifTrue: [ super basicNew: size ]
> -                ifFalse: [ self error: 'Contexts must be ' , CompiledMethod smallFrameSize , ' or ' , CompiledMethod fullFrameSize , ' bytes.' ]!
> 
> Item was added:
> + ----- Method: Context class>>instanceCreationError (in category 'private') -----
> + instanceCreationError
> +        self error: 'Contexts must only be created with newForMethod:'!
> 
> Item was changed:
>   ----- Method: Context class>>new (in category 'instance creation') -----
>   new
> +        self instanceCreationError!
> - 
> -        self error: 'Contexts must only be created with newForMethod:'!
> 
> Item was changed:
>   ----- Method: Context class>>new: (in category 'instance creation') -----
>   new: size
> +        self instanceCreationError!
> - 
> -        self error: 'Contexts must only be created with newForMethod:'!
> 
> Item was changed:
>   ----- Method: Context class>>newForMethod: (in category 'instance creation') -----
> + newForMethod: aCompiledCode
> - newForMethod: aMethod
>          "This is the only method for creating new contexts, other than primitive cloning.
>          Any other attempts, such as inherited methods like shallowCopy, should be
>          avoided or must at least be rewritten to determine the proper size from the
>          method being activated.  This is because asking a context its size (even basicSize!!)
>          will not return the real object size but only the number of fields currently
>          accessible, as determined by stackp."
>   
> +        ^(super basicNew: aCompiledCode frameSize)
> +                privMethod: aCompiledCode!
> -        ^ super basicNew: aMethod frameSize!
> 
> Item was changed:
>   ----- Method: Context class>>sender:receiver:method:arguments: (in category 'instance creation') -----
>   sender: s receiver: r method: m arguments: args 
>          "Answer an instance of me with attributes set to the arguments."
>   
> +        ^(self newForMethod: m) setSender: s receiver: r arguments: args!
> -        ^(self newForMethod: m) setSender: s receiver: r method: m arguments: args!
> 
> Item was added:
> + ----- Method: Context>>privMethod: (in category 'private') -----
> + privMethod: aCompiledCode
> + 
> +        method := aCompiledCode!
> 
> Item was added:
> + ----- Method: Context>>setSender:receiver:arguments: (in category 'private') -----
> + setSender: s receiver: r arguments: args 
> +        "Initialize the receiver's initial state."
> + 
> +        pc := method initialPC.
> +        self stackp: method numTemps.
> +        sender := s.
> +        receiver := r.
> +        closureOrNil := nil.
> +        1 to: args size do: [:i | self at: i put: (args at: i)]!
> 
> Item was added:
> + ----- Method: Context>>setSender:receiver:closure:startpc: (in category 'private') -----
> + setSender: s receiver: r closure: c startpc: startpc
> +        "Create the receiver's initial state."
> + 
> +        sender := s.
> +        receiver := r.
> +        closureOrNil := c.
> +        pc := startpc.
> +        stackp := 0!
> 
> Item was removed:
> - ----- Method: Context>>setSender:receiver:method:arguments: (in category 'private') -----
> - setSender: s receiver: r method: m arguments: args 
> -        "Create the receiver's initial state."
> - 
> -        sender := s.
> -        receiver := r.
> -        method := m.
> -        closureOrNil := nil.
> -        pc := method initialPC.
> -        self stackp: method numTemps.
> -        1 to: args size do: [:i | self at: i put: (args at: i)]!
> 
> Item was removed:
> - ----- Method: Context>>setSender:receiver:method:closure:startpc: (in category 'private') -----
> - setSender: s receiver: r method: m closure: c startpc: startpc
> -        "Create the receiver's initial state."
> - 
> -        sender := s.
> -        receiver := r.
> -        method := m.
> -        closureOrNil := c.
> -        pc := startpc.
> -        stackp := 0!
> 
> Item was changed:
>   ----- Method: FullBlockClosure>>asContextWithSender: (in category 'private') -----
>   asContextWithSender: aContext
>          "Inner private support method for evaluation.  Do not use unless you know what you're doing."
>   
>          ^(Context newForMethod: startpcOrMethod)
>                  setSender: aContext
>                  receiver: self receiver
> -                method: startpcOrMethod
>                  closure: self
>                  startpc: startpcOrMethod initialPC;
>                  privRefresh!
> 
> Item was changed:
>   ----- Method: FullBlockClosure>>simulateValueWithArguments:caller: (in category 'simulation') -----
>   simulateValueWithArguments: anArray caller: aContext
>          "Simulate the valueWithArguments: primitive. Fail if anArray is not an array of the right arity."
>          | newContext |
>          newContext := (Context newForMethod: startpcOrMethod)
>                                                  setSender: aContext
>                                                  receiver: receiver
> -                                                method: startpcOrMethod
>                                                  closure: self
>                                                  startpc: startpcOrMethod initialPC.
>          ((newContext objectClass: anArray) ~~ Array
>           or: [numArgs ~= anArray size]) ifTrue:
>                  [^Context primitiveFailTokenFor: nil].
>          newContext stackp: startpcOrMethod numTemps.
>          1 to: numArgs do:
>                  [:i| newContext at: i put: (anArray at: i)].
>          1 to: self basicSize do:
>                  [:i| newContext at: i + numArgs put: (self at: i)].
>          ^newContext!
> 
> 
>  
> 
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20220919/880fa5e5/attachment-0001.html>