Issue 3398 in pharo: SystemNavigation and SourcedMethodReference

Status: Accepted
Owner: luc.fabresse
Labels: Milestone-1.2 Type-FailingTest

New issue 3398 by luc.fabresse: SystemNavigation and SourcedMethodReference
http://code.google.com/p/pharo/issues/detail?id=3398

6 tests in SystemNavigationTest and  
SystemNavigationOnNewlyCreatedEnvironementTest are failing in 1.2-12272
This is because of the introduction of SourcedMethodReference.

I already speak about this problem here:  
http://forum.world.st/About-SourcedMethodReference-td3035187.html
SourcedMethodReference>>= must not be based on identity (==) because it  
prevents to have multiple SourcedMethodReference pointing to the same  
CompiledMethod.
And we need that because each tools will use different  
SourcedMethodReference.
IMHO, SourcedMethodReference are just proxies and we don't care of their  
identity.

I attach a cs that make these 6 tests pass.
I replace MethodReference by SourcedMethodReference in SystemNavigationTest  
and al...
And I modified the equality method of SourcedMethodReference:

SourcedMethodReference>>= anotherMethodReference
        ^ (super = anotherMethodReference)
                and: [ (self timeStamp = self timeStamp)
                        and: [ self sourceCode =  self sourceCode ]]


I have no idea of the impact of this change for the SourcedMethodReference  
stuff point of view.


Attachments:
        FixFailingSystemNavigationTests.1.cs  3.6 KB

Updates:
        Status: Fixed

Comment #1 on issue 3398 by siguctua: SystemNavigation and  
SourcedMethodReference
http://code.google.com/p/pharo/issues/detail?id=3398

(No comment was entered for this change.)

Comment #2 on issue 3398 by stephane.ducasse: SystemNavigation and  
SourcedMethodReference
http://code.google.com/p/pharo/issues/detail?id=3398

Luc I revised your definition :)
(self timeStamp = self timeStamp)
                        and: [ self sourceCode =  self sourceCode ]]

looks supiscious and I defined also hash

Comment #3 on issue 3398 by stephane.ducasse: SystemNavigation and  
SourcedMethodReference
http://code.google.com/p/pharo/issues/detail?id=3398

Now my definition of hash did not work:

hash
        ^ ((super hash) bitXor:  self timeStamp)
                        bitXor: self sourceCode

Because timeStamp is nil. So I will rollback this change.

Comment #4 on issue 3398 by luc.fabresse: SystemNavigation and  
SourcedMethodReference
http://code.google.com/p/pharo/issues/detail?id=3398

oups, too fast ;-)
good catch!

Comment #5 on issue 3398 by stephane.ducasse: SystemNavigation and  
SourcedMethodReference
http://code.google.com/p/pharo/issues/detail?id=3398

luc can you run the tests once you will have a solution because this is  
strange and sensible.

Comment #6 on issue 3398 by marcus.denker: SystemNavigation and  
SourcedMethodReference
http://code.google.com/p/pharo/issues/detail?id=3398

Checking the source for equality all the time seems to be far too expensive  
and not needed.
(reading source from disk...)

As the class does not add any more state, why not just re-use the  
definition of the superclass?
(that is, just delete the definition). What is strange is that the  
super-superclass defines state
that the equality of the superclass does not bother with.

The whole thing really needs a cleanup...

Comment #7 on issue 3398 by Benjamin.VanRyseghem.Pharo: SystemNavigation  
and SourcedMethodReference
http://code.google.com/p/pharo/issues/detail?id=3398

@Marcus : here a little fix which is basicly the same, but I compare  
sourcePointer instead of sourceCode :)

Yes it really need a clean up ... We have submit a fix and ask people to  
test it, but nobodies have tried (or at least nobodies have given  
feedback) ...

Attachments:
        EqualInSourcedMethodReference.1.cs  3.5 KB

Updates:
        Status: Closed

Comment #8 on issue 3398 by marcus.denker: SystemNavigation and  
SourcedMethodReference
http://code.google.com/p/pharo/issues/detail?id=3398

12277