This bug relates to the "potentially ambiguous overload" warning which is
enabled by `-Xlint:overloads`.
The warning detects certain ambiguities that can cause problems for lambdas.
For example, consider the interface `Spliterator.OfInt`, which declares these
two methods:
void forEachRemaining(Consumer<? super Integer> action);
void forEachRemaining(IntConsumer action);
Both methods have the same name, same number of parameters, and take a lambda
with the same "shape" in the same argument position. This causes an ambiguity
in any code that wants to do this:
spliterator.forEachRemaining(x -> { ... });
That code won't compile; instead, you'll get this error:
Ambiguity.java:4: error: reference to forEachRemaining is ambiguous
spliterator.forEachRemaining(x -> { });
^
both method forEachRemaining(IntConsumer) in OfInt and method
forEachRemaining(Consumer<? super Integer>) in OfInt match
The problem reported by the bug is that the warning fails to detect ambiguities
which are created purely by inheritance, for example:
interface ConsumerOfInteger {
void foo(Consumer<Integer> c);
}
interface IntegerConsumer {
void foo(IntConsumer c);
}
// We should get a warning here...
interface Test extends ConsumerOfInteger, IntegerConsumer {
}
The cause of the bug is that ambiguities are detected on a per-method basis, by
checking whether a method is part of an ambiguity pair when we visit that
method. So if the methods in an ambiguity pair are inherited from two distinct
supertypes, we'll miss the ambiguity.
To fix the problem, we need to look for ambiguities on a per-class level,
checking all pairs of methods. However, it's not that simple - we only want to
"blame" a class when that class itself, and not some supertype, is responsible
for creating the ambiguity. For example, any interface extending
`Spliterator.OfInt` will automatically inherit the two ambiguities mentioned
above, but these are not the interface's fault so to speak so no warning should
be generated. Making things more complicated is the fact that methods can be
overridden and declared in generic classes so they only conflict in some
subtypes, etc.
So we generate the warning when there are two methods m1 and m2 in a class C
such that:
* m1 and m2 consitiute a "potentially ambiguous overload" (using the same
definition as before)
* There is no direct supertype T of C such that m1 and m2, or some methods they
override, both exist in T and constitute a "potentially ambiguous overload" as
members of T
* We haven't already generated a warning for either m1 or m2 in class C
If either method is declared in C, we locate the warning there, but when both
methods are inherited, there's no method declaration to point at so the warning
is instead located at the class declaration.
I noticed a couple of other minor bugs; these are also being fixed here:
(1) For inherited methods, the method signatures were being reported as they
are declared, rather than in the context of the class being visited. As a
result, when a methods is inherited from a generic supertype, the ambiguity is
less clear. Here's an example:
interface Upper<T> {
void foo(T c);
}
interface Lower extends Upper<IntConsumer> {
void foo(Consumer<Integer> c);
}
Currently, the error is reported as:
warning: [overloads] foo(Consumer<Integer>) in Lower is potentially ambiguous
with foo(T) in Upper
Reporting the method signatures in the context of the class being visited makes
the ambiguity clearer:
warning: [overloads] foo(Consumer<Integer>) in Lower is potentially ambiguous
with foo(IntConsumer) in Upper
(2) When a method is identified as part of an ambiguous pair, we were setting a
`POTENTIALLY_AMBIGUOUS` flag on it. This caused it to be forever excluded from
future warnings. For methods that are declared in the class we're visiting,
this makes sense, but it doesn't make sense for inherited methods, because it
disqualifies them from participating in the analysis of any other class that
also inherits them.
As a result, for a class like the one below, the compiler was only generating
one warning instead of three:
public interface SuperIface {
void foo(Consumer<Integer> c);
}
public interface I1 extends SuperIface {
void foo(IntConsumer c); // warning was generated here
}
public interface I2 extends SuperIface {
void foo(IntConsumer c); // no warning was generated here
}
public interface I3 extends SuperIface {
void foo(IntConsumer c); // no warning was generated here
}
With this patch the `POTENTIALLY_AMBIGUOUS` flag is no longer needed. I wasn't
sure whether to renumber all the subsequent flags, or just leave an empty
placeholder, so I chose the latter.
Finally, this fix uncovers new warnings in `java.base` and `java.desktop`, so
these are now suppressed in the patch.
-------------
Commit messages:
- Fix incomplete detection of potentially ambiguous method declarations.
Changes: https://git.openjdk.org/jdk/pull/12645/files
Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=12645&range=00
Issue: https://bugs.openjdk.org/browse/JDK-8026369
Stats: 356 lines in 18 files changed: 280 ins; 36 del; 40 mod
Patch: https://git.openjdk.org/jdk/pull/12645.diff
Fetch: git fetch https://git.openjdk.org/jdk pull/12645/head:pull/12645
PR: https://git.openjdk.org/jdk/pull/12645
