8026369: javac potentially ambiguous overload warning needs an improved scheme
Reviewed-by: vromero
This commit is contained in:
parent
14a014d430
commit
1e3c9fd67e
@ -63,6 +63,7 @@ import java.util.stream.Collector;
|
||||
* <p>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;
|
||||
|
@ -91,6 +91,7 @@ public interface PrimitiveIterator<T, T_CONS> extends Iterator<T> {
|
||||
* An Iterator specialized for {@code int} values.
|
||||
* @since 1.8
|
||||
*/
|
||||
@SuppressWarnings("overloads")
|
||||
public static interface OfInt extends PrimitiveIterator<Integer, IntConsumer> {
|
||||
|
||||
/**
|
||||
@ -158,6 +159,7 @@ public interface PrimitiveIterator<T, T_CONS> extends Iterator<T> {
|
||||
* An Iterator specialized for {@code long} values.
|
||||
* @since 1.8
|
||||
*/
|
||||
@SuppressWarnings("overloads")
|
||||
public static interface OfLong extends PrimitiveIterator<Long, LongConsumer> {
|
||||
|
||||
/**
|
||||
@ -224,6 +226,7 @@ public interface PrimitiveIterator<T, T_CONS> extends Iterator<T> {
|
||||
* An Iterator specialized for {@code double} values.
|
||||
* @since 1.8
|
||||
*/
|
||||
@SuppressWarnings("overloads")
|
||||
public static interface OfDouble extends PrimitiveIterator<Double, DoubleConsumer> {
|
||||
|
||||
/**
|
||||
|
@ -659,6 +659,7 @@ public interface Spliterator<T> {
|
||||
* A Spliterator specialized for {@code int} values.
|
||||
* @since 1.8
|
||||
*/
|
||||
@SuppressWarnings("overloads")
|
||||
public interface OfInt extends OfPrimitive<Integer, IntConsumer, OfInt> {
|
||||
|
||||
@Override
|
||||
@ -723,6 +724,7 @@ public interface Spliterator<T> {
|
||||
* A Spliterator specialized for {@code long} values.
|
||||
* @since 1.8
|
||||
*/
|
||||
@SuppressWarnings("overloads")
|
||||
public interface OfLong extends OfPrimitive<Long, LongConsumer, OfLong> {
|
||||
|
||||
@Override
|
||||
@ -787,6 +789,7 @@ public interface Spliterator<T> {
|
||||
* A Spliterator specialized for {@code double} values.
|
||||
* @since 1.8
|
||||
*/
|
||||
@SuppressWarnings("overloads")
|
||||
public interface OfDouble extends OfPrimitive<Double, DoubleConsumer, OfDouble> {
|
||||
|
||||
@Override
|
||||
|
@ -908,18 +908,21 @@ public final class Spliterators {
|
||||
OfRef() { }
|
||||
}
|
||||
|
||||
@SuppressWarnings("overloads")
|
||||
private static final class OfInt
|
||||
extends EmptySpliterator<Integer, Spliterator.OfInt, IntConsumer>
|
||||
implements Spliterator.OfInt {
|
||||
OfInt() { }
|
||||
}
|
||||
|
||||
@SuppressWarnings("overloads")
|
||||
private static final class OfLong
|
||||
extends EmptySpliterator<Long, Spliterator.OfLong, LongConsumer>
|
||||
implements Spliterator.OfLong {
|
||||
OfLong() { }
|
||||
}
|
||||
|
||||
@SuppressWarnings("overloads")
|
||||
private static final class OfDouble
|
||||
extends EmptySpliterator<Double, Spliterator.OfDouble, DoubleConsumer>
|
||||
implements Spliterator.OfDouble {
|
||||
|
@ -314,6 +314,7 @@ interface Node<T> {
|
||||
/**
|
||||
* Specialized {@code Node} for int elements
|
||||
*/
|
||||
@SuppressWarnings("overloads")
|
||||
interface OfInt extends OfPrimitive<Integer, IntConsumer, int[], Spliterator.OfInt, OfInt> {
|
||||
|
||||
/**
|
||||
@ -391,6 +392,7 @@ interface Node<T> {
|
||||
/**
|
||||
* Specialized {@code Node} for long elements
|
||||
*/
|
||||
@SuppressWarnings("overloads")
|
||||
interface OfLong extends OfPrimitive<Long, LongConsumer, long[], Spliterator.OfLong, OfLong> {
|
||||
|
||||
/**
|
||||
@ -468,6 +470,7 @@ interface Node<T> {
|
||||
/**
|
||||
* Specialized {@code Node} for double elements
|
||||
*/
|
||||
@SuppressWarnings("overloads")
|
||||
interface OfDouble extends OfPrimitive<Double, DoubleConsumer, double[], Spliterator.OfDouble, OfDouble> {
|
||||
|
||||
/**
|
||||
|
@ -582,6 +582,7 @@ final class Nodes {
|
||||
}
|
||||
}
|
||||
|
||||
@SuppressWarnings("overloads")
|
||||
private static final class OfInt
|
||||
extends EmptyNode<Integer, int[], IntConsumer>
|
||||
implements Node.OfInt {
|
||||
@ -599,6 +600,7 @@ final class Nodes {
|
||||
}
|
||||
}
|
||||
|
||||
@SuppressWarnings("overloads")
|
||||
private static final class OfLong
|
||||
extends EmptyNode<Long, long[], LongConsumer>
|
||||
implements Node.OfLong {
|
||||
@ -616,6 +618,7 @@ final class Nodes {
|
||||
}
|
||||
}
|
||||
|
||||
@SuppressWarnings("overloads")
|
||||
private static final class OfDouble
|
||||
extends EmptyNode<Double, double[], DoubleConsumer>
|
||||
implements Node.OfDouble {
|
||||
@ -880,6 +883,7 @@ final class Nodes {
|
||||
}
|
||||
}
|
||||
|
||||
@SuppressWarnings("overloads")
|
||||
static final class OfInt
|
||||
extends ConcNode.OfPrimitive<Integer, IntConsumer, int[], Spliterator.OfInt, Node.OfInt>
|
||||
implements Node.OfInt {
|
||||
@ -894,6 +898,7 @@ final class Nodes {
|
||||
}
|
||||
}
|
||||
|
||||
@SuppressWarnings("overloads")
|
||||
static final class OfLong
|
||||
extends ConcNode.OfPrimitive<Long, LongConsumer, long[], Spliterator.OfLong, Node.OfLong>
|
||||
implements Node.OfLong {
|
||||
@ -908,6 +913,7 @@ final class Nodes {
|
||||
}
|
||||
}
|
||||
|
||||
@SuppressWarnings("overloads")
|
||||
static final class OfDouble
|
||||
extends ConcNode.OfPrimitive<Double, DoubleConsumer, double[], Spliterator.OfDouble, Node.OfDouble>
|
||||
implements Node.OfDouble {
|
||||
@ -1160,6 +1166,7 @@ final class Nodes {
|
||||
}
|
||||
}
|
||||
|
||||
@SuppressWarnings("overloads")
|
||||
private static final class OfInt
|
||||
extends OfPrimitive<Integer, IntConsumer, int[], Spliterator.OfInt, Node.OfInt>
|
||||
implements Spliterator.OfInt {
|
||||
@ -1169,6 +1176,7 @@ final class Nodes {
|
||||
}
|
||||
}
|
||||
|
||||
@SuppressWarnings("overloads")
|
||||
private static final class OfLong
|
||||
extends OfPrimitive<Long, LongConsumer, long[], Spliterator.OfLong, Node.OfLong>
|
||||
implements Spliterator.OfLong {
|
||||
@ -1178,6 +1186,7 @@ final class Nodes {
|
||||
}
|
||||
}
|
||||
|
||||
@SuppressWarnings("overloads")
|
||||
private static final class OfDouble
|
||||
extends OfPrimitive<Double, DoubleConsumer, double[], Spliterator.OfDouble, Node.OfDouble>
|
||||
implements Spliterator.OfDouble {
|
||||
|
@ -186,6 +186,7 @@ interface Sink<T> extends Consumer<T> {
|
||||
* {@code accept(int)}, and wires {@code accept(Integer)} to bridge to
|
||||
* {@code accept(int)}.
|
||||
*/
|
||||
@SuppressWarnings("overloads")
|
||||
interface OfInt extends Sink<Integer>, IntConsumer {
|
||||
@Override
|
||||
void accept(int value);
|
||||
@ -203,6 +204,7 @@ interface Sink<T> extends Consumer<T> {
|
||||
* {@code accept(long)}, and wires {@code accept(Long)} to bridge to
|
||||
* {@code accept(long)}.
|
||||
*/
|
||||
@SuppressWarnings("overloads")
|
||||
interface OfLong extends Sink<Long>, LongConsumer {
|
||||
@Override
|
||||
void accept(long value);
|
||||
@ -220,6 +222,7 @@ interface Sink<T> extends Consumer<T> {
|
||||
* {@code accept(double)}, and wires {@code accept(Double)} to bridge to
|
||||
* {@code accept(double)}.
|
||||
*/
|
||||
@SuppressWarnings("overloads")
|
||||
interface OfDouble extends Sink<Double>, DoubleConsumer {
|
||||
@Override
|
||||
void accept(double value);
|
||||
|
@ -720,6 +720,7 @@ class SpinedBuffer<E>
|
||||
/**
|
||||
* An ordered collection of {@code int} values.
|
||||
*/
|
||||
@SuppressWarnings("overloads")
|
||||
static class OfInt extends SpinedBuffer.OfPrimitive<Integer, int[], IntConsumer>
|
||||
implements IntConsumer {
|
||||
OfInt() { }
|
||||
@ -785,6 +786,7 @@ class SpinedBuffer<E>
|
||||
}
|
||||
|
||||
public Spliterator.OfInt spliterator() {
|
||||
@SuppressWarnings("overloads")
|
||||
class Splitr extends BaseSpliterator<Spliterator.OfInt>
|
||||
implements Spliterator.OfInt {
|
||||
Splitr(int firstSpineIndex, int lastSpineIndex,
|
||||
@ -833,6 +835,7 @@ class SpinedBuffer<E>
|
||||
/**
|
||||
* An ordered collection of {@code long} values.
|
||||
*/
|
||||
@SuppressWarnings("overloads")
|
||||
static class OfLong extends SpinedBuffer.OfPrimitive<Long, long[], LongConsumer>
|
||||
implements LongConsumer {
|
||||
OfLong() { }
|
||||
@ -899,6 +902,7 @@ class SpinedBuffer<E>
|
||||
|
||||
|
||||
public Spliterator.OfLong spliterator() {
|
||||
@SuppressWarnings("overloads")
|
||||
class Splitr extends BaseSpliterator<Spliterator.OfLong>
|
||||
implements Spliterator.OfLong {
|
||||
Splitr(int firstSpineIndex, int lastSpineIndex,
|
||||
@ -947,6 +951,7 @@ class SpinedBuffer<E>
|
||||
/**
|
||||
* An ordered collection of {@code double} values.
|
||||
*/
|
||||
@SuppressWarnings("overloads")
|
||||
static class OfDouble
|
||||
extends SpinedBuffer.OfPrimitive<Double, double[], DoubleConsumer>
|
||||
implements DoubleConsumer {
|
||||
@ -1013,6 +1018,7 @@ class SpinedBuffer<E>
|
||||
}
|
||||
|
||||
public Spliterator.OfDouble spliterator() {
|
||||
@SuppressWarnings("overloads")
|
||||
class Splitr extends BaseSpliterator<Spliterator.OfDouble>
|
||||
implements Spliterator.OfDouble {
|
||||
Splitr(int firstSpineIndex, int lastSpineIndex,
|
||||
|
@ -572,6 +572,7 @@ class StreamSpliterators {
|
||||
}
|
||||
}
|
||||
|
||||
@SuppressWarnings("overloads")
|
||||
static final class OfInt
|
||||
extends OfPrimitive<Integer, IntConsumer, Spliterator.OfInt>
|
||||
implements Spliterator.OfInt {
|
||||
@ -581,6 +582,7 @@ class StreamSpliterators {
|
||||
}
|
||||
}
|
||||
|
||||
@SuppressWarnings("overloads")
|
||||
static final class OfLong
|
||||
extends OfPrimitive<Long, LongConsumer, Spliterator.OfLong>
|
||||
implements Spliterator.OfLong {
|
||||
@ -590,6 +592,7 @@ class StreamSpliterators {
|
||||
}
|
||||
}
|
||||
|
||||
@SuppressWarnings("overloads")
|
||||
static final class OfDouble
|
||||
extends OfPrimitive<Double, DoubleConsumer, Spliterator.OfDouble>
|
||||
implements Spliterator.OfDouble {
|
||||
@ -815,6 +818,7 @@ class StreamSpliterators {
|
||||
protected abstract T_CONS emptyConsumer();
|
||||
}
|
||||
|
||||
@SuppressWarnings("overloads")
|
||||
static final class OfInt extends OfPrimitive<Integer, Spliterator.OfInt, IntConsumer>
|
||||
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<Long, Spliterator.OfLong, LongConsumer>
|
||||
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<Double, Spliterator.OfDouble, DoubleConsumer>
|
||||
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<Integer, IntConsumer, ArrayBuffer.OfInt, Spliterator.OfInt>
|
||||
implements Spliterator.OfInt, IntConsumer {
|
||||
@ -1163,6 +1170,7 @@ class StreamSpliterators {
|
||||
}
|
||||
}
|
||||
|
||||
@SuppressWarnings("overloads")
|
||||
static final class OfLong
|
||||
extends OfPrimitive<Long, LongConsumer, ArrayBuffer.OfLong, Spliterator.OfLong>
|
||||
implements Spliterator.OfLong, LongConsumer {
|
||||
@ -1198,6 +1206,7 @@ class StreamSpliterators {
|
||||
}
|
||||
}
|
||||
|
||||
@SuppressWarnings("overloads")
|
||||
static final class OfDouble
|
||||
extends OfPrimitive<Double, DoubleConsumer, ArrayBuffer.OfDouble, Spliterator.OfDouble>
|
||||
implements Spliterator.OfDouble, DoubleConsumer {
|
||||
|
@ -804,6 +804,7 @@ final class Streams {
|
||||
}
|
||||
}
|
||||
|
||||
@SuppressWarnings("overloads")
|
||||
static class OfInt
|
||||
extends ConcatSpliterator.OfPrimitive<Integer, IntConsumer, Spliterator.OfInt>
|
||||
implements Spliterator.OfInt {
|
||||
@ -812,6 +813,7 @@ final class Streams {
|
||||
}
|
||||
}
|
||||
|
||||
@SuppressWarnings("overloads")
|
||||
static class OfLong
|
||||
extends ConcatSpliterator.OfPrimitive<Long, LongConsumer, Spliterator.OfLong>
|
||||
implements Spliterator.OfLong {
|
||||
@ -820,6 +822,7 @@ final class Streams {
|
||||
}
|
||||
}
|
||||
|
||||
@SuppressWarnings("overloads")
|
||||
static class OfDouble
|
||||
extends ConcatSpliterator.OfPrimitive<Double, DoubleConsumer, Spliterator.OfDouble>
|
||||
implements Spliterator.OfDouble {
|
||||
|
@ -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),
|
||||
|
@ -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
|
||||
|
@ -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<Check> 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<MethodSymbol> potentiallyAmbiguousList = List.nil();
|
||||
boolean overridesAny = false;
|
||||
ArrayList<Symbol> 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<java.util.List<MethodSymbol>> methodGroups = methodsGroupedByName(site,
|
||||
new PotentiallyAmbiguousFilter(site), ArrayList::new);
|
||||
|
||||
// Build the predicate that determines if site is responsible for an ambiguity
|
||||
BiPredicate<MethodSymbol, MethodSymbol> 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<MethodSymbol, MethodSymbol> buildResponsiblePredicate(Type site,
|
||||
List<? extends Collection<MethodSymbol>> methodGroups) {
|
||||
|
||||
// Define the "overrides" predicate
|
||||
BiPredicate<MethodSymbol, MethodSymbol> 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<MethodSymbol, ArrayList<MethodSymbol>> 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<MethodSymbol> 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<MethodSymbol> overriddenMethods1 = overriddenMethodsMap.get(m1);
|
||||
java.util.List<MethodSymbol> 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<MethodSymbol> supertypeMethods1 = overriddenMethods1 != null ?
|
||||
overriddenMethods1 : Collections.singletonList(m1);
|
||||
java.util.List<MethodSymbol> 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.
|
||||
*/
|
||||
<C extends Collection<MethodSymbol>> List<C> methodsGroupedByName(Type site,
|
||||
Predicate<Symbol> filter, Supplier<? extends C> groupMaker) {
|
||||
Iterable<Symbol> 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
|
||||
*/
|
||||
<T> void compareAndRemove(java.util.List<T> list, ToIntBiFunction<? super T, ? super T> 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
|
||||
*/
|
||||
<T> void removePreempted(java.util.List<T> list, BiPredicate<? super T, ? super T> 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<Type> args1 = rs.adjustArgs(mt1.getParameterTypes(), msym1, maxLength, true);
|
||||
List<Type> 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<Type> args1 = rs.adjustArgs(mt1.getParameterTypes(), msym1, maxLength, true);
|
||||
List<Type> 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) {
|
||||
|
@ -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
|
||||
4 warnings
|
||||
|
@ -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<IntConsumer> {
|
||||
void foo(Consumer<Integer> c);
|
||||
}
|
||||
|
||||
// The test cases below are from JDK-8026369
|
||||
|
||||
interface I6 {
|
||||
void foo(Consumer<Integer> 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<T> {
|
||||
void foo(Consumer<Integer> c);
|
||||
void foo(T c);
|
||||
}
|
||||
|
||||
//a warning should be fired, I11 is provoking the issue
|
||||
interface I11 extends I10<IntConsumer> { }
|
||||
|
||||
// No warning should be fired here
|
||||
interface I12<T> extends Consumer<T>, IntSupplier {
|
||||
// A warning should be fired here
|
||||
interface OfInt extends I12<Integer>, 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<T> implements I12.OfInt { }
|
||||
|
||||
default <U> Object foo() {
|
||||
// No warning should be fired here
|
||||
return new C6<U>() {
|
||||
@Override
|
||||
public void accept(int value) { }
|
||||
};
|
||||
}
|
||||
|
||||
// Overrides should not trigger warnings
|
||||
interface I13 extends I8 {
|
||||
@Override
|
||||
void foo(Consumer<Integer> 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
|
||||
}
|
||||
}
|
||||
|
@ -1,9 +1,12 @@
|
||||
PotentiallyAmbiguousWarningTest.java:15:14: compiler.warn.potentially.ambiguous.overload: foo(java.util.function.Consumer<java.lang.Integer>), PotentiallyAmbiguousWarningTest.I1, foo(java.util.function.IntConsumer), PotentiallyAmbiguousWarningTest.I1
|
||||
PotentiallyAmbiguousWarningTest.java:21:14: compiler.warn.potentially.ambiguous.overload: foo(java.util.function.Consumer<java.lang.Integer>), 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<java.lang.Integer>), PotentiallyAmbiguousWarningTest.I1
|
||||
PotentiallyAmbiguousWarningTest.java:22:14: compiler.warn.potentially.ambiguous.overload: foo(java.util.function.IntConsumer), PotentiallyAmbiguousWarningTest.C1, foo(java.util.function.Consumer<java.lang.Integer>), PotentiallyAmbiguousWarningTest.C1
|
||||
PotentiallyAmbiguousWarningTest.java:31:14: compiler.warn.potentially.ambiguous.overload: foo(java.util.function.IntConsumer), PotentiallyAmbiguousWarningTest.J1, foo(java.util.function.Consumer<java.lang.Integer>), PotentiallyAmbiguousWarningTest.I2
|
||||
PotentiallyAmbiguousWarningTest.java:48:14: compiler.warn.potentially.ambiguous.overload: foo(java.util.function.IntConsumer), PotentiallyAmbiguousWarningTest.D1, foo(java.util.function.Consumer<java.lang.Integer>), PotentiallyAmbiguousWarningTest.C2
|
||||
PotentiallyAmbiguousWarningTest.java:54:21: compiler.warn.potentially.ambiguous.overload: foo(java.util.function.IntConsumer), PotentiallyAmbiguousWarningTest.C3, foo(java.util.function.Consumer<java.lang.Integer>), PotentiallyAmbiguousWarningTest.C3
|
||||
PotentiallyAmbiguousWarningTest.java:71:14: compiler.warn.potentially.ambiguous.overload: foo(java.util.function.Consumer<java.lang.Integer>), PotentiallyAmbiguousWarningTest.J2, foo(T), PotentiallyAmbiguousWarningTest.I5
|
||||
PotentiallyAmbiguousWarningTest.java:71:14: compiler.warn.potentially.ambiguous.overload: foo(java.util.function.Consumer<java.lang.Integer>), 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<java.lang.Integer>), PotentiallyAmbiguousWarningTest.I6
|
||||
PotentiallyAmbiguousWarningTest.java:97:5: compiler.warn.potentially.ambiguous.overload: foo(java.util.function.IntConsumer), PotentiallyAmbiguousWarningTest.I10, foo(java.util.function.Consumer<java.lang.Integer>), 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<? super java.lang.Integer>), java.util.function.Consumer
|
||||
- compiler.err.warnings.and.werror
|
||||
1 error
|
||||
6 warnings
|
||||
9 warnings
|
||||
|
Loading…
x
Reference in New Issue
Block a user