[Vm-dev] Bug in #instVarAt:put: for read-only objects

Norbert Hartl norbert at hartl.name
Sat Jan 6 14:09:50 UTC 2018


Thanks Clement!

> Am 06.01.2018 um 14:44 schrieb Clément Bera <bera.clement at gmail.com>:
> 
> 
> 
> On Sat, Jan 6, 2018 at 1:32 PM, Norbert Hartl <norbert at hartl.name <mailto:norbert at hartl.name>> wrote:
>  
> Btw. why is the name NoModificationError? Isn’t the purpose of an Error that it is not resumable? And is NoModificationError very likely to be resumed? Like in #retryModification?
> 
> Normally the NoModificationError should be able to retry the modification, and then resumes execution just after the primitive call / the inst var store. 
> 
I was actually asking why the mentioned NoModificationError is an error and not a NoModificationException. 

> In the case of primitive failure I guess either the error should resume with the value to return or the primitive fall-back code should return the correct value. In the case of inst var store, we need to resume execution using #jump (See #attemptToAssign: value withIndex: index)
>  
> For at:put: we could have:
> 
> 	<primitive: 174 error: ec>
> 	self isReadOnlyObject 
> 		ifTrue: [^ (ModificationForbidden for: self atInstVar: index with: anObject) signal]
> 		ifFalse: [ self primitiveFailed ]
> 
> OR 
> 
> 	<primitive: 174 error: ec>
> 	self isReadOnlyObject 
> 		ifTrue: [(ModificationForbidden for: self atInstVar: index with: anObject) signal. ^ anObject]
> 		ifFalse: [ self primitiveFailed ]
> 
> + the index bound check mentioned by Eliot and asInteger trick (based on at: primitive failure code)
> 
> So that when the exception resumes the correct behavior happens.
> 
I have now

at: index put: value 
	"Primitive. Assumes receiver is indexable. Store the argument value in 
	the indexable element of the receiver indicated by index. Fail if the 
	index is not an Integer or is out of bounds. Or fail if the value is not of 
	the right type for this kind of collection. Answer the value that was 
	stored. Essential. See Object documentation whatIsAPrimitive."

	<primitive: 61>
	index isInteger 
		ifTrue: [self class isVariable
			ifTrue: [(index between: 1 and: self size)
				ifFalse: [self errorSubscriptBounds: index]]
			ifFalse: [self errorNotIndexable]]
		ifFalse: [  
			index isNumber
				ifTrue: [^self at: index asInteger put: value]
				ifFalse: [self errorNonIntegerIndex] ].
	self isReadOnlyObject 
		ifTrue: [ ^ (InstVarModificationForbidden for: self atInstVar: index with: value) signal ].

Norbert

> 
> Norbert
> 
>> Am 05.01.2018 um 14:34 schrieb Eliot Miranda <eliot.miranda at gmail.com <mailto:eliot.miranda at gmail.com>>:
>> 
>> Hi Clément,
>> 
>>     is it too late to take a look at the VisualWorks code and use the same class names and selectors they use?  IIRC it is NoMidificationError.  It may make e.g. Gemstone's job easier if there is some consistency.
>> 
>> _,,,^..^,,,_ (phone)
>> 
>> On Jan 5, 2018, at 5:22 AM, Clément Bera <bera.clement at gmail.com <mailto:bera.clement at gmail.com>> wrote:
>> 
>>> Hi,
>>> 
>>> No this is not a bug.
>>> 
>>> This needs to be handled in the primitive failure code in the image, the method should be something like that :
>>> 
>>> Object>>instVarAt: index put: anObject
>>> 	<primitive: 174 error: ec>
>>>         self isReadOnlyObject ifTrue: [(ModificationForbidden for: self atInstVar: index with: anObject) signal]
>>> 	self primitiveFailed
>>> 
>>> All primitive fall-back code triggering object mutation should be rewritten this way, especially primitives such as #at:put:, #instVarAt:put:, etc.
>>> 
>>> Cheers
>>> 
>>> On Fri, Jan 5, 2018 at 1:42 PM, Norbert Hartl <norbert at hartl.name <mailto:norbert at hartl.name>> wrote:
>>>  
>>> If I do 
>>> 
>>> (#foo -> #bar)
>>> 	beReadOnlyObject;
>>> 	value: #baz
>>> 
>>> it throws
>>> 
>>> ModificationForbidden:  #foo->#bar is read-only, hence its field 2 cannot be modified with #baz
>>> 
>>>  which is as expected. But if I do
>>> 
>>> (#foo -> #bar)
>>> 	beReadOnlyObject;
>>> 	instVarNamed: #value put: #baz
>>> 
>>> I get
>>> 
>>> PrimitiveFailed: primitive #instVarAt:put: in Association failed
>>> 
>>> I think this a bug, no?
>>> 
>>> Norbert
>>> 
>>> 
>>> 
>>> 
>>> -- 
>>> Clément Béra
>>> Pharo consortium engineer
>>> https://clementbera.wordpress.com/ <https://clementbera.wordpress.com/>
>>> Bâtiment B 40, avenue Halley 59650 Villeneuve d'Ascq
> 
> 
> 
> 
> 
> -- 
> Clément Béra
> Pharo consortium engineer
> https://clementbera.wordpress.com/ <https://clementbera.wordpress.com/>
> Bâtiment B 40, avenue Halley 59650 Villeneuve d'Ascq

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squeakfoundation.org/pipermail/vm-dev/attachments/20180106/55b59689/attachment-0001.html>


More information about the Vm-dev mailing list