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
|
Hi, No this is not a bug. 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 <[hidden email]> wrote:
Clément Béra Pharo consortium engineer Bâtiment B 40, avenue Halley 59650 Villeneuve d'Ascq |
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 Fri, Jan 5, 2018 at 2:34 PM, Eliot Miranda <[hidden email]> wrote:
That was discussed on the Pharo mailing list. I used the same name and it was changed by other contributors.
Clément Béra Pharo consortium engineer Bâtiment B 40, avenue Halley 59650 Villeneuve d'Ascq |
In reply to this post by Clément Béra
You mean <primitive: 174 error: ec> self isReadOnlyObject ifTrue: [(ModificationForbidden for: self atInstVar: index with: anObject) signal] ifFalse: [ self primitiveFailed ] ? Norbert
|
On Fri, Jan 5, 2018 at 4:05 PM, Norbert Hartl <[hidden email]> wrote:
yes something like that. Same thing for Object>>#at:put:, floatAt:put: and so on.
Clément Béra Pharo consortium engineer Bâtiment B 40, avenue Halley 59650 Villeneuve d'Ascq |
Hi Norbert,
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
|
In reply to this post by Clément Béra
Hi Clément,
|
In reply to this post by Eliot Miranda-2
> On 05-01-2018, at 9:17 AM, Eliot Miranda <[hidden email]> 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; [hidden email]; http://www.rowledge.org/tim Never forget: 2 + 2 = 5 for extremely large values of 2. |
Hi Tim, > On Jan 5, 2018, at 11:45 AM, tim Rowledge <[hidden email]> wrote: > > > >> On 05-01-2018, at 9:17 AM, Eliot Miranda <[hidden email]> 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? No. That would mean that an attempted out-of-bounds access on a read-only object would raise the no modification error before the bounds violation which could cause a plausible implementation to turn the object into write mode but not be able that complete the write. > > 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; [hidden email]; http://www.rowledge.org/tim > Never forget: 2 + 2 = 5 for extremely large values of 2. > > |
In reply to this post by Clément Béra
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
|
In reply to this post by Eliot Miranda-2
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
|
In reply to this post by NorbertHartl
Hi Am 05.01.2018 um 16:53 schrieb Clément Bera <[hidden email]>: In that case I would introduce hierarchy:
|
In reply to this post by NorbertHartl
On Sat, Jan 6, 2018 at 1:32 PM, Norbert Hartl <[hidden email]> wrote:
Normally the NoModificationError should be able to retry the modification, and then resumes execution just after the primitive call / the inst var store. 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.
Clément Béra Pharo consortium engineer Bâtiment B 40, avenue Halley 59650 Villeneuve d'Ascq |
Thanks Clement! I was actually asking why the mentioned NoModificationError is an error and not a NoModificationException. 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
|
In reply to this post by NorbertHartl
Hi Norbert,
On Sat, Jan 6, 2018 at 4:32 AM, Norbert Hartl <[hidden email]> wrote:
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 :-)
_,,,^..^,,,_ best, Eliot |
In reply to this post by NorbertHartl
Hi Norbert, Hi Denis,
On Sat, Jan 6, 2018 at 4:06 AM, Norbert Hartl <[hidden email]> wrote:
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
_,,,^..^,,,_ best, Eliot |
In reply to this post by NorbertHartl
Hi Norbert,
On Sat, Jan 6, 2018 at 6:09 AM, Norbert Hartl <[hidden email]> wrote:
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.
_,,,^..^,,,_ best, Eliot |
In reply to this post by Eliot Miranda-2
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
|
In reply to this post by Eliot Miranda-2
Norbert
|
Free forum by Nabble | Edit this page |