On Fri, Jan 5, 2018 at 2:34 PM, Eliot Miranda eliot.miranda@gmail.com wrote:

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.

That was discussed on the Pharo mailing list.

I used the same name and it was changed by other contributors.

_,,,^..^,,,_ (phone)

On Jan 5, 2018, at 5:22 AM, Clément Bera bera.clement@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@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

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?

Norbert

Am 05.01.2018 um 14:34 schrieb Eliot Miranda eliot.miranda@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@gmail.com mailto:bera.clement@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@hartl.name mailto:norbert@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

Thanks Clement!

Am 06.01.2018 um 14:44 schrieb Clément Bera bera.clement@gmail.com:

On Sat, Jan 6, 2018 at 1:32 PM, Norbert Hartl <norbert@hartl.name mailto:norbert@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@gmail.com mailto:eliot.miranda@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@gmail.com mailto:bera.clement@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@hartl.name mailto:norbert@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

Hi Norbert,

On Sat, Jan 6, 2018 at 4:32 AM, Norbert Hartl norbert@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?

At least for me, errors are orthogonal to reusability. In fact, that so many errors are not resumable is extremely annoying. Take the example of reading a directory. If one doesn't have the permissions to read a directory then attempting to do so is definitely an error. But if one is trying to write a find-like utility that searches a directory hierarchy then one definitely wants to resume from such an error by answering empty contents. Otherwise one's search terminates at the first unreadable directory. So very much,reusability is about an operation's results and continuation, not about it being an error or a warning.

I don't think that NoModificationErrora are very likely to be resumed. It depends on context. If one were to try and modify a literal then one would not expect that error to be resumed. If one is using a, object-to-database mapping scheme then yes, one would expect it to be resumed. But it depends on what the application is doing, and so I don't think one can say anything about likelihood.

The name is NoModificationError since that's exactly what it is. The per-object read-only bit stipulates that the object is not modifiable and any attempt to modify its state (which includes what its class is) causes an error. It's not a NoModificationWarning. It's a hard error. And in the case of a literal (or any other object we might want one not to be able to modify inadvertently such as a method) we would not expect any kind of resumption.

I hope this is clearer :-)

Norbert

Am 05.01.2018 um 14:34 schrieb Eliot Miranda eliot.miranda@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@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@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

On Fri, Jan 5, 2018 at 4:05 PM, Norbert Hartl norbert@hartl.name wrote:

You mean

<primitive: 174 error: ec> self isReadOnlyObject ifTrue: [(ModificationForbidden for: self atInstVar: index with: anObject) signal] ifFalse: [ self primitiveFailed ] ?

Norbert

yes something like that.

Same thing for Object>>#at:put:, floatAt:put: and so on.

Am 05.01.2018 um 14:22 schrieb Clément Bera bera.clement@gmail.com:

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

On 05-01-2018, at 9:17 AM, Eliot Miranda eliot.miranda@gmail.com wrote:

The index and bounds validation should also occur, probably first. So something like

Object>>instVarAt: index put: anObject <primitive: 174 error: ec> (index isInteger and: [index between: 1 and: self class instSize + self basicSize]) ifFalse: [self badIndexError: index]. self isReadOnlyObject ifTrue: [^(NoModificationError for: self atInstVar: index with: anObject) signal]. self primitiveFailed

Surely the no-modification check ought to come first since the index is completely irrelevant in that case?

And obviously we ought to be doing cleverer things with the returned error codes; we could avoid the all the tests here since the prim already provides error codes for bad argument (the index isInteger test), bad index (index bounds) and bad rcvr (immutability - though might we not want a specific error here instead?) I know, I know; somebody has to find the time to do the work.

tim -- tim Rowledge; tim@rowledge.org; http://www.rowledge.org/tim Never forget: 2 + 2 = 5 for extremely large values of 2.

Am 05.01.2018 um 16:53 schrieb Clément Bera bera.clement@gmail.com:

On Fri, Jan 5, 2018 at 4:05 PM, Norbert Hartl <norbert@hartl.name mailto:norbert@hartl.name> wrote:

You mean

<primitive: 174 error: ec> self isReadOnlyObject ifTrue: [(ModificationForbidden for: self atInstVar: index with: anObject) signal] ifFalse: [ self primitiveFailed ] ?

Norbert

yes something like that.

Same thing for Object>>#at:put:, floatAt:put: and so on.

I’m working on it and came by

Object>>#primitiveChangeClassTo:

and I wonder what would be the approach here. The ModificationForbidden/NoModificationError seems to be tight to an index. What would be the case for a class change? Another Exception class? How is that solved in VW?

Norbert

...

Am 05.01.2018 um 14:22 schrieb Clément Bera <bera.clement@gmail.com mailto:bera.clement@gmail.com>:

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@hartl.name mailto:norbert@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

Hi Norbert, Hi Denis,

On Sat, Jan 6, 2018 at 4:06 AM, Norbert Hartl norbert@hartl.name wrote:

Am 05.01.2018 um 16:53 schrieb Clément Bera bera.clement@gmail.com:

On Fri, Jan 5, 2018 at 4:05 PM, Norbert Hartl norbert@hartl.name wrote:

...

You mean

<primitive: 174 error: ec> self isReadOnlyObject ifTrue: [(ModificationForbidden for: self atInstVar: index with: anObject) signal] ifFalse: [ self primitiveFailed ] ?

Norbert

yes something like that.

Same thing for Object>>#at:put:, floatAt:put: and so on.

I’m working on it and came by

Object>>#primitiveChangeClassTo:

and I wonder what would be the approach here. The ModificationForbidden/NoModificationError seems to be tight to an index. What would be the case for a class change? Another Exception class? How is that solved in VW?

I don't think that adding variants of NoModificationError is wise. One can imagine adding other state changes that read-onliness should prevent, such as unpinning. So one could end up having to add a lot of subclasses of NoModificationError, each rather inflexible and able to respond to only one specific error. We would end up with at least InstanceVariableModificationError IndexedVariableModificationError ClassModificationError BecomeIdentotyChangeError etc. And even then IndexedVariableModificationError would need a message since there are many different kinds of at:put:s, at:put:, byteAt:put: unsignedLongAt:put: etc.

In VW it is handled by the NoModificationError holding a Message that can be used for the retry operation. i.e.

Object>>noModificationErrorFor: selector index: index value: value ^(NoModificationError receiver: self selector: selector index: index value: value) raiseRequest

(N.B. the following is sent by the VM when an inst var assignment fails due to read-onlyness; see recreateSpecialObjectsArray) Object>>attemptToAssign: aValue withIndex: index ^self noModificationErrorFor: #instVarAt:put: index: index value: aValue

Object>>at: index put: value "Store the argument value in the indexable field of the receiver indicated by index. Fail if the index is not an Integer or is out of bounds. Fail if the value is not of the right type for this kind of collection. Answer the value that was stored."

<primitive: 61 errorCode: ec> index isInteger ifTrue: [(index >= 1 and: [index <= self basicSize]) ifTrue: [self isImmutable ifTrue: [^self noModificationErrorFor: #at:put: index: index value: value] ifFalse: [^self improperStoreError]] ifFalse: [^self subscriptBoundsErrorFor: #at:put: index: index value: value]]. index respondsToArithmetic ifTrue: [^self at: index asSmallInteger put: value] ifFalse: [^self nonIntegerIndexError: index]

Behavior>>adoptInstance: anObject "Changes the class of the argument to be that of the receiver provided that a) anObject is not immutable (which includes immediates) b) anObject is bits and the receiver is a bits class, or c) anObject is pointers and the receiver is pointers and anObject has the same number of inst vars as specified by the receiver, or the receiver is variable and anObject has at least as many inst vars as specified by the receiver. Answers the receiver."

<primitive: 536 errorCode: errCode> ^(errCode ~~ nil and: [errCode name = #'no modification']) ifTrue: [anObject noModificationErrorFor: #changeClassTo: index: nil value: self] ifFalse: [self primitiveFailed]

etc. This is much more flexible and works well in practice. In the vw7.7nc image I looked at there are 20 senders of #noModificationErrorFor:index:value: for variants of 5 operations:

- attempting to assign to an inst var of a read-only object, either directly in code via an inst var assignment or via an instVarAt:put: send - attempting to assign to an indexed inst var of a read-only object, via an at:put: send (at:put: byteAt:put: unsignedLongAt:put: etc) - attempting to assign to an indexed inst var of a read-only object via a BitBlt primitive (e.g. copyBitsClippedStride:width:atX:y:from:stride:width:atX:y:width:height:rule:) - attempting to become a read-only object via two-way become; forwarding become is not considered a modification. - attempting to change the class of a read-only object

Digression:

Looking at the above the obvious convenience is to add

Object>>noModificationErrorFor: selector value: value ^(NoModificationError receiver: self selector: selector index: nil value: value) raiseRequest

which would be applicable in 8 of the 20 uses, but it would imply adding overrides in subclasses. #noModificationErrorFor:index:value: is implemented in ProtoObect, Object, Symbol and VariableBinding. The implementations in Symbol and VariableBinding simply provide more explanatory error messages.

HTH

Norbert

...

Am 05.01.2018 um 14:22 schrieb Clément Bera bera.clement@gmail.com:

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

Am 06.01.2018 um 19:45 schrieb Eliot Miranda eliot.miranda@gmail.com:

Hi Norbert, Hi Denis,

On Sat, Jan 6, 2018 at 4:06 AM, Norbert Hartl <norbert@hartl.name mailto:norbert@hartl.name> wrote:

...

Am 05.01.2018 um 16:53 schrieb Clément Bera <bera.clement@gmail.com mailto:bera.clement@gmail.com>:

On Fri, Jan 5, 2018 at 4:05 PM, Norbert Hartl <norbert@hartl.name mailto:norbert@hartl.name> wrote:

You mean

<primitive: 174 error: ec> self isReadOnlyObject ifTrue: [(ModificationForbidden for: self atInstVar: index with: anObject) signal] ifFalse: [ self primitiveFailed ] ?

Norbert

yes something like that.

Same thing for Object>>#at:put:, floatAt:put: and so on.

I’m working on it and came by

Object>>#primitiveChangeClassTo:

and I wonder what would be the approach here. The ModificationForbidden/NoModificationError seems to be tight to an index. What would be the case for a class change? Another Exception class? How is that solved in VW?

I don't think that adding variants of NoModificationError is wise. One can imagine adding other state changes that read-onliness should prevent, such as unpinning. So one could end up having to add a lot of subclasses of NoModificationError, each rather inflexible and able to respond to only one specific error. We would end up with at least InstanceVariableModificationError IndexedVariableModificationError ClassModificationError BecomeIdentotyChangeError etc. And even then IndexedVariableModificationError would need a message since there are many different kinds of at:put:s, at:put:, byteAt:put: unsignedLongAt:put: etc.

In VW it is handled by the NoModificationError holding a Message that can be used for the retry operation. i.e.

Object>>noModificationErrorFor: selector index: index value: value ^(NoModificationError receiver: self selector: selector index: index value: value) raiseRequest

(N.B. the following is sent by the VM when an inst var assignment fails due to read-onlyness; see recreateSpecialObjectsArray) Object>>attemptToAssign: aValue withIndex: index ^self noModificationErrorFor: #instVarAt:put: index: index value: aValue

Object>>at: index put: value "Store the argument value in the indexable field of the receiver indicated by index. Fail if the index is not an Integer or is out of bounds. Fail if the value is not of the right type for this kind of collection. Answer the value that was stored."

<primitive: 61 errorCode: ec> index isInteger ifTrue: [(index >= 1 and: [index <= self basicSize]) ifTrue: [self isImmutable ifTrue: [^self noModificationErrorFor: #at:put: index: index value: value] ifFalse: [^self improperStoreError]] ifFalse: [^self subscriptBoundsErrorFor: #at:put: index: index value: value]]. index respondsToArithmetic ifTrue: [^self at: index asSmallInteger put: value] ifFalse: [^self nonIntegerIndexError: index]

Behavior>>adoptInstance: anObject "Changes the class of the argument to be that of the receiver provided that a) anObject is not immutable (which includes immediates) b) anObject is bits and the receiver is a bits class, or c) anObject is pointers and the receiver is pointers and anObject has the same number of inst vars as specified by the receiver, or the receiver is variable and anObject has at least as many inst vars as specified by the receiver. Answers the receiver."

<primitive: 536 errorCode: errCode> ^(errCode ~~ nil and: [errCode name = #'no modification']) ifTrue: [anObject noModificationErrorFor: #changeClassTo: index: nil value: self] ifFalse: [self primitiveFailed]

etc. This is much more flexible and works well in practice. In the vw7.7nc image I looked at there are 20 senders of #noModificationErrorFor:index:value: for variants of 5 operations:

• attempting to assign to an inst var of a read-only object, either directly in code via an inst var assignment or via an instVarAt:put: send
• attempting to assign to an indexed inst var of a read-only object, via an at:put: send (at:put: byteAt:put: unsignedLongAt:put: etc)
• attempting to assign to an indexed inst var of a read-only object via a BitBlt primitive (e.g. copyBitsClippedStride:width:atX:y:from:stride:width:atX:y:width:height:rule:)
• attempting to become a read-only object via two-way become; forwarding become is not considered a modification.
• attempting to change the class of a read-only object

Digression:

Looking at the above the obvious convenience is to add

Object>>noModificationErrorFor: selector value: value ^(NoModificationError receiver: self selector: selector index: nil value: value) raiseRequest

which would be applicable in 8 of the 20 uses, but it would imply adding overrides in subclasses. #noModificationErrorFor:index:value: is implemented in ProtoObect, Object, Symbol and VariableBinding. The implementations in Symbol and VariableBinding simply provide more explanatory error messages.

I forgot the ask why the implementation should be in both ProtoObject and Object.

Norbert

HTH

Norbert

...

...

Am 05.01.2018 um 14:22 schrieb Clément Bera <bera.clement@gmail.com mailto:bera.clement@gmail.com>:

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@hartl.name mailto:norbert@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

-- _,,,^..^,,,_ best, Eliot

Hi Eliot.

I definitely sure that nil parameter smells very bad:

Object>>noModificationErrorFor: selector value: value ^(NoModificationError receiver: self selector: selector *index: nil* value: value) raiseRequest

Because in that case all users will be forced to check nils if they are interesting only about field modification. I can imaging such library which records history of object changes. It will need exact field index which was modified. Also ORM frameworks needs to handle differently field modification and class modification of objects. Because new class can be mapped in different table and such update will require complex logic. It would be natural to dispatch concrete logic by error classes.

So I would introduce little hierarchy which I already mentioned:

ModificationForbidden FieldModificationForbidden ClassModificationForbidden BecomeModificationForbidden

On the base class there will be object, newValue, retrySelector and isMirror flag. And fieldIndex will be moved to the FieldModificationForbidden. Mirror flag is required to correctly retry mirror based modification because first argument in that case is always "actual receiver" instead of index.

I split class and become based modifications because it is logically different errors. Maybe it is not important distinction from the first look but at least it is easy to name them separately. Do we have other cases which are not related to field modification?

Also I have question about become which I sent to VM list.

Best regards, Denis

2018-01-06 19:45 GMT+01:00 Eliot Miranda eliot.miranda@gmail.com:

Hi Norbert, Hi Denis,

On Sat, Jan 6, 2018 at 4:06 AM, Norbert Hartl norbert@hartl.name wrote:

...

Am 05.01.2018 um 16:53 schrieb Clément Bera bera.clement@gmail.com:

On Fri, Jan 5, 2018 at 4:05 PM, Norbert Hartl norbert@hartl.name wrote:

...

You mean

<primitive: 174 error: ec> self isReadOnlyObject ifTrue: [(ModificationForbidden for: self atInstVar: index with: anObject) signal] ifFalse: [ self primitiveFailed ] ?

Norbert

yes something like that.

Same thing for Object>>#at:put:, floatAt:put: and so on.

I’m working on it and came by

Object>>#primitiveChangeClassTo:

and I wonder what would be the approach here. The ModificationForbidden/NoModificationError seems to be tight to an index. What would be the case for a class change? Another Exception class? How is that solved in VW?

I don't think that adding variants of NoModificationError is wise. One can imagine adding other state changes that read-onliness should prevent, such as unpinning. So one could end up having to add a lot of subclasses of NoModificationError, each rather inflexible and able to respond to only one specific error. We would end up with at least InstanceVariableModificationError IndexedVariableModificationError ClassModificationError BecomeIdentotyChangeError etc. And even then IndexedVariableModificationError would need a message since there are many different kinds of at:put:s, at:put:, byteAt:put: unsignedLongAt:put: etc.

In VW it is handled by the NoModificationError holding a Message that can be used for the retry operation. i.e.

Object>>noModificationErrorFor: selector index: index value: value ^(NoModificationError receiver: self selector: selector index: index value: value) raiseRequest

(N.B. the following is sent by the VM when an inst var assignment fails due to read-onlyness; see recreateSpecialObjectsArray) Object>>attemptToAssign: aValue withIndex: index ^self noModificationErrorFor: #instVarAt:put: index: index value: aValue

Object>>at: index put: value "Store the argument value in the indexable field of the receiver indicated by index. Fail if the index is not an Integer or is out of bounds. Fail if the value is not of the right type for this kind of collection. Answer the value that was stored."

<primitive: 61 errorCode: ec> index isInteger ifTrue: [(index >= 1 and: [index <= self basicSize]) ifTrue: [self isImmutable ifTrue: [^self noModificationErrorFor: #at:put: index: index value: value] ifFalse: [^self improperStoreError]] ifFalse: [^self subscriptBoundsErrorFor: #at:put: index: index value: value]]. index respondsToArithmetic ifTrue: [^self at: index asSmallInteger put: value] ifFalse: [^self nonIntegerIndexError: index]

Behavior>>adoptInstance: anObject "Changes the class of the argument to be that of the receiver provided that a) anObject is not immutable (which includes immediates) b) anObject is bits and the receiver is a bits class, or c) anObject is pointers and the receiver is pointers and anObject has the same number of inst vars as specified by the receiver, or the receiver is variable and anObject has at least as many inst vars as specified by the receiver. Answers the receiver."

<primitive: 536 errorCode: errCode> ^(errCode ~~ nil and: [errCode name = #'no modification']) ifTrue: [anObject noModificationErrorFor: #changeClassTo: index: nil value: self] ifFalse: [self primitiveFailed]

etc. This is much more flexible and works well in practice. In the vw7.7nc image I looked at there are 20 senders of #noModificationErrorFor:index:value: for variants of 5 operations:

• attempting to assign to an inst var of a read-only object, either

directly in code via an inst var assignment or via an instVarAt:put: send

• attempting to assign to an indexed inst var of a read-only object, via

an at:put: send (at:put: byteAt:put: unsignedLongAt:put: etc)

• attempting to assign to an indexed inst var of a read-only object via a

BitBlt primitive (e.g. copyBitsClippedStride: width:atX:y:from:stride:width:atX:y:width:height:rule:)

• attempting to become a read-only object via two-way become; forwarding

become is not considered a modification.

• attempting to change the class of a read-only object

Digression:

Looking at the above the obvious convenience is to add

Object>>noModificationErrorFor: selector value: value ^(NoModificationError receiver: self selector: selector index: nil value: value) raiseRequest

which would be applicable in 8 of the 20 uses, but it would imply adding overrides in subclasses. #noModificationErrorFor:index:value: is implemented in ProtoObect, Object, Symbol and VariableBinding. The implementations in Symbol and VariableBinding simply provide more explanatory error messages.

HTH

...

Norbert

...

Am 05.01.2018 um 14:22 schrieb Clément Bera bera.clement@gmail.com:

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@hartl.name wro te:

...

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