RFR: JDK-8203277: preflow visitor used during lambda attribution shouldn't visit class definitions inside the lambda body

[Vicente Romero]

On 11/20/18 6:54 AM, Maurizio Cimadamore wrote:
>
> Looks good - why is there a new change in the test?
>

good catch, I didn't think much about this but you are correct that at 
first glance the particular test case compiled in the test 
TestGetScopeResult shouldn't trigger the code in the patch. We are 
talking about this case:

class Test {
     void test() {
         cand((t, var s) -> "");
     }
     void cand(I i) {}

     interface I {
         public String test(String s);
     }
}

which fails during the parser phase so it shouldn't exercise the new 
code added in this patch. But the test TestGetScopeResult forces the 
attribution of the test above when it does:

     ...
     Scope scope = Trees.instance(t).getScope(new 
TreePath(getCurrentPath(), node.getBody()));
     ...

and then we have the following invocation chain:

JavacTrees::getScope -> JavacTrees::getAttrContext -> 
JavacTrees::attribStatToTree -> Attr::attribStatToTree

which finally forces the attribution of the lambda and thus the 
execution of the new code.

Thanks,
Vicente

> Maurizio
>
> On 19/11/2018 20:57, Vicente Romero wrote:
>> Hi Maurizio,
>>
>> On 11/9/18 5:57 AM, Maurizio Cimadamore wrote:
>>>
>>> Looks good - can we cook up a test that is also stressing the other 
>>> new code path?
>>>
>>
>> what about: http://cr.openjdk.java.net/~vromero/8203277/webrev.02/?
>>
>>> Maurizio
>>>
>>
>> Thanks,
>> Vicente
>>
>>> On 09/11/2018 00:12, Vicente Romero wrote:
>>>>
>>>>
>>>> On 11/8/18 6:02 PM, Maurizio Cimadamore wrote:
>>>>>
>>>>> Hi,
>>>>> I agree that preflow shouldn't care about nested constructs; but I 
>>>>> wonder - is diamond anon class the only case we need to worry 
>>>>> about here? What about a nested lambda expression?
>>>>>
>>>>
>>>> right I agree with you, what about [3]?
>>>>
>>>>> Maurizio
>>>>>
>>>> Vicente
>>>>
>>>> [3] http://cr.openjdk.java.net/~vromero/8203277/webrev.01/
>>>>>
>>>>> On 08/11/2018 00:24, Vicente Romero wrote:
>>>>>> Please review the fix for [1] at [2]. The bug can be illustrated 
>>>>>> with this test case:
>>>>>>
>>>>>> import java.util.List;
>>>>>> import java.util.function.Function;
>>>>>>
>>>>>> class DiamondInsideLambdaTest {
>>>>>>     void build() {
>>>>>>         List<Function<String, Double>> list = transform(null,
>>>>>>                 builder -> new Function<>() {
>>>>>>                     public Double apply(String params) { return 
>>>>>> null; }
>>>>>>                 });
>>>>>>     }
>>>>>>
>>>>>>     static <Argument, Result> List<Result> 
>>>>>> transform(List<Argument> fromList, Function<? super Argument,? 
>>>>>> extends Result> function) { return null; }
>>>>>> }
>>>>>>
>>>>>> During lambda attribution, there is a preflow visitor that fill 
>>>>>> some holes, missing types / symbols, before doing an special flow 
>>>>>> analysis on the lambda. This flow analysis skip inner classes 
>>>>>> defined inside the lambda body. The preflow visitor is anyway 
>>>>>> visiting inner classes and filling, with garbage, some of the 
>>>>>> wholes. In most cases as the preflow step happens after the 
>>>>>> lambda body has been attributed, it should be a no-op. But if the 
>>>>>> lambda body contains a diamond as in this case, then it could be 
>>>>>> that the attribution can't be done previous to the preflow step. 
>>>>>> Simply because some types in the diamond could be waiting for 
>>>>>> type inference to come to a resolution. In that case preflow will 
>>>>>> modify the diamond expression and later on during completion, as 
>>>>>> some symbols and types are filled with erroneous types, they will 
>>>>>> keep that erroneous type.
>>>>>>
>>>>>> [1] https://bugs.openjdk.java.net/browse/JDK-8203277
>>>>>> [2] http://cr.openjdk.java.net/~vromero/8203277/webrev.00/ 
>>>>
>>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/compiler-dev/attachments/20181126/1e17e9e2/attachment-0001.html>