[Vm-dev] Request for review of VMMaker-dtl.262.mcz

[David T. Lewis]

I would appreciate if one or two knowledgeable VM folks can take a look
at the change below and confirm that it is not harmful. This is the patch
to support the mirror primitive usage for primitivePerformInSuperclass
<http://bugs.squeak.org/view.php?id=7429>

This is not necessarily an elegant approach, and it is not done the same
way as in Cog, but it is the simplest thing I could think to do without
modifying the existing perform code in the standard VM (I am not experienced
in that area and don't what to change things that I do not understand).

I'm not worried about whether this is the best possible implementation,
I just want to make sure that it does not produce instability or unintended
side effects.

Thanks,
Dave


On Thu, Jan 05, 2012 at 05:41:53AM +0000, commits at source.squeak.org wrote:
>  
> David T. Lewis uploaded a new version of VMMaker to project VM Maker:
> http://source.squeak.org/VMMaker/VMMaker-dtl.262.mcz
> 
> ==================== Summary ====================
> 
> Name: VMMaker-dtl.262
> Author: dtl
> Time: 5 January 2012, 12:41:29.745 am
> UUID: 2ffbb8a5-3390-4047-ad76-f460087b1194
> Ancestors: VMMaker-dtl.261
> 
> VMMaker 4.7.19
> 
> Reference Mantis 7429: Add Mirror Primitives to the VM
> 
> Update primitivePerformInSuperclass to support ContextPart>>object:perform:withArguments:inClass:
> 
> Implementation differs from that of oscog in that the original primitivePerformAt: is retained unmodified, and the necessary stack adjustments are done in primitivePerformInSuperclass for the special case of argumentCount 4 (mirror primitive call) rather than 3. The oscog approach may be prefered (not least for its clearer method naming), but making the change in primitivePerformInSuperclass is low risk and more easily implemented by a Sunday Squeaker.
> 
> All MirrorPrimitiveTests pass.
> 
> =============== Diff against VMMaker-dtl.261 ===============
> 
> Item was changed:
>   ----- Method: Interpreter>>primitivePerformInSuperclass (in category 'control primitives') -----
>   primitivePerformInSuperclass
>   	| lookupClass rcvr currentClass |
>   	lookupClass := self stackTop.
> + 	rcvr := self stackValue: 3.
> - 	rcvr := self stackValue: argumentCount.
>   	currentClass := self fetchClassOf: rcvr.
>   	[currentClass ~= lookupClass]
>   		whileTrue:
>   		[currentClass := self superclassOf: currentClass.
>   		currentClass = nilObj ifTrue: [^ self primitiveFail]].
>   
> + 	argumentCount = 3
> + 		ifTrue: ["normal primitive call with 3 arguments expected on the stack"
> + 			self popStack.
> + 			self primitivePerformAt: lookupClass.
> + 			self successful ifFalse:
> + 				[self push: lookupClass]]
> + 		ifFalse: ["mirror primitive call with extra argument specifying object to serve as receiver"
> + 			| s1 s2 s3 s4 s5 |
> + 			"save stack contents"
> + 			s1 := self popStack. "lookupClass"
> + 			s2 := self popStack. "args"
> + 			s3 := self popStack. "selector"
> + 			s4 := self popStack. "mirror receiver"
> + 			s5 := self popStack. "actual receiver"
> + 			"slide stack up one, omitting the actual receiver parameter"
> + 			self push: s4. "mirror receiver"
> + 			self push: s3. "selector"
> + 			self push: s2. "args"
> + 			"perform as if mirror receiver had been the actual receiver"
> + 			self primitivePerformAt: lookupClass.
> + 			self successful ifFalse:
> + 				["restore original stack"
> + 				self pop: 3. "args, selector, mirror receiver"
> + 				self push: s5. "actual receiver"
> + 				self push: s4. "mirror receiver"				
> + 				self push: s3. "selector"
> + 				self push: s2. "args"
> + 				self push: s1. "lookup class" ]]
> + !
> - 	self popStack.
> - 	self primitivePerformAt: lookupClass.
> - 	self successful ifFalse:
> - 		[self push: lookupClass]!
> 
> Item was changed:
>   ----- Method: VMMaker class>>versionString (in category 'version testing') -----
>   versionString
>   
>   	"VMMaker versionString"
>   
> + 	^'4.7.19'!
> - 	^'4.7.18'!