[squeak-dev] renaming a class causes invalid super pointer CompiledMethod..!

Bert Freudenberg bert at freudenbergs.de
Tue Jan 20 21:09:25 UTC 2015


(got interrupted writing this, and now the discussion has progressed, but I'll send anyway)

On 20.01.2015, at 21:26, Eliot Miranda <eliot.miranda at gmail.com> wrote:
> 
> This is indeed an Environments regression.  The issue is where to fix the problem.  One way to look at it is that Environment>>renameClass:from:to: does not update bindings in the renamed class's methods.

I sat down with Tobias today, and a fix is in Trunk. It updates the bindings in method literals.

> I bet Colin would have not made this slip had there been a test.

It seemed to me as if Colin made a conscious decision to not keep the binding "alive". Instead, he changed the bindings in the compiled method to be #OldName->nil and puts it in Undeclared, and created a new binding in the environment as #NewName->aClass. There is even a test ensuring that oldBinding ~~ newBinding.

I guess this is to ensure that what the method source says matches what the compiled method does. After renaming the class, the source code still uses OldName, but accepting it as-is would result in a binding like #OldName->nil to be created in Undeclared. Essentially, this way of doing it ensures that decompiling the method is not different from the actual source after a class rename, both use 'OldName'.

However, I'd argue it's more important that the system keeps working while renaming a class. That's what the old scheme did - it simply stored the new class name as key of the binding, not disturbing execution at all.

Our fix restores that behavior, albeit using #becomeForward: as did Colin's code, and we were not exactly sure, why. I still don't see why the become would be necessary.

Note that we did not actually test this with multiple environments, aliases etc.

One idea I had after this is that we perhaps could move the old binding to undeclared (keeping methods both working and decompilable) and additionally create a new binding under the new class name. Perhaps this is what Colin intended all along, and it was only the implementation that was buggy? If we knew exactly what we want, it seems like the environment code is reasonably simple to make it do that.


- Bert -



-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 4115 bytes
Desc: not available
Url : http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20150120/720c43c1/smime.bin


More information about the Squeak-dev mailing list