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

[Eliot Miranda]

Hi Norbert,

On Sat, Jan 6, 2018 at 6:09 AM, Norbert Hartl <norbert at hartl.name> wrote:

>
> 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> 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.
>

Ah, that's a great distinction.  Indeed NoModificationException is a better
name.  So ignore my comments about orthogonality.  I like the distinction
between exceptions being resumable and errors not,.  I've learned
something.  Thank you.


>
> 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>:
>>
>> 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> 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> 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/
>> Bâtiment B 40, avenue Halley 59650 Villeneuve d'Ascq
>>
>>
>>
>>
>
>
> --
> Clément Béra
> Pharo consortium engineer
> https://clementbera.wordpress.com/
> Bâtiment B 40, avenue Halley 59650 Villeneuve d'Ascq
>
>
>
>


-- 
_,,,^..^,,,_
best, Eliot
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squeakfoundation.org/pipermail/vm-dev/attachments/20180106/3b082242/attachment.html>