From 1e3c9fd67efd8428f702c7a3e26ac2b60e0fe618 Mon Sep 17 00:00:00 2001 From: "Archie L. Cobbs" Date: Tue, 28 Feb 2023 03:33:14 +0000 Subject: [PATCH] 8026369: javac potentially ambiguous overload warning needs an improved scheme Reviewed-by: vromero --- .../java/util/LongSummaryStatistics.java | 1 + .../classes/java/util/PrimitiveIterator.java | 3 + .../share/classes/java/util/Spliterator.java | 3 + .../share/classes/java/util/Spliterators.java | 3 + .../share/classes/java/util/stream/Node.java | 3 + .../share/classes/java/util/stream/Nodes.java | 9 + .../share/classes/java/util/stream/Sink.java | 3 + .../java/util/stream/SpinedBuffer.java | 6 + .../java/util/stream/StreamSpliterators.java | 9 + .../classes/java/util/stream/Streams.java | 3 + .../com/sun/tools/javac/code/Flags.java | 5 +- .../com/sun/tools/javac/comp/Attr.java | 1 + .../com/sun/tools/javac/comp/Check.java | 304 ++++++++++++++---- .../tools/javac/8230827/T8230827.out | 10 +- .../PotentiallyAmbiguousWarningTest.java | 72 ++++- .../PotentiallyAmbiguousWarningTest.out | 11 +- 16 files changed, 364 insertions(+), 82 deletions(-) diff --git a/src/java.base/share/classes/java/util/LongSummaryStatistics.java b/src/java.base/share/classes/java/util/LongSummaryStatistics.java index ec8d9441c79..e84d70acf86 100644 --- a/src/java.base/share/classes/java/util/LongSummaryStatistics.java +++ b/src/java.base/share/classes/java/util/LongSummaryStatistics.java @@ -63,6 +63,7 @@ import java.util.stream.Collector; *

This implementation does not check for overflow of the count or the sum. * @since 1.8 */ +@SuppressWarnings("overloads") public class LongSummaryStatistics implements LongConsumer, IntConsumer { private long count; private long sum; diff --git a/src/java.base/share/classes/java/util/PrimitiveIterator.java b/src/java.base/share/classes/java/util/PrimitiveIterator.java index e6fd3a57dbc..b0e07ef6e5a 100644 --- a/src/java.base/share/classes/java/util/PrimitiveIterator.java +++ b/src/java.base/share/classes/java/util/PrimitiveIterator.java @@ -91,6 +91,7 @@ public interface PrimitiveIterator extends Iterator { * An Iterator specialized for {@code int} values. * @since 1.8 */ + @SuppressWarnings("overloads") public static interface OfInt extends PrimitiveIterator { /** @@ -158,6 +159,7 @@ public interface PrimitiveIterator extends Iterator { * An Iterator specialized for {@code long} values. * @since 1.8 */ + @SuppressWarnings("overloads") public static interface OfLong extends PrimitiveIterator { /** @@ -224,6 +226,7 @@ public interface PrimitiveIterator extends Iterator { * An Iterator specialized for {@code double} values. * @since 1.8 */ + @SuppressWarnings("overloads") public static interface OfDouble extends PrimitiveIterator { /** diff --git a/src/java.base/share/classes/java/util/Spliterator.java b/src/java.base/share/classes/java/util/Spliterator.java index 0f49a183f12..a4f95e42ddf 100644 --- a/src/java.base/share/classes/java/util/Spliterator.java +++ b/src/java.base/share/classes/java/util/Spliterator.java @@ -659,6 +659,7 @@ public interface Spliterator { * A Spliterator specialized for {@code int} values. * @since 1.8 */ + @SuppressWarnings("overloads") public interface OfInt extends OfPrimitive { @Override @@ -723,6 +724,7 @@ public interface Spliterator { * A Spliterator specialized for {@code long} values. * @since 1.8 */ + @SuppressWarnings("overloads") public interface OfLong extends OfPrimitive { @Override @@ -787,6 +789,7 @@ public interface Spliterator { * A Spliterator specialized for {@code double} values. * @since 1.8 */ + @SuppressWarnings("overloads") public interface OfDouble extends OfPrimitive { @Override diff --git a/src/java.base/share/classes/java/util/Spliterators.java b/src/java.base/share/classes/java/util/Spliterators.java index 48633e4caf0..b0d1f3a9124 100644 --- a/src/java.base/share/classes/java/util/Spliterators.java +++ b/src/java.base/share/classes/java/util/Spliterators.java @@ -908,18 +908,21 @@ public final class Spliterators { OfRef() { } } + @SuppressWarnings("overloads") private static final class OfInt extends EmptySpliterator implements Spliterator.OfInt { OfInt() { } } + @SuppressWarnings("overloads") private static final class OfLong extends EmptySpliterator implements Spliterator.OfLong { OfLong() { } } + @SuppressWarnings("overloads") private static final class OfDouble extends EmptySpliterator implements Spliterator.OfDouble { diff --git a/src/java.base/share/classes/java/util/stream/Node.java b/src/java.base/share/classes/java/util/stream/Node.java index 7c4dda8adaf..0f88850cfc5 100644 --- a/src/java.base/share/classes/java/util/stream/Node.java +++ b/src/java.base/share/classes/java/util/stream/Node.java @@ -314,6 +314,7 @@ interface Node { /** * Specialized {@code Node} for int elements */ + @SuppressWarnings("overloads") interface OfInt extends OfPrimitive { /** @@ -391,6 +392,7 @@ interface Node { /** * Specialized {@code Node} for long elements */ + @SuppressWarnings("overloads") interface OfLong extends OfPrimitive { /** @@ -468,6 +470,7 @@ interface Node { /** * Specialized {@code Node} for double elements */ + @SuppressWarnings("overloads") interface OfDouble extends OfPrimitive { /** diff --git a/src/java.base/share/classes/java/util/stream/Nodes.java b/src/java.base/share/classes/java/util/stream/Nodes.java index ffc5ae9cb10..219ebda9628 100644 --- a/src/java.base/share/classes/java/util/stream/Nodes.java +++ b/src/java.base/share/classes/java/util/stream/Nodes.java @@ -582,6 +582,7 @@ final class Nodes { } } + @SuppressWarnings("overloads") private static final class OfInt extends EmptyNode implements Node.OfInt { @@ -599,6 +600,7 @@ final class Nodes { } } + @SuppressWarnings("overloads") private static final class OfLong extends EmptyNode implements Node.OfLong { @@ -616,6 +618,7 @@ final class Nodes { } } + @SuppressWarnings("overloads") private static final class OfDouble extends EmptyNode implements Node.OfDouble { @@ -880,6 +883,7 @@ final class Nodes { } } + @SuppressWarnings("overloads") static final class OfInt extends ConcNode.OfPrimitive implements Node.OfInt { @@ -894,6 +898,7 @@ final class Nodes { } } + @SuppressWarnings("overloads") static final class OfLong extends ConcNode.OfPrimitive implements Node.OfLong { @@ -908,6 +913,7 @@ final class Nodes { } } + @SuppressWarnings("overloads") static final class OfDouble extends ConcNode.OfPrimitive implements Node.OfDouble { @@ -1160,6 +1166,7 @@ final class Nodes { } } + @SuppressWarnings("overloads") private static final class OfInt extends OfPrimitive implements Spliterator.OfInt { @@ -1169,6 +1176,7 @@ final class Nodes { } } + @SuppressWarnings("overloads") private static final class OfLong extends OfPrimitive implements Spliterator.OfLong { @@ -1178,6 +1186,7 @@ final class Nodes { } } + @SuppressWarnings("overloads") private static final class OfDouble extends OfPrimitive implements Spliterator.OfDouble { diff --git a/src/java.base/share/classes/java/util/stream/Sink.java b/src/java.base/share/classes/java/util/stream/Sink.java index a0fe5b31721..34d09fd6611 100644 --- a/src/java.base/share/classes/java/util/stream/Sink.java +++ b/src/java.base/share/classes/java/util/stream/Sink.java @@ -186,6 +186,7 @@ interface Sink extends Consumer { * {@code accept(int)}, and wires {@code accept(Integer)} to bridge to * {@code accept(int)}. */ + @SuppressWarnings("overloads") interface OfInt extends Sink, IntConsumer { @Override void accept(int value); @@ -203,6 +204,7 @@ interface Sink extends Consumer { * {@code accept(long)}, and wires {@code accept(Long)} to bridge to * {@code accept(long)}. */ + @SuppressWarnings("overloads") interface OfLong extends Sink, LongConsumer { @Override void accept(long value); @@ -220,6 +222,7 @@ interface Sink extends Consumer { * {@code accept(double)}, and wires {@code accept(Double)} to bridge to * {@code accept(double)}. */ + @SuppressWarnings("overloads") interface OfDouble extends Sink, DoubleConsumer { @Override void accept(double value); diff --git a/src/java.base/share/classes/java/util/stream/SpinedBuffer.java b/src/java.base/share/classes/java/util/stream/SpinedBuffer.java index 163692cf858..98bbc040a7b 100644 --- a/src/java.base/share/classes/java/util/stream/SpinedBuffer.java +++ b/src/java.base/share/classes/java/util/stream/SpinedBuffer.java @@ -720,6 +720,7 @@ class SpinedBuffer /** * An ordered collection of {@code int} values. */ + @SuppressWarnings("overloads") static class OfInt extends SpinedBuffer.OfPrimitive implements IntConsumer { OfInt() { } @@ -785,6 +786,7 @@ class SpinedBuffer } public Spliterator.OfInt spliterator() { + @SuppressWarnings("overloads") class Splitr extends BaseSpliterator implements Spliterator.OfInt { Splitr(int firstSpineIndex, int lastSpineIndex, @@ -833,6 +835,7 @@ class SpinedBuffer /** * An ordered collection of {@code long} values. */ + @SuppressWarnings("overloads") static class OfLong extends SpinedBuffer.OfPrimitive implements LongConsumer { OfLong() { } @@ -899,6 +902,7 @@ class SpinedBuffer public Spliterator.OfLong spliterator() { + @SuppressWarnings("overloads") class Splitr extends BaseSpliterator implements Spliterator.OfLong { Splitr(int firstSpineIndex, int lastSpineIndex, @@ -947,6 +951,7 @@ class SpinedBuffer /** * An ordered collection of {@code double} values. */ + @SuppressWarnings("overloads") static class OfDouble extends SpinedBuffer.OfPrimitive implements DoubleConsumer { @@ -1013,6 +1018,7 @@ class SpinedBuffer } public Spliterator.OfDouble spliterator() { + @SuppressWarnings("overloads") class Splitr extends BaseSpliterator implements Spliterator.OfDouble { Splitr(int firstSpineIndex, int lastSpineIndex, diff --git a/src/java.base/share/classes/java/util/stream/StreamSpliterators.java b/src/java.base/share/classes/java/util/stream/StreamSpliterators.java index 01ba72d1950..d2f558c2821 100644 --- a/src/java.base/share/classes/java/util/stream/StreamSpliterators.java +++ b/src/java.base/share/classes/java/util/stream/StreamSpliterators.java @@ -572,6 +572,7 @@ class StreamSpliterators { } } + @SuppressWarnings("overloads") static final class OfInt extends OfPrimitive implements Spliterator.OfInt { @@ -581,6 +582,7 @@ class StreamSpliterators { } } + @SuppressWarnings("overloads") static final class OfLong extends OfPrimitive implements Spliterator.OfLong { @@ -590,6 +592,7 @@ class StreamSpliterators { } } + @SuppressWarnings("overloads") static final class OfDouble extends OfPrimitive implements Spliterator.OfDouble { @@ -815,6 +818,7 @@ class StreamSpliterators { protected abstract T_CONS emptyConsumer(); } + @SuppressWarnings("overloads") static final class OfInt extends OfPrimitive implements Spliterator.OfInt { OfInt(Spliterator.OfInt s, long sliceOrigin, long sliceFence) { @@ -839,6 +843,7 @@ class StreamSpliterators { } } + @SuppressWarnings("overloads") static final class OfLong extends OfPrimitive implements Spliterator.OfLong { OfLong(Spliterator.OfLong s, long sliceOrigin, long sliceFence) { @@ -863,6 +868,7 @@ class StreamSpliterators { } } + @SuppressWarnings("overloads") static final class OfDouble extends OfPrimitive implements Spliterator.OfDouble { OfDouble(Spliterator.OfDouble s, long sliceOrigin, long sliceFence) { @@ -1128,6 +1134,7 @@ class StreamSpliterators { protected abstract T_BUFF bufferCreate(int initialCapacity); } + @SuppressWarnings("overloads") static final class OfInt extends OfPrimitive implements Spliterator.OfInt, IntConsumer { @@ -1163,6 +1170,7 @@ class StreamSpliterators { } } + @SuppressWarnings("overloads") static final class OfLong extends OfPrimitive implements Spliterator.OfLong, LongConsumer { @@ -1198,6 +1206,7 @@ class StreamSpliterators { } } + @SuppressWarnings("overloads") static final class OfDouble extends OfPrimitive implements Spliterator.OfDouble, DoubleConsumer { diff --git a/src/java.base/share/classes/java/util/stream/Streams.java b/src/java.base/share/classes/java/util/stream/Streams.java index aa696c5ddb5..1b2d7e5c98c 100644 --- a/src/java.base/share/classes/java/util/stream/Streams.java +++ b/src/java.base/share/classes/java/util/stream/Streams.java @@ -804,6 +804,7 @@ final class Streams { } } + @SuppressWarnings("overloads") static class OfInt extends ConcatSpliterator.OfPrimitive implements Spliterator.OfInt { @@ -812,6 +813,7 @@ final class Streams { } } + @SuppressWarnings("overloads") static class OfLong extends ConcatSpliterator.OfPrimitive implements Spliterator.OfLong { @@ -820,6 +822,7 @@ final class Streams { } } + @SuppressWarnings("overloads") static class OfDouble extends ConcatSpliterator.OfPrimitive implements Spliterator.OfDouble { diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Flags.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Flags.java index 6b8d57730a3..f87dcee526e 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Flags.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Flags.java @@ -275,9 +275,8 @@ public class Flags { public static final long THROWS = 1L<<47; /** - * Flag that marks potentially ambiguous overloads + * Currently available: Bit 48. */ - public static final long POTENTIALLY_AMBIGUOUS = 1L<<48; /** * Flag that marks a synthetic method body for a lambda expression @@ -526,7 +525,7 @@ public class Flags { DEPRECATED_ANNOTATION(Flags.DEPRECATED_ANNOTATION), DEPRECATED_REMOVAL(Flags.DEPRECATED_REMOVAL), HAS_RESOURCE(Flags.HAS_RESOURCE), - POTENTIALLY_AMBIGUOUS(Flags.POTENTIALLY_AMBIGUOUS), + // Bit 48 is currently available ANONCONSTR_BASED(Flags.ANONCONSTR_BASED), NAME_FILLED(Flags.NAME_FILLED), PREVIEW_API(Flags.PREVIEW_API), diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java index 849c0039492..2163ad5bcbf 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java @@ -5563,6 +5563,7 @@ public class Attr extends JCTree.Visitor { // yet different return types). (JLS 8.4.8.3) chk.checkCompatibleSupertypes(tree.pos(), c.type); chk.checkDefaultMethodClashes(tree.pos(), c.type); + chk.checkPotentiallyAmbiguousOverloads(tree, c.type); } // Check that class does not import the same parameterized interface diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java index f98291f3de8..2e1689335e5 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java @@ -27,8 +27,13 @@ package com.sun.tools.javac.comp; import java.util.*; import java.util.function.BiConsumer; +import java.util.function.BiPredicate; +import java.util.function.Consumer; import java.util.function.Predicate; import java.util.function.Supplier; +import java.util.function.ToIntBiFunction; +import java.util.stream.Collectors; +import java.util.stream.StreamSupport; import javax.lang.model.element.ElementKind; import javax.lang.model.element.NestingKind; @@ -68,6 +73,7 @@ import static com.sun.tools.javac.code.Flags.SYNCHRONIZED; import static com.sun.tools.javac.code.Kinds.*; import static com.sun.tools.javac.code.Kinds.Kind.*; import static com.sun.tools.javac.code.Scope.LookupKind.NON_RECURSIVE; +import static com.sun.tools.javac.code.Scope.LookupKind.RECURSIVE; import static com.sun.tools.javac.code.TypeTag.*; import static com.sun.tools.javac.code.TypeTag.WILDCARD; @@ -90,6 +96,10 @@ import javax.lang.model.util.ElementKindVisitor14; public class Check { protected static final Context.Key checkKey = new Context.Key<>(); + // Flag bits indicating which item(s) chosen from a pair of items + private static final int FIRST = 0x01; + private static final int SECOND = 0x02; + private final Names names; private final Log log; private final Resolve rs; @@ -2554,27 +2564,13 @@ public class Check { //for each method m1 that is overridden (directly or indirectly) //by method 'sym' in 'site'... - List potentiallyAmbiguousList = List.nil(); - boolean overridesAny = false; ArrayList symbolsByName = new ArrayList<>(); types.membersClosure(site, false).getSymbolsByName(sym.name, cf).forEach(symbolsByName::add); for (Symbol m1 : symbolsByName) { if (!sym.overrides(m1, site.tsym, types, false)) { - if (m1 == sym) { - continue; - } - - if (!overridesAny) { - potentiallyAmbiguousList = potentiallyAmbiguousList.prepend((MethodSymbol)m1); - } continue; } - if (m1 != sym) { - overridesAny = true; - potentiallyAmbiguousList = List.nil(); - } - //...check each method m2 that is a member of 'site' for (Symbol m2 : symbolsByName) { if (m2 == m1) continue; @@ -2604,12 +2600,6 @@ public class Check { } } } - - if (!overridesAny) { - for (MethodSymbol m: potentiallyAmbiguousList) { - checkPotentiallyAmbiguousOverloads(pos, site, sym, m); - } - } } /** Check that all static methods accessible from 'site' are @@ -2630,8 +2620,6 @@ public class Check { log.error(pos, Errors.NameClashSameErasureNoHide(sym, sym.location(), s, s.location())); return; - } else { - checkPotentiallyAmbiguousOverloads(pos, site, sym, (MethodSymbol)s); } } } @@ -2720,59 +2708,237 @@ public class Check { } } + /** Report warnings for potentially ambiguous method declarations in the given site. */ + void checkPotentiallyAmbiguousOverloads(JCClassDecl tree, Type site) { + + // Skip if warning not enabled + if (!lint.isEnabled(LintCategory.OVERLOADS)) + return; + + // Gather all of site's methods, including overridden methods, grouped by name (except Object methods) + List> methodGroups = methodsGroupedByName(site, + new PotentiallyAmbiguousFilter(site), ArrayList::new); + + // Build the predicate that determines if site is responsible for an ambiguity + BiPredicate responsible = buildResponsiblePredicate(site, methodGroups); + + // Now remove overridden methods from each group, leaving only site's actual members + methodGroups.forEach(list -> removePreempted(list, (m1, m2) -> m1.overrides(m2, site.tsym, types, false))); + + // Allow site's own declared methods (only) to apply @SuppressWarnings("overloads") + methodGroups.forEach(list -> list.removeIf( + m -> m.owner == site.tsym && !lint.augment(m).isEnabled(LintCategory.OVERLOADS))); + + // Warn about ambiguous overload method pairs for which site is responsible + methodGroups.forEach(list -> compareAndRemove(list, (m1, m2) -> { + + // See if this is an ambiguous overload for which "site" is responsible + if (!potentiallyAmbiguousOverload(site, m1, m2) || !responsible.test(m1, m2)) + return 0; + + // Locate the warning at one of the methods, if possible + DiagnosticPosition pos = + m1.owner == site.tsym ? TreeInfo.diagnosticPositionFor(m1, tree) : + m2.owner == site.tsym ? TreeInfo.diagnosticPositionFor(m2, tree) : + tree.pos(); + + // Log the warning + log.warning(LintCategory.OVERLOADS, pos, + Warnings.PotentiallyAmbiguousOverload( + m1.asMemberOf(site, types), m1.location(), + m2.asMemberOf(site, types), m2.location())); + + // Don't warn again for either of these two methods + return FIRST | SECOND; + })); + } + + /** Build a predicate that determines, given two methods that are members of the given class, + * whether the class should be held "responsible" if the methods are potentially ambiguous. + * + * Sometimes ambiguous methods are unavoidable because they're inherited from a supertype. + * For example, any subtype of Spliterator.OfInt will have ambiguities for both + * forEachRemaining() and tryAdvance() (in both cases the overloads are IntConsumer and + * Consumer<? super Integer>). So we only want to "blame" a class when that class is + * itself responsible for creating the ambiguity. We declare that a class C is "responsible" + * for the ambiguity between two methods m1 and m2 if there is no direct supertype T of C + * such that m1 and m2, or some overrides thereof, both exist in T and are ambiguous in T. + * As an optimization, we first check if either method is declared in C and does not override + * any other methods; in this case the class is definitely responsible. + */ + BiPredicate buildResponsiblePredicate(Type site, + List> methodGroups) { + + // Define the "overrides" predicate + BiPredicate overrides = (m1, m2) -> m1.overrides(m2, site.tsym, types, false); + + // Map each method declared in site to a list of the supertype method(s) it directly overrides + HashMap> overriddenMethodsMap = new HashMap<>(); + methodGroups.forEach(list -> { + for (MethodSymbol m : list) { + + // Skip methods not declared in site + if (m.owner != site.tsym) + continue; + + // Gather all supertype methods overridden by m, directly or indirectly + ArrayList overriddenMethods = list.stream() + .filter(m2 -> m2 != m && overrides.test(m, m2)) + .collect(Collectors.toCollection(ArrayList::new)); + + // Eliminate non-direct overrides + removePreempted(overriddenMethods, overrides); + + // Add to map + overriddenMethodsMap.put(m, overriddenMethods); + } + }); + + // Build the predicate + return (m1, m2) -> { + + // Get corresponding supertype methods (if declared in site) + java.util.List overriddenMethods1 = overriddenMethodsMap.get(m1); + java.util.List overriddenMethods2 = overriddenMethodsMap.get(m2); + + // Quick check for the case where a method was added by site itself + if (overriddenMethods1 != null && overriddenMethods1.isEmpty()) + return true; + if (overriddenMethods2 != null && overriddenMethods2.isEmpty()) + return true; + + // Get each method's corresponding method(s) from supertypes of site + java.util.List supertypeMethods1 = overriddenMethods1 != null ? + overriddenMethods1 : Collections.singletonList(m1); + java.util.List supertypeMethods2 = overriddenMethods2 != null ? + overriddenMethods2 : Collections.singletonList(m2); + + // See if we can blame some direct supertype instead + return types.directSupertypes(site).stream() + .filter(stype -> stype != syms.objectType) + .map(stype -> stype.tsym.type) // view supertype in its original form + .noneMatch(stype -> { + for (MethodSymbol sm1 : supertypeMethods1) { + if (!types.isSubtype(types.erasure(stype), types.erasure(sm1.owner.type))) + continue; + for (MethodSymbol sm2 : supertypeMethods2) { + if (!types.isSubtype(types.erasure(stype), types.erasure(sm2.owner.type))) + continue; + if (potentiallyAmbiguousOverload(stype, sm1, sm2)) + return true; + } + } + return false; + }); + }; + } + + /** Gather all of site's methods, including overridden methods, grouped and sorted by name, + * after applying the given filter. + */ + > List methodsGroupedByName(Type site, + Predicate filter, Supplier groupMaker) { + Iterable symbols = types.membersClosure(site, false).getSymbols(filter, RECURSIVE); + return StreamSupport.stream(symbols.spliterator(), false) + .map(MethodSymbol.class::cast) + .collect(Collectors.groupingBy(m -> m.name, Collectors.toCollection(groupMaker))) + .entrySet() + .stream() + .sorted(Comparator.comparing(e -> e.getKey().toString())) + .map(Map.Entry::getValue) + .collect(List.collector()); + } + + /** Compare elements in a list pair-wise in order to remove some of them. + * @param list mutable list of items + * @param comparer returns flag bit(s) to remove FIRST and/or SECOND + */ + void compareAndRemove(java.util.List list, ToIntBiFunction comparer) { + for (int index1 = 0; index1 < list.size() - 1; index1++) { + T item1 = list.get(index1); + for (int index2 = index1 + 1; index2 < list.size(); index2++) { + T item2 = list.get(index2); + int flags = comparer.applyAsInt(item1, item2); + if ((flags & SECOND) != 0) + list.remove(index2--); // remove item2 + if ((flags & FIRST) != 0) { + list.remove(index1--); // remove item1 + break; + } + } + } + } + + /** Remove elements in a list that are preempted by some other element in the list. + * @param list mutable list of items + * @param preempts decides if one item preempts another, causing the second one to be removed + */ + void removePreempted(java.util.List list, BiPredicate preempts) { + compareAndRemove(list, (item1, item2) -> { + int flags = 0; + if (preempts.test(item1, item2)) + flags |= SECOND; + if (preempts.test(item2, item1)) + flags |= FIRST; + return flags; + }); + } + + /** Filters method candidates for the "potentially ambiguous method" check */ + class PotentiallyAmbiguousFilter extends ClashFilter { + + PotentiallyAmbiguousFilter(Type site) { + super(site); + } + + @Override + boolean shouldSkip(Symbol s) { + return s.owner.type.tsym == syms.objectType.tsym || super.shouldSkip(s); + } + } + /** * Report warnings for potentially ambiguous method declarations. Two declarations * are potentially ambiguous if they feature two unrelated functional interface * in same argument position (in which case, a call site passing an implicit - * lambda would be ambiguous). + * lambda would be ambiguous). This assumes they already have the same name. */ - void checkPotentiallyAmbiguousOverloads(DiagnosticPosition pos, Type site, - MethodSymbol msym1, MethodSymbol msym2) { - if (msym1 != msym2 && - lint.isEnabled(LintCategory.OVERLOADS) && - (msym1.flags() & POTENTIALLY_AMBIGUOUS) == 0 && - (msym2.flags() & POTENTIALLY_AMBIGUOUS) == 0) { - Type mt1 = types.memberType(site, msym1); - Type mt2 = types.memberType(site, msym2); - //if both generic methods, adjust type variables - if (mt1.hasTag(FORALL) && mt2.hasTag(FORALL) && - types.hasSameBounds((ForAll)mt1, (ForAll)mt2)) { - mt2 = types.subst(mt2, ((ForAll)mt2).tvars, ((ForAll)mt1).tvars); - } - //expand varargs methods if needed - int maxLength = Math.max(mt1.getParameterTypes().length(), mt2.getParameterTypes().length()); - List args1 = rs.adjustArgs(mt1.getParameterTypes(), msym1, maxLength, true); - List args2 = rs.adjustArgs(mt2.getParameterTypes(), msym2, maxLength, true); - //if arities don't match, exit - if (args1.length() != args2.length()) return; - boolean potentiallyAmbiguous = false; - while (args1.nonEmpty() && args2.nonEmpty()) { - Type s = args1.head; - Type t = args2.head; - if (!types.isSubtype(t, s) && !types.isSubtype(s, t)) { - if (types.isFunctionalInterface(s) && types.isFunctionalInterface(t) && - types.findDescriptorType(s).getParameterTypes().length() > 0 && - types.findDescriptorType(s).getParameterTypes().length() == - types.findDescriptorType(t).getParameterTypes().length()) { - potentiallyAmbiguous = true; - } else { - return; - } - } - args1 = args1.tail; - args2 = args2.tail; - } - if (potentiallyAmbiguous) { - //we found two incompatible functional interfaces with same arity - //this means a call site passing an implicit lambda would be ambiguous - msym1.flags_field |= POTENTIALLY_AMBIGUOUS; - msym2.flags_field |= POTENTIALLY_AMBIGUOUS; - log.warning(LintCategory.OVERLOADS, pos, - Warnings.PotentiallyAmbiguousOverload(msym1, msym1.location(), - msym2, msym2.location())); - return; - } + boolean potentiallyAmbiguousOverload(Type site, MethodSymbol msym1, MethodSymbol msym2) { + Assert.check(msym1.name == msym2.name); + if (msym1 == msym2) + return false; + Type mt1 = types.memberType(site, msym1); + Type mt2 = types.memberType(site, msym2); + //if both generic methods, adjust type variables + if (mt1.hasTag(FORALL) && mt2.hasTag(FORALL) && + types.hasSameBounds((ForAll)mt1, (ForAll)mt2)) { + mt2 = types.subst(mt2, ((ForAll)mt2).tvars, ((ForAll)mt1).tvars); } + //expand varargs methods if needed + int maxLength = Math.max(mt1.getParameterTypes().length(), mt2.getParameterTypes().length()); + List args1 = rs.adjustArgs(mt1.getParameterTypes(), msym1, maxLength, true); + List args2 = rs.adjustArgs(mt2.getParameterTypes(), msym2, maxLength, true); + //if arities don't match, exit + if (args1.length() != args2.length()) + return false; + boolean potentiallyAmbiguous = false; + while (args1.nonEmpty() && args2.nonEmpty()) { + Type s = args1.head; + Type t = args2.head; + if (!types.isSubtype(t, s) && !types.isSubtype(s, t)) { + if (types.isFunctionalInterface(s) && types.isFunctionalInterface(t) && + types.findDescriptorType(s).getParameterTypes().length() > 0 && + types.findDescriptorType(s).getParameterTypes().length() == + types.findDescriptorType(t).getParameterTypes().length()) { + potentiallyAmbiguous = true; + } else { + return false; + } + } + args1 = args1.tail; + args2 = args2.tail; + } + return potentiallyAmbiguous; } void checkAccessFromSerializableElement(final JCTree tree, boolean isLambda) { diff --git a/test/langtools/tools/javac/8230827/T8230827.out b/test/langtools/tools/javac/8230827/T8230827.out index f5a1f9b2ffc..84d636f07ad 100644 --- a/test/langtools/tools/javac/8230827/T8230827.out +++ b/test/langtools/tools/javac/8230827/T8230827.out @@ -1,7 +1,7 @@ -T8230827.java:27:17: compiler.warn.potentially.ambiguous.overload: ambiguousMethod1(java.lang.Object,T8230827.I1), T8230827, ambiguousMethod1(java.lang.Object,T8230827.I2), T8230827 -T8230827.java:32:17: compiler.warn.potentially.ambiguous.overload: ambiguousMethod2(T8230827.I1,java.lang.Object), T8230827, ambiguousMethod2(T8230827.I2,java.lang.Object), T8230827 -T8230827.java:37:17: compiler.warn.potentially.ambiguous.overload: ambiguousMethod3(T8230827.I1,T8230827.I1), T8230827, ambiguousMethod3(T8230827.I2,T8230827.I1), T8230827 -T8230827.java:42:17: compiler.warn.potentially.ambiguous.overload: ambiguousMethod4(java.lang.Object,T8230827.I1,java.lang.String), T8230827, ambiguousMethod4(java.lang.String,T8230827.I2,java.lang.Object), T8230827 +T8230827.java:29:17: compiler.warn.potentially.ambiguous.overload: ambiguousMethod1(java.lang.Object,T8230827.I2), T8230827, ambiguousMethod1(java.lang.Object,T8230827.I1), T8230827 +T8230827.java:34:17: compiler.warn.potentially.ambiguous.overload: ambiguousMethod2(T8230827.I2,java.lang.Object), T8230827, ambiguousMethod2(T8230827.I1,java.lang.Object), T8230827 +T8230827.java:39:17: compiler.warn.potentially.ambiguous.overload: ambiguousMethod3(T8230827.I2,T8230827.I1), T8230827, ambiguousMethod3(T8230827.I1,T8230827.I1), T8230827 +T8230827.java:44:17: compiler.warn.potentially.ambiguous.overload: ambiguousMethod4(java.lang.String,T8230827.I2,java.lang.Object), T8230827, ambiguousMethod4(java.lang.Object,T8230827.I1,java.lang.String), T8230827 - compiler.err.warnings.and.werror 1 error -4 warnings \ No newline at end of file +4 warnings diff --git a/test/langtools/tools/javac/lambda/T8024947/PotentiallyAmbiguousWarningTest.java b/test/langtools/tools/javac/lambda/T8024947/PotentiallyAmbiguousWarningTest.java index 7d4b07a4190..e35495eaa21 100644 --- a/test/langtools/tools/javac/lambda/T8024947/PotentiallyAmbiguousWarningTest.java +++ b/test/langtools/tools/javac/lambda/T8024947/PotentiallyAmbiguousWarningTest.java @@ -1,6 +1,6 @@ /* * @test /nodynamiccopyright/ - * @bug 8024947 + * @bug 8024947 8026369 * @summary javac should issue the potentially ambiguous overload warning only * where the problem appears * @compile/fail/ref=PotentiallyAmbiguousWarningTest.out -XDrawDiagnostics -Werror -Xlint:overloads PotentiallyAmbiguousWarningTest.java @@ -70,4 +70,74 @@ public interface PotentiallyAmbiguousWarningTest { interface J2 extends I5 { void foo(Consumer c); } + +// The test cases below are from JDK-8026369 + + interface I6 { + void foo(Consumer c); + } + + interface I7 { + void foo(IntConsumer c); + } + + //a warning should be fired, I8 is provoking the issue + interface I8 extends I6, I7 { } + + //no warning here, the issue is introduced in I8 + interface I9 extends I8 { } + + //no warning here + interface I10 { + void foo(Consumer c); + void foo(T c); + } + + //a warning should be fired, I11 is provoking the issue + interface I11 extends I10 { } + + // No warning should be fired here + interface I12 extends Consumer, IntSupplier { + // A warning should be fired here + interface OfInt extends I12, IntConsumer { + @Override + void accept(int value); + default void accept(Integer i) { } + } + @Override + default int getAsInt() { return 0; } + } + + // No warning should be fired here + abstract static class C6 implements I12.OfInt { } + + default Object foo() { + // No warning should be fired here + return new C6() { + @Override + public void accept(int value) { } + }; + } + + // Overrides should not trigger warnings + interface I13 extends I8 { + @Override + void foo(Consumer c); + @Override + void foo(IntConsumer c); + } + interface I14 extends I8 { + @Override + void foo(IntConsumer c); + } + + // Verify we can suppress warnings at the class level + @SuppressWarnings("overloads") + interface I15 extends I8 { } // would normally trigger a warning + + // Verify we can suppress warnings at the method level + interface I16 extends I2 { + @SuppressWarnings("overloads") + void foo(IntConsumer c); // would normally trigger a warning + } } diff --git a/test/langtools/tools/javac/lambda/T8024947/PotentiallyAmbiguousWarningTest.out b/test/langtools/tools/javac/lambda/T8024947/PotentiallyAmbiguousWarningTest.out index 55b50a3965d..65104dacd30 100644 --- a/test/langtools/tools/javac/lambda/T8024947/PotentiallyAmbiguousWarningTest.out +++ b/test/langtools/tools/javac/lambda/T8024947/PotentiallyAmbiguousWarningTest.out @@ -1,9 +1,12 @@ -PotentiallyAmbiguousWarningTest.java:15:14: compiler.warn.potentially.ambiguous.overload: foo(java.util.function.Consumer), PotentiallyAmbiguousWarningTest.I1, foo(java.util.function.IntConsumer), PotentiallyAmbiguousWarningTest.I1 -PotentiallyAmbiguousWarningTest.java:21:14: compiler.warn.potentially.ambiguous.overload: foo(java.util.function.Consumer), PotentiallyAmbiguousWarningTest.C1, foo(java.util.function.IntConsumer), PotentiallyAmbiguousWarningTest.C1 +PotentiallyAmbiguousWarningTest.java:16:14: compiler.warn.potentially.ambiguous.overload: foo(java.util.function.IntConsumer), PotentiallyAmbiguousWarningTest.I1, foo(java.util.function.Consumer), PotentiallyAmbiguousWarningTest.I1 +PotentiallyAmbiguousWarningTest.java:22:14: compiler.warn.potentially.ambiguous.overload: foo(java.util.function.IntConsumer), PotentiallyAmbiguousWarningTest.C1, foo(java.util.function.Consumer), PotentiallyAmbiguousWarningTest.C1 PotentiallyAmbiguousWarningTest.java:31:14: compiler.warn.potentially.ambiguous.overload: foo(java.util.function.IntConsumer), PotentiallyAmbiguousWarningTest.J1, foo(java.util.function.Consumer), PotentiallyAmbiguousWarningTest.I2 PotentiallyAmbiguousWarningTest.java:48:14: compiler.warn.potentially.ambiguous.overload: foo(java.util.function.IntConsumer), PotentiallyAmbiguousWarningTest.D1, foo(java.util.function.Consumer), PotentiallyAmbiguousWarningTest.C2 PotentiallyAmbiguousWarningTest.java:54:21: compiler.warn.potentially.ambiguous.overload: foo(java.util.function.IntConsumer), PotentiallyAmbiguousWarningTest.C3, foo(java.util.function.Consumer), PotentiallyAmbiguousWarningTest.C3 -PotentiallyAmbiguousWarningTest.java:71:14: compiler.warn.potentially.ambiguous.overload: foo(java.util.function.Consumer), PotentiallyAmbiguousWarningTest.J2, foo(T), PotentiallyAmbiguousWarningTest.I5 +PotentiallyAmbiguousWarningTest.java:71:14: compiler.warn.potentially.ambiguous.overload: foo(java.util.function.Consumer), PotentiallyAmbiguousWarningTest.J2, foo(java.util.function.IntConsumer), PotentiallyAmbiguousWarningTest.I5 +PotentiallyAmbiguousWarningTest.java:85:5: compiler.warn.potentially.ambiguous.overload: foo(java.util.function.IntConsumer), PotentiallyAmbiguousWarningTest.I7, foo(java.util.function.Consumer), PotentiallyAmbiguousWarningTest.I6 +PotentiallyAmbiguousWarningTest.java:97:5: compiler.warn.potentially.ambiguous.overload: foo(java.util.function.IntConsumer), PotentiallyAmbiguousWarningTest.I10, foo(java.util.function.Consumer), PotentiallyAmbiguousWarningTest.I10 +PotentiallyAmbiguousWarningTest.java:102:9: compiler.warn.potentially.ambiguous.overload: andThen(java.util.function.IntConsumer), java.util.function.IntConsumer, andThen(java.util.function.Consumer), java.util.function.Consumer - compiler.err.warnings.and.werror 1 error -6 warnings +9 warnings