deduplicating lambda methods

[B. Blaser]

On 19 March 2018 at 14:10, Vicente Romero <vicente.romero at oracle.com> wrote:
>
>
> On 03/17/2018 10:04 PM, Liam Miller-Cushon wrote:
>>
>> Hi Vicente,
>>
>> On Sat, Mar 17, 2018 at 11:44 AM Vicente Romero <vicente.romero at oracle.com
>> <mailto:vicente.romero at oracle.com>> wrote:
>>
>>     overall looks good, but I think that a set of regression tests
>>     should be included along with the patch.
>>
>>
>> Definitely, I have some functional tests that I'll work on turning into a
>> jtreg test.
>>
>> Do you have advice about how much test coverage is appropriate for tree
>> comparison?
>> Getting to 100% branch coverage would require a lot of test cases. I could
>> probably
>> generate some of them programatically. FWIW a certain amount of TreeDiffer
>> was
>> generated and not hand-written, so any bugs are probably structural
>> problems rather
>> than typos. (That doesn't lessen the need for tests, of course--there's
>> still the risk of
>> adding typos when changes are made in the future.)
>
>
> well I assume that covering most common cases with a combo test should be
> the general guideline here. I don't think we need 100% coverage to validate
> the approach. I would also add some corner cases, probably in separate
> tests, those corner cases should include lambdas returning lambdas, lambdas
> with inner classes in their bodies, etc.

Just playing a bit with this patch, I note that the following example
isn't de-duplicated correctly:

    Runnable r = () -> { int i = 0; if (i==0) {} };
    r = () -> { int i = 0; if (i==0) {} };

because symbols don't seem to be '==' for local variables (is that
right?). I think we should probably consider using the name instead
for local variables, as next.

Bernard


$ diff -u ./src/jdk.compiler/share/classes/com/sun/tools/javac/comp/TreeDiffer.java.r03
./src/jdk.compiler/share/classes/com/sun/tools/javac/comp/TreeDiffer.java
--- ./src/jdk.compiler/share/classes/com/sun/tools/javac/comp/TreeDiffer.java.r03
   2018-03-19 14:59:49.351288750 +0100
+++ ./src/jdk.compiler/share/classes/com/sun/tools/javac/comp/TreeDiffer.java
   2018-03-19 15:03:36.416434155 +0100
@@ -172,6 +172,10 @@
                 result = index == otherIndex;
                 return;
             }
+            else {
+                result = symbol.name == otherSymbol.name;
+                return;
+            }
         }
         result = tree.sym == that.sym;
     }

$ diff -u ./src/jdk.compiler/share/classes/com/sun/tools/javac/comp/TreeHasher.java.r03
./src/jdk.compiler/share/classes/com/sun/tools/javac/comp/TreeHasher.java
--- ./src/jdk.compiler/share/classes/com/sun/tools/javac/comp/TreeHasher.java.r03
   2018-03-19 15:00:02.743120393 +0100
+++ ./src/jdk.compiler/share/classes/com/sun/tools/javac/comp/TreeHasher.java
   2018-03-19 15:01:35.581953251 +0100
@@ -84,6 +84,10 @@
                 hash(idx);
                 return;
             }
+            else {
+                hash(tree.sym.name);
+                return;
+            }
         }
         hash(sym);
     }