deduplicating lambda methods

[Maurizio Cimadamore]

Uhm - seems to me that the currently implemented logic is more subtle 
than comparing symbols by name in the visitIdent case? E.g. it 
(correctly) uses information about the relative position of a lambda 
parameter in the parameter list and compares that instead of names. And 
visitSelect is correctly doing a == on the accessed symbol, so that

class Foo { String x; }


(Foo a) -> a.x

(Foo b) -> b.x

are deemed equal because:

(i) - a and b have same positional info
(ii) - a.x and b.x refer to the same (==) symbol (Foo::x)

Maurizio


On 28/03/18 16:44, B. Blaser wrote:
> On 27 March 2018 at 20:22, B. Blaser <bsrbnd at gmail.com> wrote:
>> On 27 March 2018 at 19:33, Vicente Romero <vicente.romero at oracle.com> wrote:
>>>
>>> On 03/27/2018 12:58 PM, Brian Goetz wrote:
>>>> It looks like there were no changes in the outcome, perhaps because there
>>>> were no within-file duplications in the JDK.  (Which I believe.)  A more
>>>> streams/Rx/CompletableFuture-heavy codebase would likely see an improvement.
>>>
>>> right, no difference :(, let's see what happens with Liam's numbers :)
>> Did you include the improvement I suggested to compare local variables
>> *by name* instead of * by symbol* ?
>> If not, there's quite no chance to find duplicates excepted trivial ones ;-)
> Field access symbols don't seem to be '==' neither (is that right?).
> So, to begin with something simple, I would compare all identifiers by name:
>
> TreeDiffer.visitIdent:
>      result = tree.sym.name == that.sym.name;
>
> TreeDiffer.visitSelect:
>      result = scan(tree.selected, that.selected) && tree.sym.name ==
> that.sym.name;
>
> TreeHasher.visitIdent:
>      hash(sym.name);
>
> TreeHasher.visitSelect:
>      hash(tree.sym.name);
>
> I'll try to write it a bit better, run some more tests and find a
> couple of examples like:
>
>      Runnable r1 = () -> {
>          Runnable r2 = () -> {};
>      };
>      r1 = () -> {
>          Runnable r2 = () -> {};
>      };
>
>      r1 = () -> {
>          Class<?> c = Integer.class;
>      };
>      r1 = () -> {
>          Class<?> c = Integer.class;
>      };
>
> What do you think?
>
> Thanks,
> Bernard
>
>
>> Bernard
>>
>>> Vicente