find original change set

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

find original change set

Javier Orsaria
Hi,
    I was trying to use, in VersionsBrowser, the feature "find
original change set" for a method. In old versions (3.8 and earlier)
this work fine. But in 3.10 and 3.9 don´t. I followed the sequence and
detect the problem at MethodChangeRecord-noteNewMethod:

noteNewMethod: newMethod
        "NEVER do this. It is evil."
        currentMethod _ nil.

should be:

noteNewMethod: newMethod
        "NEVER do this. It is evil."
        currentMethod _ newMethod.

This way work as I expect. But with that comment I understand was
intentioned. What is the problem with this? I can´t see it... I'm the
devil?

Thanks,
Javier

Reply | Threaded
Open this post in threaded view
|

Re: find original change set

Jason Johnson-5
I have a question as well: Does anyone know why ChangeSet>>uninstall
is set to just send self halt?  Nearly everything seems to be present
for this to work, did someone just give up at the very last step or?

On 8/10/07, Javier Orsaria <[hidden email]> wrote:

> Hi,
>     I was trying to use, in VersionsBrowser, the feature "find
> original change set" for a method. In old versions (3.8 and earlier)
> this work fine. But in 3.10 and 3.9 don´t. I followed the sequence and
> detect the problem at MethodChangeRecord-noteNewMethod:
>
> noteNewMethod: newMethod
>         "NEVER do this. It is evil."
>         currentMethod _ nil.
>
> should be:
>
> noteNewMethod: newMethod
>         "NEVER do this. It is evil."
>         currentMethod _ newMethod.
>
> This way work as I expect. But with that comment I understand was
> intentioned. What is the problem with this? I can´t see it... I'm the
> devil?
>
> Thanks,
> Javier
>
>


Reply | Threaded
Open this post in threaded view
|

Re: find original change set

Christoph Thiede
In reply to this post by Javier Orsaria
Hi all,

oldest thread I've been replying to so far ever. :D

While refactoring the VersionBrowser's annotations, I just found out that
ChangeRecord>>#originalChangeSetForSelector: is broken because, in my fresh
Trunk image, all ~1800 MethodChangeRecord instances have nil as
currentMethod. The comment in the relevant setter is kind of confusing ...:

MethodChangeRecord>>noteNewMethod: newMethod
        "NEVER do this. It is evil."
        currentMethod := nil.

Why is this?

After searching the mailing archive for a while, I did not only find the
attached message describing the same problem but also an obsolete fragment
from the ReleaseBuilder:

"The pointer to currentMethod is not realy needed (anybody care to fix this)
and often holds on to obsolete bindings"
MethodChangeRecord allInstancesDo: [:each | each noteNewMethod: nil].

But with regard to the present bug, I dare to say: currentMethod is needed
again, because without, #originalChangeSetForSelector: won't be able to
identify the correct method by its position in the changes file.

So why is currentMethod evil? I assume it just would blow up your image
significantly. But I'd like to repair the changeset search.

There are only two other users of currentMethod, #compileAll:from: and
#invokePhase1, which are both defined on ClassChangeRecord and do not have
any eventual senders in the Trunk, so they could probably be deprecated? If
this is correct, my proposal would be as follows:
Remove the currentMethod inst var from MethodChangeRecord, add a position
instvar instead, and adjust #noteNewMethod: and #containsMethodAtPosition:
accordingly.

Does this sound reasonable to you?
It is safe to assume that the position of a CompiledMethod instance will not
change again unless the sources file is updated?

I would be glad about your help!

Best,
Christoph



-----
Carpe Squeak!
--
Sent from: http://forum.world.st/Squeak-Dev-f45488.html

Carpe Squeak!