RFR: 8268312: Compilation error with nested generic functional interface [v2]

[Maurizio Cimadamore]

On Wed, 29 Sep 2021 06:16:22 GMT, Vicente Romero <vromero at openjdk.org> wrote:

>> Please review this PR, which is my proposal to fix an existing regression. This code:
>> 
>> 
>> import java.util.Optional;
>> 
>> class App {
>>     public static void main(String[] args) {
>>         Optional.of("").map(outer -> {
>>             Optional.of("")
>>                 .map(inner -> returnGeneric(outer))
>>                 .ifPresent(String::toString);
>>             return "";
>>         });
>>     }
>> 
>>     private static <RG> RG returnGeneric(RG generic) {
>>         return generic;
>>     }
>> }
>> 
>> is not accepted by javac but if the user passes the `-Xdiags:verbose` option then the code compiles. I tracked down the reason for this puzzling difference and I found that it is due to our diagnostic rewriters which can generate more detailed positions for error messages but in cases like the one above can trick the compiler to generate an error message too early. The code deciding if an error message should be deferred or not, depending on the position, is at `DeferredDiagnosticHandler::report`. We decide to do the rewriting if we are in diagnostics compact mode, this is why the error doesn't occur with the `-Xdiags:verbose` option. This fix will made some diagnostics to appear at a slightly different position, but won't make the compiler reject correct code. Comments?
>> 
>> TIA
>
> Vicente Romero has updated the pull request incrementally with one additional commit since the last revision:
> 
>   addressing review comments

src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Resolve.java line 4138:

> 4136: 
> 4137:             Pair<Symbol, JCDiagnostic> c = errCandidate();
> 4138:             if (compactMethodDiags) {

I think what 'm really after here is to drop this branch - e.g. always create the "non compressed" diagnostic (e.g. cannot.apply) with the right position.

But, before returning that, attach a rewriter to it e.g.


            Symbol ws = c.fst.asMemberOf(site, types);
            return diags.create(dkind, log.currentSource(), pos,
                      "cant.apply.symbol",
                      kindName(ws),
                      ws.name == names.init ? ws.owner.name : ws.name,
                      methodArguments(ws.type.getParameterTypes()),
                      methodArguments(argtypes),
                      kindName(ws.owner),
                      ws.owner.type,
                      c.snd).withRewriter(() -> MethodResolutionDiagHelper.rewrite(diags, pos, log.currentSource(), dkind, c.snd));

src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Resolve.java line 4788:

> 4786:                         diag -> diags.create(preferredKind, preferredSource, pos, "prob.found.req", cause) :
> 4787:                         null;
> 4788:                 return diags.create(preferredKind, preferredSource, preferredPos,

I guess here we should return the diagnostic unmodified? E.g. you are still returning a rewritten diagnostic here, the only thing that changes is the position (which is what avoids the bug). What I'm hoping is that we can obtain a new diagnostic (with a rewriter) from an existing diagnostic. See my other comment.

src/jdk.compiler/share/classes/com/sun/tools/javac/main/JavaCompiler.java line 1716:

> 1714:         chk.reportDeferredDiagnostics();
> 1715:         preview.reportDeferredDiagnostics();
> 1716:         if (log.compressedOutput) {

This can be kept, no?

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

PR: https://git.openjdk.java.net/jdk/pull/5586