[squeak-dev] The Inbox: Kernel-eem.1489.mczJaromir Matas mail at jaromir.netMon Sep 19 20:47:18 UTC 2022
Hi Eliot, Christoph,
> But should the selector be simply sender: or setSender: ?
I'd like it simply `#sender:`. I find #setSender inconsistent (and thus slightly irritating and confusing). And I like the idea to stop considering sender accessor private :)
> And what about swapSender: ? I like sending isContext in several places to validate the argument.
>
> I like the idea of cleaning up this code. There’s lots of old code and some duplication. Let’s keep polishing.
I guess at least these four could use `self sender:` instead of `sender :=` to force the type check:
#swapSender:
#insertSender:
#setSender:receiver:arguments:
#setSender:receiver:closure:startpc:
(Enclosing a simple suggestion; prevents Christoph's crash example)
Then there's #terminateTo: but "unfortunately" it's a primitive so it can't be fixed the same way... The following will crash the VM:
```
thisContext terminateTo: 1
```
> I also find the sends to access sender in e.g. insertSender: unnecessary and complicating.
+1 indeed
When at cleaning up: would it make sense to add this to complement the same you did for basicNew:, new: and new?
Context class>>basicNew
self instanceCreationError
Thanks,
best,
Jaromir
From: Eliot Miranda<mailto:eliot.miranda at gmail.com>
Sent: Monday, September 19, 2022 17:44
To: The general-purpose Squeak developers list<mailto:squeak-dev at lists.squeakfoundation.org>
Subject: Re: [squeak-dev] The Inbox: Kernel-eem.1489.mcz
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<mailto:commits at source.squeak.org>
Sent: Monday, September 19, 2022 1:29
To: squeak-dev at lists.squeakfoundation.org<mailto: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/bd559f49/attachment.html>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: Context-sender_accessor.jar.1.cs
URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20220919/bd559f49/attachment.ksh>
More information about the Squeak-dev mailing list |