[Vm-dev] Why become should fail on readonly objects?

[Bert Freudenberg]

On Thu, Jan 11, 2018 at 2:57 AM, Eliot Miranda <eliot.miranda at gmail.com>
wrote:

>
> Hi Denis,
>
> On Wed, Jan 10, 2018 at 2:46 AM, Denis Kudriashov <dionisiydk at gmail.com>
> wrote:
>
>>
>> Now if p1 will be read only object then become will fail:
>>
>> p1 := 10 at 20.
>> p2 := 40 at 50.
>>
>> p1 beReadOnlyObject.
>>
>> p1 becomeForward: p2. "==> fail by modification error"
>>
>> My question is why this logic is valid? As simple user I do not see how
>> become modifies the state of receiver p1.
>>
>
> Well, the reason is to prevent changing literals.
>

​I don't find this reason compelling. If the CompiledMethod itself was
read-only​, then yes, trying to change a literal in it should be an error,
just like trying to change any slot in any read-only object.

Let's say you have a method like
>
> Object>>printOn: aStream
> "Append to the argument, aStream, a sequence of characters that
> identifies the receiver."
>
> | title |
> title := self class name.
> aStream
> nextPutAll: (title first isVowel ifTrue: ['an '] ifFalse: ['a ']);
> nextPutAll: title
>
> Without read-only literals there is nothing to stop the programmer from
> making the mistake of doing something like
>     ((Object>>#printOn:) literalAt: 4) at: 1 put: $A
> which would cause Object new to print as 'An Object', not 'an Object'.
> Once literals are read-only then this can't happen; the at:put: will fail.
>

​Agreed. Literals should be read-only, they should never be modified.​

But one can use becomeForward: or become: in exactly the same way.
>

​No you can not. You can only replace the whole literal, but you can not
change it.

Unless become is illegal for read-only objects, there is nothing to stop
> the programmer from making the mistake of doing
>     ((Object>>#printOn:) literalAt: 4) becomeForward: 'An '
>     ((Object>>#printOn:) literalAt: 4) become: 'An '
>

​True. But again, unless the CompiledMethod is read-only, I don't see a
reason why this sho​uld fail any more than

    (Object>>#printOn:) literalAt: 4
 put
: 'An '


> And what I expect is failing on another case which surprisingly do not
>> fail:
>>
>> p1 := 10 at 20.
>> p2 := 40 at 50.
>> array := {  p1. p2}.
>> array beReadOnlyObject.
>> p1 becomeForward: p2.
>>
>> array = {40 at 50. 40 at 50} "==true"
>>
>> Here become operation modifies state of read only object. But it not
>> fails.
>>
>
​I would expect this to fail, too.​

The reason here is to do with instance migration on class definition.  We
> want becomeForward: to update any and all instances of a class when we
> shape change a class.  Further, the only way the VM can prevent this is by
> scanning the entire heap looking for any reference to the receiver of
> becomeForward: from a read-only object.  And we want becomeForward: to be
> fast; the last thing we want is to introduce a full heap scan when we
> introduce read-only objects.  So we're being pragmatic; the definitions
> here, that two-way become fails if either is a read-only object, and that
> one-way become fails if the receiver is read-only, work well with the way
> the system implements instance migration, and allow us to implement become
> in the presence of read-only objects without a full heap scan.
>
> This matches the VW implementation which makes the same choices for the
> same reasons.  It's not ideal, but neither is having the VM catch your last
> case.  Instead it's a workable compromise.  HTH
>

​Okay. If this is the semantics needed for a fast VM, that's a reason I can
accept. It is just surprising that the semantics are both more restrictive
(no become of a read-only object) and less restrictive (become can modify
parts of a read-only object) than

Given this behavior of the VM, wouldn't it make sense to recursively make
all sub-objects read-only when making an object read-only? That would
ensure that parts cannot become'd either.

- Bert -​
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squeakfoundation.org/pipermail/vm-dev/attachments/20180113/45c5dd2d/attachment-0001.html>