On Sun, 19 Feb 2023 23:52:52 GMT, Archie L. Cobbs <d...@openjdk.org> wrote:
> 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. In the `AWTEventMulticaster` class(es), which interfaces are causing the ambiguity? ------------- PR: https://git.openjdk.org/jdk/pull/12645