Issue 4538 in pharo: (Array & CompiledMethod)>>hasLiteral: bug

Status: New
Owner: ----

New issue 4538 by [hidden email]: (Array &  
CompiledMethod)>>hasLiteral: bug
http://code.google.com/p/pharo/issues/detail?id=4538

Pharo1.3 update: #13269

1. Find a compiled method with a literal, say #DateAndTime->DateAndTime
2. call #hasLiteral: #DateAndTime->DateAndTime

Returns false, because the comparison is done with #==.


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Comment #1 on issue 4538 by [hidden email]: (Array &  
CompiledMethod)>>hasLiteral: bug
http://code.google.com/p/pharo/issues/detail?id=4538

See  
http://forum.world.st/Why-does-CompiledMethod-gt-gt-hasLiteral-use-td3676573.html 
for discussion.

Fix from Eliot Miranda attached.

Attachments:
        hasLiteral.st  1014 bytes


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Comment #2 on issue 4538 by [hidden email]: (Array &  
CompiledMethod)>>hasLiteral: bug
http://code.google.com/p/pharo/issues/detail?id=4538

I'd say like Igor, shared variables (global, pool or class vars) works  
because a variable binding (an Association) is shared among all  
CompiledMethods. When one change the value of the binding, the other gets  
changed too.
I think hasLiteral: should reflect this property.

I propose to enforce this by refining Association>>literalEqual:



_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Comment #3 on issue 4538 by [hidden email]: (Array &  
CompiledMethod)>>hasLiteral: bug
http://code.google.com/p/pharo/issues/detail?id=4538

I'd say like Igor, shared variables (global, pool or class vars) works  
because a variable binding (an Association) is shared among all  
CompiledMethods. When one change the value of the binding, the other gets  
changed too.
I think hasLiteral: should reflect this property.

I propose to enforce this by refining Association>>literalEqual:



_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Comment #4 on issue 4538 by [hidden email]: (Array &  
CompiledMethod)>>hasLiteral: bug
http://code.google.com/p/pharo/issues/detail?id=4538

I put two slices in inbox.
I think hasLiteral: and refersToLiteral: are almost doing the same job and  
I see no good reason to maintain the two. Squeak did remove  
#refersToLiteral:

This does not solve the said "bug".
Instead, you should use
     hasLiteralSuchThat: [:lit | lit = (#DateAndTime->DateAndTime)].
Or
     hasLiteral: (Smalltalk globals associationAt: #DateAndTime).


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Comment #5 on issue 4538 by [hidden email]: (Array &  
CompiledMethod)>>hasLiteral: bug
http://code.google.com/p/pharo/issues/detail?id=4538

I don't see the advantage to keeping == semantics. When would this be  
helpful? Is hasLiteral: used like that by the system somewhere? Anytime a  
user would call hasLiteral:, how would they get the literal they were  
trying to find before finding it to pass to the search method!!


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Updates:
        Status: ReviewNeeded
        Labels: Milestone-1.4

Comment #6 on issue 4538 by [hidden email]: (Array &  
CompiledMethod)>>hasLiteral: bug
http://code.google.com/p/pharo/issues/detail?id=4538

Nicolas we looked at your changes with igor and we like them.  We  
integrated them in 14053. Now I'm a bit confused by the changes of eliot,  
the class checks. I'm too tired to think right now. So for now we will wait  
a bit before pushing them.


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Comment #7 on issue 4538 by [hidden email]: (Array &  
CompiledMethod)>>hasLiteral: bug
http://code.google.com/p/pharo/issues/detail?id=4538

Eliot reported a problem:
class side method have non unique (nil -> a Metaclass) as literal and  
cannot be compared equal...

This literal serves one purpose: identifying the class where the  
compiledMethod is defined. It is used
- by VM for invoking super,
- by some tools for browsing cross references.
There is not much advantage to use a literal Association instead of just  
the class, except:
- that the CompiledMethod is already prepared to handle Association literals
- it enables ClassBuilder to perform a cheap mutation by just changing the  
value

Using an Association raises some problems, we can easily break super by  
just changing a value in a SystemDictionary...

In the long term, we should better use direct reference to class rather  
than an Association.

In the short term, we can place workarounds in method comparison for the  
last literal, and for example test the special case of Association with nil  
key.


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Comment #8 on issue 4538 by [hidden email]: (Array &  
CompiledMethod)>>hasLiteral: bug
http://code.google.com/p/pharo/issues/detail?id=4538

Thanks for the comment. Just dead tired as usual but I was wondering which  
literal for VM for invoking super?
How does it work on class method if the association is (nil -> ...) this is  
always looking in the value of the association? Probably since the other is  
nil. But I wanted to be sure.


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Comment #9 on issue 4538 by [hidden email]: (Array &  
CompiledMethod)>>hasLiteral: bug
http://code.google.com/p/pharo/issues/detail?id=4538

Yes, only the value which holds the class or metaclass is used


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Updates:
        Cc: marianopeck

Comment #10 on issue 4538 by [hidden email]: (Array &  
CompiledMethod)>>hasLiteral: bug
http://code.google.com/p/pharo/issues/detail?id=4538

Mariano I imagine that we can close this issue now?


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker

Updates:
        Status: Closed

Comment #11 on issue 4538 by marianopeck: (Array &  
CompiledMethod)>>hasLiteral: bug
http://code.google.com/p/pharo/issues/detail?id=4538

yes.


_______________________________________________
Pharo-bugtracker mailing list
[hidden email]
http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-bugtracker