[jdk17] RFR: 8254571: Erroneous generic type inference in a lambda expression with a checked exception

[Maurizio Cimadamore]

On Thu, 17 Jun 2021 16:33:11 GMT, Vicente Romero <vromero at openjdk.org> wrote:

> Hi,
> 
> The compiler is issuing an error for code like:
> 
> 
> class JDK8254571 {
>     public static void test() {
>         outer(nested(x -> mightThrow()));
>     }
>     
>     static <A> void outer(Object o) {}
> 
>     static <B, C, E extends Throwable> B nested(Fun<C,E> fun) {
>         return null;
>     }
> 
>     interface Fun<X, Y extends Throwable> {
>         void m(X t) throws Y;
>     }
> 
>     static void mightThrow() throws Exception {}
> } 
> 
> 
> there are several things at play here:
> 
> 1) for performance reasons we decided to minimize the number of inference variables popped up when inferring nested generic method invocations, see Infer::roots
> 2) when an expression is stuck (like in this case this implicit lambda) we decide which inference variables should be present in the related inference context and thrown variables were not part of the equation
> 
> So there are three hypothesis here:
> 1) there is a bug in the code that determines which variables are passed up to the nested inference context, meaning that it is passing less variables than the bare minimum needed
> 2) there is a bug in determining which inference variables should be extracted from stuck expressions
> 3) both are buggy
> 
> The root cause of this bug is that some types escape from Attr with non instantiated type variables in them. I modified the test case to double check if the issue was not related to lambdas, but then the test passed so in the example above I added:
> 
> 
>      static <X, Y extends Throwable> Fun<X, Y> mightThrow2() throws Y { return null; }
> 
> 
> which simulates the lambda and the compiler has no issues compiling this invocation:
> 
> 
>     outer(nested(mightThrow2()));
> 
> 
> so summing up, after double checking the logic of the code that minimized the inference context I realized that the issue is not there. The code is sound and its output only depend on its input, so the issue should be on determining the input. So IMO the issue is in what inference variables we extract from stuck expressions, which is one of the inputs to Infer::roots. Looking for what variables we extract from stuck expressions, I realized that the code currently ignores type variables in the throws clause and this is why my proposed fix is addressing this issue.
> 
> Comments?

I believe the fix is good. We probably overlooked free type variables in the thrown clause (as they are relatively rare) when doing stuck expression computation. These variables are part of the bound set used by JLS chapter 18, so dropping them on the floor (as current code does) is not a good option. This patch rectifies that.

To test with method references, you would need the method reference to be "non-exact": only in that case will the method reference be considered stuck in the same way the implicit lambda is, I believe.

-------------

Marked as reviewed by mcimadamore (Reviewer).

PR: https://git.openjdk.java.net/jdk17/pull/91