(original) (raw)

I see looks good, thanks for the explanation.

Maurizio

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> list = transform(null,
builder -> new Function<>() {
public Double apply(String params) { return null; }
});
}

static List transform(List fromList, Function 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/