Re: [Pharo-project] [squeak-dev] Contexts must only be created with newForMethod: ...
Hi Chris. Yes, I have this problem when materializing MethodContext instances in Fuel. I've solved it adding the method: ContextPart class >> newFromFrameSize: aFrameSize ^ super basicNew: aFrameSize but it is a hack. So yes, if there is no real reason to do the opposite, I would change #basicNew: Cheers On Mon, Aug 1, 2011 at 8:27 PM, Chris Muller <ma.chris.m@gmail.com> wrote:
.. but #newForMethod: requires more in the API than it really uses. It only checks the methods #frameSize to determine what basicSized MethodContext to create - it doesn't actually set its method from the one passed in.
newForMethod: aMethod --- (comment snipped for brevity) --- ^ super basicNew: aMethod frameSize
So, would it be ok to relax MethodContext class>>basicNew: to something like:
basicNew: size ^ (size = CompiledMethod smallFrameSize or: [ size = CompiledMethod fullframeSize ]) ifTrue: [ super basicNew: size ] ifFalse: [ self error: 'Contexts must be ', CompiledMethod smallFrameSize, ' or ', CompiledMethod fullframeSize, ' bytes.' ]
-- Mariano http://marianopeck.wordpress.com
Since I didn't get feedback there I ask here....what do you think? I agree with Chris. Thanks On Mon, Aug 1, 2011 at 8:34 PM, Mariano Martinez Peck <marianopeck@gmail.com
wrote:
Hi Chris. Yes, I have this problem when materializing MethodContext instances in Fuel. I've solved it adding the method:
ContextPart class >> newFromFrameSize: aFrameSize
^ super basicNew: aFrameSize
but it is a hack.
So yes, if there is no real reason to do the opposite, I would change #basicNew:
Cheers
On Mon, Aug 1, 2011 at 8:27 PM, Chris Muller <ma.chris.m@gmail.com> wrote:
.. but #newForMethod: requires more in the API than it really uses. It only checks the methods #frameSize to determine what basicSized MethodContext to create - it doesn't actually set its method from the one passed in.
newForMethod: aMethod --- (comment snipped for brevity) --- ^ super basicNew: aMethod frameSize
So, would it be ok to relax MethodContext class>>basicNew: to something like:
basicNew: size ^ (size = CompiledMethod smallFrameSize or: [ size = CompiledMethod fullframeSize ]) ifTrue: [ super basicNew: size ] ifFalse: [ self error: 'Contexts must be ', CompiledMethod smallFrameSize, ' or ', CompiledMethod fullframeSize, ' bytes.' ]
-- Mariano http://marianopeck.wordpress.com
-- Mariano http://marianopeck.wordpress.com
On Thu, Sep 1, 2011 at 10:03 AM, Mariano Martinez Peck < marianopeck@gmail.com> wrote:
Since I didn't get feedback there I ask here....what do you think? I agree with Chris.
you might write it as ContextPart class >> newFromFrameSize: aFrameSize self assert: (aFrameSize = SmallFrame or: [aFrameSize = LargeFrame]). ^ super basicNew: aFrameSize or ContextPart class >> newFromFrameSize: aFrameSize ^ super basicNew: (aFrameSize > SmallFrame ifTrue: [LargeFrame] ifFalse: [SmallFrame]) either after moving SmallFrame and LargeFrame to a shared pool, or by adding an accessor for LargeFrame in CompiledMethod class (there's already an accessor for SmallFrame, CompiledMethod class>>smallFrameSize). (i.e. : ContextPart class >> newFromFrameSize: aFrameSize self assert: (aFrameSize = CompiledMethod largeFrameSize or: [aFrameSize = CompiledMethod smallFrameSize]). ^ super basicNew: aFrameSize or ContextPart class >> newFromFrameSize: aFrameSize ^ super basicNew: (aFrameSize > CompiledMethod smallFrameSize ifTrue: [CompiledMethod largeFrameSize] ifFalse: [CompiledMethod smallFrameSize ])
Thanks
On Mon, Aug 1, 2011 at 8:34 PM, Mariano Martinez Peck < marianopeck@gmail.com> wrote:
Hi Chris. Yes, I have this problem when materializing MethodContext instances in Fuel. I've solved it adding the method:
ContextPart class >> newFromFrameSize: aFrameSize
^ super basicNew: aFrameSize
but it is a hack.
So yes, if there is no real reason to do the opposite, I would change #basicNew:
Cheers
On Mon, Aug 1, 2011 at 8:27 PM, Chris Muller <ma.chris.m@gmail.com>wrote:
.. but #newForMethod: requires more in the API than it really uses. It only checks the methods #frameSize to determine what basicSized MethodContext to create - it doesn't actually set its method from the one passed in.
newForMethod: aMethod --- (comment snipped for brevity) --- ^ super basicNew: aMethod frameSize
So, would it be ok to relax MethodContext class>>basicNew: to something like:
basicNew: size ^ (size = CompiledMethod smallFrameSize or: [ size = CompiledMethod fullframeSize ]) ifTrue: [ super basicNew: size ] ifFalse: [ self error: 'Contexts must be ', CompiledMethod smallFrameSize, ' or ', CompiledMethod fullframeSize, ' bytes.' ]
-- Mariano http://marianopeck.wordpress.com
-- Mariano http://marianopeck.wordpress.com
-- best, Eliot
participants (2)
-
Eliot Miranda -
Mariano Martinez Peck