search

hpi-swa / sandblocks

A projectional editor for Squeak/Smalltalk.
MIT License
35 stars 3 forks source link

Comparison of methods in CodeHolder is wrong #71

Open LinqLover opened 2 years ago

LinqLover commented 2 years ago

sandblocks-bug-method-equality

Please let me know if this is caused by a bug in the Trunk.

tom95 commented 2 years ago

Yeah, there was a discussion about this at some point with no clear agreement in the end -- I tried finding the thread again just now but without success. Keywords should have been something along the lines of "compiledmethod" and "literals". The problem as far as I remember was that in = compiledmethod did not check the selector for equality. Or, alternatively, that the selector was not part of the method's embedded literals.

LinqLover commented 2 years ago

That's likely, in this case Sandblocks needs to check the selector separately, I think.

tom95 commented 2 years ago

I believe it was actually a regression from behavior in 5.3, but I'll have to look it up again :/

tom95 commented 2 years ago

no, not a regression. I just checked 5.3 and 5.0 and the behavior was the same. So I guess we have to contend with that behavior.

This is tricky because the methods are just another type of artefact in Sandblocks, and Sandblocks expects to be able to find that the same artefact is opened somewhere using #=, which also works for every other type of artefact that we've been using thus far. Options include littering the code with special cases for CompiledMethods, switching to identity comparisons, or adding a new sameArtefact: hook in Object.

LinqLover commented 2 years ago

Do you have the thread? Maybe we could change this in the Trunk ...

tom95 commented 2 years ago

Thinking about it again, it probably makes sense to introduce a dedicated hook for this. In fact, comparing the selector doesn't suffice in the general case, we would also have to compare the class.

LinqLover commented 2 years ago

Or just switch to identity comparison. If a method has been recompiled, it should be re-rendered in Sandblocks anyway, shouldn't it?

tom95 commented 2 years ago

It used to be identity comparison, which I switched when I started work on the debugger. Maybe that was a bad call. I'll try it with identity comparison for a day and see where things break :)

LinqLover commented 2 years ago

FYIO, CompiledMethod >> #= indeed behaves as intended in the Trunk: http://lists.squeakfoundation.org/pipermail/squeak-dev/2022-January/218148.html

By the way, another comparison strategy would be to compare the compiled method's code references, which would also treat different versions of a method as equal (not sure if this would be intended though). :)