Tree hashCode/equals final

On Wed, Jan 5, 2011 at 10:50 AM, Mirko Stocker <me@misto.ch> wrote:

Hi!

I noticed that in Changeset 23884, hashCode and equals have changed and are
now final. Is is really necessary to make them final? The problem is that in the
refactoring library, I'm creating new subclasses of Tree and override
hashCode/equals..

Probably this is not necessary. The scala compiler is very unlikely to try to inline any of these methods, and for HotSpot marking a field final makes no difference [1] (this is something that was surprising at first, so I think the article is worth a read). 
Paul can say more about the reasons behind this change. On the other hand, I'm not sure it's such a good idea to change those methods. You can subclass Tree without changing equality. Since you're probably mixing original Scala trees and your own, you may be better off implementing tree comparison/equality using an external method.
iulian[1] http://www.ibm.com/developerworks/java/library/j-jtp04223.html 

Thanks,

Mirko

--
Mirko Stocker | me@misto.ch
Work: http://ifs.hsr.ch | http://infoq.com
Personal: http://misto.ch | http://twitter.com/m_st

--
« Je déteste la montagne, ça cache le paysage »
Alphonse Allais

Hi Iulian!

On Wednesday 05 January 2011 12:45:55 you wrote:
> Probably this is not necessary. The scala compiler is very unlikely to try
> to inline any of these methods, and for HotSpot marking a field final
> makes no difference [1] (this is something that was surprising at first,
> so I think the article is worth a read).

Indeed, that's interesting. (And reminds me that I still haven't read your
thesis :) )

> Paul can say more about the reasons behind this change. On the other hand,
> I'm not sure it's such a good idea to change those methods. You can
> subclass Tree without changing equality. Since you're probably mixing
> original Scala trees and your own, you may be better off implementing tree
> comparison/equality using an external method.

I agree; it just was quite convenient at the time and worked fine. I should
probably change it to avoid any more problems, but I sadly don't have the time
right now... So, to make me (and Eugene) happy, it would be nice if that
'final' could be removed.

Cheers,

Mirko

On Wed, Jan 05, 2011 at 12:45:55PM +0100, iulian dragos wrote:
> Probably this is not necessary. The scala compiler is very unlikely to
> try to inline any of these methods, and for HotSpot marking a field
> final makes no difference [1] (this is something that was surprising
> at first, so I think the article is worth a read).
>
> Paul can say more about the reasons behind this change. On the other
> hand, I'm not sure it's such a good idea to change those methods. You
> can subclass Tree without changing equality. Since you're probably
> mixing original Scala trees and your own, you may be better off
> implementing tree comparison/equality using an external method.

In recent finalizations I've been sending signals to the scala inliner
more than to hotspot. (Even if it will have no effect right now, at
least it could if we wanted it to.) Also when I first started working on
the compiler it took me by surprise that Trees use reference equality,
because they're all case classes. So maybe it was "for emphasis" for
the next guy.

Making your own trees is sketchy no matter what because of all the
places the compiler pattern matches them with a bit of an implicit
closed world assumption: altering their equality is sketchier still.
But far be it for me to stand in the way of living dangerously! So I
fixed it, you can override to your heart's content.

Hi Paul

On Thursday 06 January 2011 00:15:20 you wrote:
> Making your own trees is sketchy no matter what because of all the
> places the compiler pattern matches them with a bit of an implicit
> closed world assumption: altering their equality is sketchier still.

Oh, but that should never be a problem: these trees are created internally in
the refactoring-library and are never passed back to the compiler for further
processing.

> But far be it for me to stand in the way of living dangerously! So I
> fixed it, you can override to your heart's content.

Thanks! I'll go over my code and see if I can change it with reasonable effort
and get back to you so you can make them final again :-)

Cheers,

Mirko