Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8268312: Compilation error with nested generic functional interface #5586

Closed
Closed
@@ -68,6 +68,7 @@
import java.util.function.Consumer;
import java.util.function.Function;
import java.util.function.Predicate;
import java.util.function.UnaryOperator;
import java.util.stream.Stream;
import java.util.stream.StreamSupport;

@@ -4135,8 +4136,7 @@ JCDiagnostic getDiagnostic(JCDiagnostic.DiagnosticType dkind,

Pair<Symbol, JCDiagnostic> c = errCandidate();
if (compactMethodDiags) {
Copy link
Contributor

@mcimadamore mcimadamore Sep 29, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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));

Copy link
Contributor

@mcimadamore mcimadamore Jul 11, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe the issue is here - the old code made the rewriting dependent on the value of compactMethodDiags, which in turns looked at which diagnostic formatter was installed. I think we should either move this check in Log (when we decide if we need to rewrite), or don't attach a rewriter if the compactMethodDiags is not set. I think it would be preferable to keep existing diagnostics as they are (and maybe cleanup the behavior of compactMethodDiags later, as an orthogonal fix).

Copy link
Contributor Author

@vicente-romero-oracle vicente-romero-oracle Jul 11, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep I think this is the reason, will upload an updated iteration

JCDiagnostic simpleDiag =
MethodResolutionDiagHelper.rewrite(diags, pos, log.currentSource(), dkind, c.snd);
JCDiagnostic simpleDiag = MethodResolutionDiagHelper.rewrite(diags, pos, log.currentSource(), dkind, c.snd);
if (simpleDiag != null) {
return simpleDiag;
}
@@ -4199,13 +4199,10 @@ JCDiagnostic getDiagnostic(JCDiagnostic.DiagnosticType dkind,
if (filteredCandidates.isEmpty()) {
filteredCandidates = candidatesMap;
}
boolean truncatedDiag = candidatesMap.size() != filteredCandidates.size();
if (filteredCandidates.size() > 1) {
JCDiagnostic err = diags.create(dkind,
null,
truncatedDiag ?
EnumSet.of(DiagnosticFlag.COMPRESSED) :
EnumSet.noneOf(DiagnosticFlag.class),
EnumSet.noneOf(DiagnosticFlag.class),
log.currentSource(),
pos,
"cant.apply.symbols",
@@ -4224,9 +4221,6 @@ protected Pair<Symbol, JCDiagnostic> errCandidate() {
}
}.getDiagnostic(dkind, pos,
location, site, name, argtypes, typeargtypes);
if (truncatedDiag) {
d.setFlag(DiagnosticFlag.COMPRESSED);
}
return d;
} else {
return new SymbolNotFoundError(ABSENT_MTH).getDiagnostic(dkind, pos,
@@ -4787,12 +4781,12 @@ public JCDiagnostic rewriteDiagnostic(JCDiagnostic.Factory diags,
DiagnosticPosition preferredPos, DiagnosticSource preferredSource,
DiagnosticType preferredKind, JCDiagnostic d) {
JCDiagnostic cause = (JCDiagnostic)d.getArgs()[causeIndex];
DiagnosticPosition pos = d.getDiagnosticPosition();
if (pos == null) {
pos = preferredPos;
}
return diags.create(preferredKind, preferredSource, pos,
"prob.found.req", cause);
final DiagnosticPosition pos = d.getDiagnosticPosition();
UnaryOperator<JCDiagnostic> rewriter = pos != null ?
diag -> diags.create(preferredKind, preferredSource, pos, "prob.found.req", cause) :
null;
return diags.create(preferredKind, preferredSource, preferredPos,
Copy link
Contributor

@mcimadamore mcimadamore Sep 29, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

"prob.found.req", rewriter, cause);
}
}

@@ -4854,7 +4848,6 @@ static JCDiagnostic rewrite(JCDiagnostic.Factory diags, DiagnosticPosition pos,
if (_entry.getKey().matches(d)) {
JCDiagnostic simpleDiag =
_entry.getValue().rewriteDiagnostic(diags, pos, source, dkind, d);
simpleDiag.setFlag(DiagnosticFlag.COMPRESSED);
return simpleDiag;
}
}
@@ -1713,9 +1713,6 @@ public void reportDeferredDiagnostics() {
}
chk.reportDeferredDiagnostics();
preview.reportDeferredDiagnostics();
if (log.compressedOutput) {
Copy link
Contributor

@mcimadamore mcimadamore Sep 29, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be kept, no?

log.mandatoryNote(null, Notes.CompressedDiags);
}
}

public void enterDone() {
@@ -1582,9 +1582,6 @@ compiler.warn.file.from.future=\
## The following string will appear before all messages keyed as:
## "compiler.note".

compiler.note.compressed.diags=\
Some messages have been simplified; recompile with -Xdiags:verbose to get full output

# 0: boolean, 1: symbol
compiler.note.lambda.stat=\
Translating lambda expression\n\
@@ -26,6 +26,7 @@
package com.sun.tools.javac.util;

import java.util.EnumSet;
import java.util.function.UnaryOperator;
import java.util.Locale;
import java.util.Set;
import java.util.stream.Stream;
@@ -243,6 +244,21 @@ public JCDiagnostic create(
return create(null, EnumSet.noneOf(DiagnosticFlag.class), source, pos, DiagnosticInfo.of(kind, prefix, key, args));
}

/**
* Create a new diagnostic of the given kind, which is not mandatory and which has
* no lint category.
* @param kind The diagnostic kind
* @param source The source of the compilation unit, if any, in which to report the message.
* @param pos The source position at which to report the message.
* @param key The key for the localized message.
* @param rewriter A rewriter function used if this diagnostic needs to be rewritten
* @param args Fields of the message.
*/
public JCDiagnostic create(
DiagnosticType kind, DiagnosticSource source, DiagnosticPosition pos, String key, UnaryOperator<JCDiagnostic> rewriter, Object... args) {
return create(null, EnumSet.noneOf(DiagnosticFlag.class), source, pos, DiagnosticInfo.of(kind, prefix, key, args), rewriter);
}

/**
* Create a new diagnostic of the given kind, which is not mandatory and which has
* no lint category.
@@ -282,6 +298,11 @@ public JCDiagnostic create(
LintCategory lc, Set<DiagnosticFlag> flags, DiagnosticSource source, DiagnosticPosition pos, DiagnosticInfo diagnosticInfo) {
return new JCDiagnostic(formatter, normalize(diagnosticInfo), lc, flags, source, pos);
}

public JCDiagnostic create(
LintCategory lc, Set<DiagnosticFlag> flags, DiagnosticSource source, DiagnosticPosition pos, DiagnosticInfo diagnosticInfo, UnaryOperator<JCDiagnostic> rewriter) {
return new JCDiagnostic(formatter, normalize(diagnosticInfo), lc, flags, source, pos, rewriter);
}
//where
DiagnosticInfo normalize(DiagnosticInfo diagnosticInfo) {
//replace all nested FragmentKey with full-blown JCDiagnostic objects
@@ -428,7 +449,6 @@ public enum DiagnosticFlag {
SYNTAX,
RECOVERABLE,
NON_DEFERRABLE,
COMPRESSED,
/** Flag for diagnostics that were reported through API methods.
*/
API,
@@ -446,6 +466,8 @@ public enum DiagnosticFlag {
/** source line position (set lazily) */
private SourcePosition sourcePosition;

private final UnaryOperator<JCDiagnostic> rewriter;

/**
* This class is used to defer the line/column position fetch logic after diagnostic construction.
*/
@@ -596,6 +618,25 @@ protected JCDiagnostic(DiagnosticFormatter<JCDiagnostic> formatter,
Set<DiagnosticFlag> flags,
DiagnosticSource source,
DiagnosticPosition pos) {
this(formatter, diagnosticInfo, lc, flags, source, pos, null);
}

/**
* Create a diagnostic object.
* @param formatter the formatter to use for the diagnostic
* @param diagnosticInfo the diagnostic key
* @param lc the lint category for the diagnostic
* @param source the name of the source file, or null if none.
* @param pos the character offset within the source file, if given.
* @param rewriter the rewriter function used if this diagnostic needs to be rewritten
*/
protected JCDiagnostic(DiagnosticFormatter<JCDiagnostic> formatter,
DiagnosticInfo diagnosticInfo,
LintCategory lc,
Set<DiagnosticFlag> flags,
DiagnosticSource source,
DiagnosticPosition pos,
UnaryOperator<JCDiagnostic> rewriter) {
if (source == null && pos != null && pos.getPreferredPosition() != Position.NOPOS)
throw new IllegalArgumentException();

@@ -605,6 +646,7 @@ protected JCDiagnostic(DiagnosticFormatter<JCDiagnostic> formatter,
this.flags = flags;
this.source = source;
this.position = pos;
this.rewriter = rewriter;
}

/**
@@ -807,6 +849,14 @@ public boolean isFlagSet(DiagnosticFlag flag) {
return flags.contains(flag);
}

boolean hasRewriter() {
return rewriter != null;
}

JCDiagnostic rewrite() {
return rewriter.apply(this);
}

public static class MultilineDiagnostic extends JCDiagnostic {

private final List<JCDiagnostic> subdiagnostics;
@@ -28,7 +28,6 @@
import java.io.*;
import java.util.Arrays;
import java.util.EnumMap;
import java.util.EnumSet;
import java.util.HashSet;
import java.util.Map;
import java.util.Queue;
@@ -214,11 +213,6 @@ public enum WriterKind { NOTICE, WARNING, ERROR, STDOUT, STDERR }
*/
public Set<String> expectDiagKeys;

/**
* Set to true if a compressed diagnostic is reported
*/
public boolean compressedOutput;
Copy link
Contributor

@mcimadamore mcimadamore Jul 7, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see that this patch drops the option for compressing diagnostic - this seems to go a step too far, and I'm not sure this is what we want, at least in this patch. I believe there's an orthogonal discussion to be had there. I think the flag should stay as is for now (and control whether diagnostics should be rewritten at all in Log).


/**
* JavacMessages object used for localization.
*/
@@ -671,6 +665,10 @@ public void report(JCDiagnostic diagnostic) {
if (expectDiagKeys != null)
expectDiagKeys.remove(diagnostic.getCode());

if (diagnostic.hasRewriter()) {
diagnostic = diagnostic.rewrite();
}

switch (diagnostic.getType()) {
case FRAGMENT:
throw new IllegalArgumentException();
@@ -707,9 +705,6 @@ public void report(JCDiagnostic diagnostic) {
}
break;
}
if (diagnostic.isFlagSet(JCDiagnostic.DiagnosticFlag.COMPRESSED)) {
Copy link
Contributor

@mcimadamore mcimadamore Sep 29, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Who sets this now?

compressedOutput = true;
}
}
}

@@ -2,5 +2,4 @@ T8067883.java:15:17: compiler.err.prob.found.req: (compiler.misc.inconvertible.t
T8067883.java:16:9: compiler.err.cant.apply.symbol: kindname.method, m, java.util.List<Z>,java.util.List<java.lang.String>, int,java.util.List<java.lang.Integer>, kindname.class, T8067883, (compiler.misc.infer.no.conforming.assignment.exists: Z, (compiler.misc.inconvertible.types: int, java.util.List<Z>))
T8067883.java:20:25: compiler.err.prob.found.req: (compiler.misc.inconvertible.types: java.util.List<java.lang.Integer>, java.util.List<java.lang.String>)
T8067883.java:21:9: compiler.err.cant.apply.diamond.1: (compiler.misc.diamond: T8067883.Box), (compiler.misc.infer.no.conforming.assignment.exists: X, (compiler.misc.inconvertible.types: int, java.util.List<X>))
- compiler.note.compressed.diags
4 errors
@@ -2,5 +2,4 @@ T8012003a.java:19:12: compiler.err.prob.found.req: (compiler.misc.inconvertible.
T8012003a.java:20:20: compiler.err.prob.found.req: (compiler.misc.incompatible.type.in.conditional: (compiler.misc.inconvertible.types: java.lang.String, java.lang.Integer))
T8012003a.java:21:12: compiler.err.prob.found.req: (compiler.misc.inconvertible.types: java.lang.String, java.lang.Integer)
T8012003a.java:22:9: compiler.err.cant.apply.symbols: kindname.method, m3, char,{(compiler.misc.inapplicable.method: kindname.method, T8012003a, m3(java.lang.Integer), (compiler.misc.no.conforming.assignment.exists: (compiler.misc.inconvertible.types: char, java.lang.Integer))),(compiler.misc.inapplicable.method: kindname.method, T8012003a, m3(java.lang.String), (compiler.misc.no.conforming.assignment.exists: (compiler.misc.inconvertible.types: char, java.lang.String)))}
- compiler.note.compressed.diags
4 errors
@@ -4,5 +4,4 @@ T8012003b.java:32:22: compiler.err.prob.found.req: (compiler.misc.incompatible.r
T8012003b.java:33:12: compiler.err.prob.found.req: (compiler.misc.invalid.mref: kindname.method, (compiler.misc.prob.found.req: (compiler.misc.inconvertible.types: java.lang.Integer, java.lang.String)))
T8012003b.java:34:12: compiler.err.prob.found.req: (compiler.misc.incompatible.ret.type.in.mref: (compiler.misc.inconvertible.types: java.lang.String, java.lang.Integer))
T8012003b.java:35:12: compiler.err.invalid.mref: kindname.method, (compiler.misc.cant.resolve.location.args: kindname.method, k, , java.lang.String, (compiler.misc.location: kindname.class, T8012003b, null))
- compiler.note.compressed.diags
6 errors
@@ -1,4 +1,3 @@
T8020286.java:12:13: compiler.err.prob.found.req: (compiler.misc.inconvertible.types: int, java.lang.String)
T8020286.java:13:10: compiler.err.prob.found.req: (compiler.misc.inconvertible.types: int, java.lang.String)
- compiler.note.compressed.diags
2 errors
@@ -1,3 +1,2 @@
DiagnosticRewriterTest.java:12:15: compiler.err.prob.found.req: (compiler.misc.possible.loss.of.precision: long, int)
- compiler.note.compressed.diags
1 error
@@ -1,4 +1,3 @@
DiagnosticRewriterTest2.java:15:15: compiler.err.prob.found.req: (compiler.misc.possible.loss.of.precision: long, int)
DiagnosticRewriterTest2.java:16:13: compiler.err.cant.apply.symbol: kindname.method, m, int, compiler.misc.no.args, kindname.class, DiagnosticRewriterTest2, (compiler.misc.arg.length.mismatch)
- compiler.note.compressed.diags
2 errors
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2013, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.
vicente-romero-oracle marked this conversation as resolved.
Show resolved Hide resolved
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -21,19 +21,24 @@
* questions.
*/

// key: compiler.err.prob.found.req
// key: compiler.misc.inconvertible.types
// key: compiler.note.compressed.diags
// key: compiler.note.note
// key: compiler.misc.count.error
// key: compiler.err.error
// run: backdoor

class CompressedDiags {
/*
* @test
* @bug 8268312
* @summary Compilation error with nested generic functional interface
* @compile DiagnosticRewriterTest3.java
*/

void m(String s) { }
class DiagnosticRewriterTest3 {
void m() {
Optional.of("").map(outer -> {
Optional.of("")
.map(inner -> returnGeneric(outer))
.ifPresent(String::toString);
return "";
});
}

void test() {
m(1);
<T> T returnGeneric(T generic) {
return generic;
}
}