[Vm-dev] [squeak-dev] Object new becomeForward: #normal

[Eliot Miranda]

Hi Bert,

   [please see the "however" later on]

On Tue, Jul 18, 2017 at 7:30 AM, Bert Freudenberg <bert at freudenbergs.de>
wrote:

> On Tue, Jul 18, 2017 at 4:09 PM, Levente Uzonyi <leves at caesar.elte.hu>
> wrote:
>
>> On Tue, 18 Jul 2017, marcel.taeumel wrote:
>>
>> Hi, there.
>>>
>>> Why are there guards to prevent forward-becoming to immediates (chars,
>>> ints)
>>> but not to symbols?
>>>
>>
>> The former is technically impossible due to different object
>> representations, the latter is possible and not restricted at all. For
>> example, true and false are not immediate objects, you can use #become*: on
>> them to blow your image up.
>> So, there's no restriction at all if it's technically possible to use
>> #become*:.
>> The responsibility model is the simplest here: use at your own risk.
>> Since this comes up every once in a while, I suggest a comment be added
>> to those methods stating the responsibility model explicitly.
>>
>
> ​+1
>
> *Especially* a warning that becomeForward: does modify the target's hash.
> ​
>
>> I don't know how the VM handles immutability in this case, but it's
>> possible that it wouldn't let #become*: affect immutable objects.
>
>
> ​I think that would be fine, you should​ be able to become an immutable
> object and vice versa.
>
>
>> On the other hand, I'm sure it would let you change fields of immutable
>> objects via #become*:, but that's not an issue in your case.
>
>
> ​This is debatable ...​ I would rather have the VM raise an error when
> trying to become a field of an immutable object. Immutable should mean
> immutable, no?
>

Agreed.  I'm modifying the Spur VM to fail becomeForward: when copyHash is
true and the target object is immutable.  So when we have immutable
literals, #normal would be immutable and the become would fail.  The
operative code was

containsOnlyValidBecomeObjects: array1 and: array2 twoWay: isTwoWay
copyHash: copyHash
...
[fieldOffset >= self baseHeaderSize] whileTrue:
[...
isTwoWay
ifTrue:
[...]
ifFalse:
>> [(copyHash and: [self isImmediate: oop2]) ifTrue:
>> [^PrimErrInappropriate]].
fieldOffset := fieldOffset - self bytesPerOop].
...
^0

and is now

containsOnlyValidBecomeObjects: array1 and: array2 twoWay: isTwoWay
copyHash: copyHash
...
[fieldOffset >= self baseHeaderSize] whileTrue:
[...
 isTwoWay
ifTrue:
[...]
ifFalse:
>> [(copyHash and: [(self isImmediate: oop2) or: [self isImmutable: oop2]])
ifTrue:
>> [^PrimErrInappropriate]].
 fieldOffset := fieldOffset - self bytesPerOop].
...
^0

This was a slip on mine and Clément's part when we added read-only object
support.

However, it seems to me that becomerForward: doing a copyHash is itself a
bug.  What's the rationale for copying the receiver's hash to the target
object?  I've seen various frameworks use become to clean-up objects by
doing aNoLongerWantedObject becomeFor4ward: nil, and with our current
default implementation for becomeForward: that will cause havoc.

​- Bert -​
>
> Levente
>>
>> This will break your image: "Object new becomeForward: #normal"
>>>
>>> ... because Cursor class MNU #normal ... :-) Either this is a bug or
>>> forward-becoming to symbols is not intended at all.
>>>
>>> Best,
>>> Marcel
>>>
>>
_,,,^..^,,,_
best, Eliot
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squeakfoundation.org/pipermail/vm-dev/attachments/20170718/61583741/attachment.html>