RFR: 8325859: potential information loss during type inference [v3]

[Maurizio Cimadamore]

On Fri, 2 May 2025 20:28:06 GMT, Vicente Romero <vromero at openjdk.org> wrote:

>> When an inference context (`IC`) is minimized, the minimized inference context (`MIC`) contains a proper subset of `IC's` inference vars (`IC_IV`). In other words there will be at least one inference variable `T` that belongs to `IC_IV` which doesn't belong to the minimized context inference variable's set (`MIC_IV`). MIC can be propagated to another inference context (AIC) as part of the type inference algo. If at any moment a type containing `T` needs, for example, to be instantiated `AIC` won't be able to perform that operation as it lacks the information to realize that `T` is a free variable it was supposed to know about hadn't a minimization step been performed. This PR is proposing to fix this issue without reducing the universe of inference contexts we apply the minimization algo to.
>> 
>> We are basically proposing to add a field to class InferenceContext which will be supplementary inference context that can be used to look for additional information if necessary. Also when registering a free type listener to an inference context, it could be that it won't be registered in the inference context the client could expect but in a supplementary inference context if it is found out that it contains and instantiation to all the free variables related to the listener. See also some comments inlined in the code,
>> 
>> TIA
>
> Vicente Romero has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains four additional commits since the last revision:
> 
>  - Merge branch 'master' into JDK-8325859
>  - make visitor static
>  - updating doc
>  - 8325859: potential information loss during type inference

Btw, I've looked at the issue in JBS again, and simplified it a bit:


class Generics {
       public static void main(String... args) {
            Integer i = null;
            runT(() -> supplyNull(i));
        }

        static <R, X> R supplyNull(X... varargs) {
            System.out.println(varargs.getClass().getComponentType());
            return null;
        }
              
        static <T> void runT(Runnable runnable) { runnable.run(); }
}


(I've removed a call to `Integer::valueOf` that was not necessary to reproduce the issue -- but was adding some extra noise).

Note that here the calls to `supplyNull` and `runT` are completely independent. That's because the call to `supplyNull` occurs in a void-compatible lambda (the target of the lambda is `Runnable`). As such, it is not possible for inference of the nested call to `supplyNull` to have any effect on the inference of the `runT` call.

However, when debugging, I see that when we call `checkMethod` for `supplyNull`, we attempt to propagate its inference context outwards. This is done even though the `resultInfo` says that the expected type is `void` (where `void` is the return type of the function descriptor).

In other words, it seems to me that `Infer::shouldPropagate` is buggy: it returns true if all the conditions below are satisfied:
* the enclosing method is a generic method
* the return type of the current method contains some inference vars
* no eager resolution is required

But we never check what the target-type of the current method actually is. Normally, the target will be some non-void type -- so when we propagate, we will need to check compatibility again, which will fix up all the types. But in this case (which is a corner case due to void-compatible lambdas), the target happens to be `void` -- meaning inference will never be "completed".

See the implementation of `Attr.ExpressionLambdaReturnContext::compatible`:


           @Override
            public boolean compatible(Type found, Type req, Warner warn) {
                //a void return is compatible with an expression statement lambda
                if (req.hasTag(VOID)) {
                    expStmtExpected = true;
                    return TreeInfo.isExpressionStatement(expr);
                } else {
                    return super.compatible(found, req, warn);
                }
            }


In other words, if the expected type is `void` (as in this case), `super.compatible` will not be called -- which means the deferred type for the nested generic method call will remain uninferred.

Adding an extra check to `Infer::shouldPropagate` to _not_ propagate when the target happens to be `void` seems to fix the issue in JBS. (but, I have not ran any other test).

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

PR Comment: https://git.openjdk.org/jdk/pull/25011#issuecomment-2883767203