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.
Thanks for the detailed answer. I’m not fully convinced that stripping off the quality for variants of forbidden modification is the right way to go. But I also had the gut feeling multiple classes could be over-done. I try and see if I’m satisfied with the single exception. Why I would favor the sublcasses is the fact that modifify the position of an object, modifying the shape of an object and modifying the state of an object are separated concerns that usually don’t go together. So the potential that each of the three can be treated differently could be helpful.
thanks again,
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