deduplicating lambda methods

[Vicente Romero]

Hi again,

I could be wrong but it seems to me that TreeHasher generates too many 
false positives as the hash number obtained won't have any topological 
information about the tree. In other words, it seems to me that the hash 
generated will be the same regardless of the way the tree is traversed. 
I think that we are interested in obtaining a hash that is a topological 
descriptor of the tree. This will probably imply making TreeHasher a 
full fledged visitor.

Thanks,
Vicente

On 03/16/2018 03:21 PM, Vicente Romero wrote:
> hum, that one lost the format, reformatting:
>
> Hi Liam,
>
> Thanks for the updates to the patch. I also agree with Maurizio about 
> using TreeScanner. Additional comment: TreeHasher should skip 
> parenthesis of expressions, to obtain the same hash for, for example: 
> return (i + j); and return i + j;
>
> Also, as a bonus, consider constant expressions, we probably want: 
> return 5; and return 2 + 3; to be equal.
>
> I think that these two use cases will need a "normalization" step to 
> be applied to the tree and store in the map, set, only a normalized 
> version of the tree. I understand that at least considering constants 
> could be out of the scope of this patch, so I can see it as a follow 
> up development.
>
> Thanks,
> Vicente
>
> On 03/16/2018 03:17 PM, Vicente Romero wrote:
>> Hi Liam, Thanks for the updates to the patch. I also agree with 
>> Maurizio about using TreeScanner. Additional comment: TreeHasher 
>> should skip parenthesis of expressions, to obtain the same hash for, 
>> for example: return (i + j); and return i + j; Also, as a bonus, 
>> consider constant expressions, we probably want: return 5; and return 
>> 2 + 3; to be equal. I think that these two use cases will need a 
>> "normalization" step to be applied to the tree and store in the map, 
>> set, only a normalized version of the tree. I understand that at 
>> least considering constants could be out of the scope of this patch, 
>> so I can see it as a follow up development. Thanks, Vicente
>>
>>
>>
>> On 03/16/2018 02:46 PM, Maurizio Cimadamore wrote:
>>>
>>>
>>> On 16/03/18 00:36, Liam Miller-Cushon wrote:
>>>> Thanks, I'll investigate that. My initial impression is that using 
>>>> fields
>>>> to replace the return value and the second parameter of the visit 
>>>> methods
>>>> in TreeVisitor will complicate control flow somewhat.
>>>>
>>>> Have you considered adding a TreeVisitor-like API that for JCTrees 
>>>> to javac?
>>> Having a tree scanner accepting a visitor parameter would be good - 
>>> although can be done outside the scope of this patch.
>>>
>>> Note that, in the meantime, it's rather easy to mimic the visitor 
>>> parameter idiom - just define a 'scan' method like this:
>>>
>>> ```
>>> Object parameter; //visitor field
>>>
>>> scan(JCTree tree, Object parameter) {
>>>    Object prev = this.parameter;
>>>    try {
>>>        this.parameter = parameter;
>>>        super.scan(tree);
>>>    } finally {
>>>        this.parameter = prev;
>>>    }
>>> }
>>> ```
>>>
>>> And then your code should not be altered much (I hope! :-)).
>>>
>>> Maurizio
>>>
>>
>