Skip to content

Commit 5437622

Browse files
xedinxedin
committed
[Diagnostics] Diagnose ambiguity with conflicting arguments to generic parameters
It's done by first retrieving all generic parameters from each solution, filtering boundings into distrinct set and diagnosing any differences. For example: ```swift func foo<T>(_: T, _: T) {} func bar(x: Int, y: Float) { foo(x, y) } ```
1 parent ab59032 commit 5437622

File tree

11 files changed

+187
-46
lines changed

11 files changed

+187
-46
lines changed

‎include/swift/AST/DiagnosticsSema.def

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -385,6 +385,10 @@ ERROR(cannot_convert_argument_value_generic,none,
385385
"cannot convert value of type %0 (%1) to expected argument type %2 (%3)",
386386
(Type, StringRef, Type, StringRef))
387387

388+
ERROR(conflicting_arguments_for_generic_parameter,none,
389+
"conflicting arguments to generic parameter %0 (%1)",
390+
(Type, StringRef))
391+
388392
// @_nonEphemeral conversion diagnostics
389393
ERROR(cannot_pass_type_to_non_ephemeral,none,
390394
"cannot pass %0 to parameter; argument %1 must be a pointer that "

‎lib/Sema/CSDiagnostics.cpp

Lines changed: 0 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -5585,40 +5585,6 @@ bool ArgumentMismatchFailure::diagnoseArchetypeMismatch() const {
55855585
if (!(paramDecl && argDecl))
55865586
return false;
55875587

5588-
auto describeGenericType = [&](ValueDecl *genericParam,
5589-
bool includeName = false) -> std::string {
5590-
if (!genericParam)
5591-
return "";
5592-
5593-
Decl *parent = nullptr;
5594-
if (auto *AT = dyn_cast<AssociatedTypeDecl>(genericParam)) {
5595-
parent = AT->getProtocol();
5596-
} else {
5597-
auto *dc = genericParam->getDeclContext();
5598-
parent = dc->getInnermostDeclarationDeclContext();
5599-
}
5600-
5601-
if (!parent)
5602-
return "";
5603-
5604-
llvm::SmallString<64> result;
5605-
llvm::raw_svector_ostream OS(result);
5606-
5607-
OS << Decl::getDescriptiveKindName(genericParam->getDescriptiveKind());
5608-
5609-
if (includeName && genericParam->hasName())
5610-
OS << " '" << genericParam->getBaseName() << "'";
5611-
5612-
OS << " of ";
5613-
OS << Decl::getDescriptiveKindName(parent->getDescriptiveKind());
5614-
if (auto *decl = dyn_cast<ValueDecl>(parent)) {
5615-
if (decl->hasName())
5616-
OS << " '" << decl->getFullName() << "'";
5617-
}
5618-
5619-
return OS.str();
5620-
};
5621-
56225588
emitDiagnostic(
56235589
getAnchor()->getLoc(), diag::cannot_convert_argument_value_generic, argTy,
56245590
describeGenericType(argDecl), paramTy, describeGenericType(paramDecl));

‎lib/Sema/CSFix.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1106,6 +1106,12 @@ UseValueTypeOfRawRepresentative::attempt(ConstraintSystem &cs, Type argType,
11061106
return nullptr;
11071107
}
11081108

1109+
unsigned AllowArgumentMismatch::getParamIdx() const {
1110+
const auto *locator = getLocator();
1111+
auto elt = locator->castLastElementTo<LocatorPathElt::ApplyArgToParam>();
1112+
return elt.getParamIdx();
1113+
}
1114+
11091115
bool AllowArgumentMismatch::diagnose(bool asNote) const {
11101116
auto &cs = getConstraintSystem();
11111117
ArgumentMismatchFailure failure(cs, getFromType(), getToType(),

‎lib/Sema/CSFix.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1499,11 +1499,17 @@ class AllowArgumentMismatch : public ContextualMismatch {
14991499
return "allow argument to parameter type conversion mismatch";
15001500
}
15011501

1502+
unsigned getParamIdx() const;
1503+
15021504
bool diagnose(bool asNote = false) const override;
15031505

15041506
static AllowArgumentMismatch *create(ConstraintSystem &cs, Type argType,
15051507
Type paramType,
15061508
ConstraintLocator *locator);
1509+
1510+
static bool classof(const ConstraintFix *fix) {
1511+
return fix->getKind() == FixKind::AllowArgumentTypeMismatch;
1512+
}
15071513
};
15081514

15091515
class ExpandArrayIntoVarargs final : public AllowArgumentMismatch {

‎lib/Sema/ConstraintSystem.cpp

Lines changed: 153 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2729,6 +2729,154 @@ static void diagnoseOperatorAmbiguity(ConstraintSystem &cs,
27292729
llvm::join(parameters, ", "));
27302730
}
27312731

2732+
std::string swift::describeGenericType(ValueDecl *GP, bool includeName) {
2733+
if (!GP)
2734+
return "";
2735+
2736+
Decl *parent = nullptr;
2737+
if (auto *AT = dyn_cast<AssociatedTypeDecl>(GP)) {
2738+
parent = AT->getProtocol();
2739+
} else {
2740+
auto *dc = GP->getDeclContext();
2741+
parent = dc->getInnermostDeclarationDeclContext();
2742+
}
2743+
2744+
if (!parent)
2745+
return "";
2746+
2747+
llvm::SmallString<64> result;
2748+
llvm::raw_svector_ostream OS(result);
2749+
2750+
OS << Decl::getDescriptiveKindName(GP->getDescriptiveKind());
2751+
2752+
if (includeName && GP->hasName())
2753+
OS << " '" << GP->getBaseName() << "'";
2754+
2755+
OS << " of ";
2756+
OS << Decl::getDescriptiveKindName(parent->getDescriptiveKind());
2757+
if (auto *decl = dyn_cast<ValueDecl>(parent)) {
2758+
if (decl->hasName())
2759+
OS << " '" << decl->getFullName() << "'";
2760+
}
2761+
2762+
return OS.str();
2763+
}
2764+
2765+
/// Special handling of conflicts associated with generic arguments.
2766+
///
2767+
/// func foo<T>(_: T, _: T) {}
2768+
/// func bar(x: Int, y: Float) {
2769+
/// foo(x, y)
2770+
/// }
2771+
///
2772+
/// It's done by first retrieving all generic parameters from each solution,
2773+
/// filtering boundings into distrinct set and diagnosing any differences.
2774+
static bool diagnoseConflictingArguments(ConstraintSystem &cs,
2775+
const SolutionDiff &diff,
2776+
ArrayRef<Solution> solutions) {
2777+
if (!diff.overloads.empty())
2778+
return false;
2779+
2780+
if (!llvm::all_of(solutions, [](const Solution &solution) -> bool {
2781+
return llvm::all_of(
2782+
solution.Fixes, [](const ConstraintFix *fix) -> bool {
2783+
return fix->getKind() == FixKind::AllowArgumentTypeMismatch ||
2784+
fix->getKind() == FixKind::AllowFunctionTypeMismatch ||
2785+
fix->getKind() == FixKind::AllowTupleTypeMismatch;
2786+
});
2787+
}))
2788+
return false;
2789+
2790+
auto &DE = cs.getASTContext().Diags;
2791+
2792+
llvm::SmallDenseMap<TypeVariableType *, SmallVector<Type, 4>> conflicts;
2793+
2794+
for (const auto &binding : solutions[0].typeBindings) {
2795+
auto *typeVar = binding.first;
2796+
2797+
if (!typeVar->getImpl().getGenericParameter())
2798+
continue;
2799+
2800+
llvm::SmallSetVector<Type, 4> arguments;
2801+
arguments.insert(binding.second);
2802+
2803+
if (!llvm::all_of(solutions.slice(1), [&](const Solution &solution) {
2804+
auto binding = solution.typeBindings.find(typeVar);
2805+
if (binding == solution.typeBindings.end())
2806+
return false;
2807+
2808+
if (auto *opaque =
2809+
binding->second->getAs<OpaqueTypeArchetypeType>()) {
2810+
auto *decl = opaque->getDecl();
2811+
arguments.remove_if([&](Type argType) -> bool {
2812+
if (auto *otherOpaque =
2813+
argType->getAs<OpaqueTypeArchetypeType>()) {
2814+
return decl == otherOpaque->getDecl();
2815+
}
2816+
return false;
2817+
});
2818+
}
2819+
2820+
arguments.insert(binding->second);
2821+
return true;
2822+
}))
2823+
continue;
2824+
2825+
if (arguments.size() > 1) {
2826+
conflicts[typeVar].append(arguments.begin(), arguments.end());
2827+
}
2828+
}
2829+
2830+
auto getGenericTypeDecl = [&](ArchetypeType *archetype) -> ValueDecl * {
2831+
auto type = archetype->getInterfaceType();
2832+
2833+
if (auto *GTPT = type->getAs<GenericTypeParamType>())
2834+
return GTPT->getDecl();
2835+
2836+
if (auto *DMT = type->getAs<DependentMemberType>())
2837+
return DMT->getAssocType();
2838+
2839+
return nullptr;
2840+
};
2841+
2842+
bool diagnosed = false;
2843+
for (auto &conflict : conflicts) {
2844+
auto *typeVar = conflict.first;
2845+
auto *locator = typeVar->getImpl().getLocator();
2846+
auto conflictingArguments = conflict.second;
2847+
2848+
llvm::SmallString<64> arguments;
2849+
llvm::raw_svector_ostream OS(arguments);
2850+
2851+
interleave(
2852+
conflictingArguments,
2853+
[&](Type argType) {
2854+
OS << "'" << argType << "'";
2855+
2856+
if (auto *opaque = argType->getAs<OpaqueTypeArchetypeType>()) {
2857+
auto *decl = opaque->getDecl()->getNamingDecl();
2858+
OS << " (result type of '" << decl->getBaseName().userFacingName()
2859+
<< "')";
2860+
return;
2861+
}
2862+
2863+
if (auto archetype = argType->getAs<ArchetypeType>()) {
2864+
if (auto *GTD = getGenericTypeDecl(archetype))
2865+
OS << " (" << describeGenericType(GTD) << ")";
2866+
}
2867+
},
2868+
[&OS] { OS << " vs. "; });
2869+
2870+
auto *anchor = locator->getAnchor();
2871+
DE.diagnose(anchor->getLoc(),
2872+
diag::conflicting_arguments_for_generic_parameter,
2873+
typeVar->getImpl().getGenericParameter(), OS.str());
2874+
diagnosed = true;
2875+
}
2876+
2877+
return diagnosed;
2878+
}
2879+
27322880
bool ConstraintSystem::diagnoseAmbiguityWithFixes(
27332881
SmallVectorImpl<Solution> &solutions) {
27342882
if (solutions.empty())
@@ -2749,6 +2897,11 @@ bool ConstraintSystem::diagnoseAmbiguityWithFixes(
27492897
return false;
27502898
}
27512899

2900+
SolutionDiff solutionDiff(solutions);
2901+
2902+
if (diagnoseConflictingArguments(*this, solutionDiff, solutions))
2903+
return true;
2904+
27522905
// Collect aggregated fixes from all solutions
27532906
llvm::SmallMapVector<std::pair<ConstraintLocator *, FixKind>,
27542907
llvm::SmallVector<ConstraintFix *, 4>, 4> aggregatedFixes;
@@ -2763,7 +2916,6 @@ bool ConstraintSystem::diagnoseAmbiguityWithFixes(
27632916

27642917
// If there is an overload difference, let's see if there's a common callee
27652918
// locator for all of the fixes.
2766-
SolutionDiff solutionDiff(solutions);
27672919
auto ambiguousOverload = llvm::find_if(solutionDiff.overloads,
27682920
[&](const auto &overloadDiff) {
27692921
return llvm::all_of(aggregatedFixes, [&](const auto &aggregatedFix) {

‎lib/Sema/ConstraintSystem.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5115,6 +5115,8 @@ bool exprNeedsParensOutsideFollowingOperator(
51155115
/// Determine whether this is a SIMD operator.
51165116
bool isSIMDOperator(ValueDecl *value);
51175117

5118+
std::string describeGenericType(ValueDecl *GP, bool includeName = false);
5119+
51185120
/// Apply the given function builder transform within a specific solution
51195121
/// to produce the rewritten body.
51205122
///

‎test/Generics/deduction.swift

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ func useIdentity(_ x: Int, y: Float, i32: Int32) {
2828
xx = identity2(yy) // expected-error{{no exact matches in call to global function 'identity2'}}
2929
}
3030

31-
// FIXME: Crummy diagnostic!
3231
func twoIdentical<T>(_ x: T, _ y: T) -> T {}
3332

3433
func useTwoIdentical(_ xi: Int, yi: Float) {
@@ -40,7 +39,7 @@ func useTwoIdentical(_ xi: Int, yi: Float) {
4039
y = twoIdentical(1.0, y)
4140
y = twoIdentical(y, 1.0)
4241

43-
twoIdentical(x, y) // expected-error{{cannot convert value of type 'Float' to expected argument type 'Int'}}
42+
twoIdentical(x, y) // expected-error{{conflicting arguments to generic parameter 'T' ('Int' vs. 'Float')}}
4443
}
4544

4645
func mySwap<T>(_ x: inout T,
@@ -67,8 +66,9 @@ func takeTuples<T, U>(_: (T, U), _: (U, T)) {
6766
func useTuples(_ x: Int, y: Float, z: (Float, Int)) {
6867
takeTuples((x, y), (y, x))
6968

70-
takeTuples((x, y), (x, y)) // expected-error{{cannot convert value of type '(Int, Float)' to expected argument type '(Float, Int)'}}
71-
69+
takeTuples((x, y), (x, y))
70+
// expected-error@-1 {{conflicting arguments to generic parameter 'U' ('Float' vs. 'Int')}}
71+
// expected-error@-2 {{conflicting arguments to generic parameter 'T' ('Int' vs. 'Float')}}
7272
// FIXME: Use 'z', which requires us to fix our tuple-conversion
7373
// representation.
7474
}
@@ -315,7 +315,7 @@ struct A {}
315315
func foo() {
316316
for i in min(1,2) { // expected-error{{for-in loop requires 'Int' to conform to 'Sequence'}}
317317
}
318-
let j = min(Int(3), Float(2.5)) // expected-error{{cannot convert value of type 'Float' to expected argument type 'Int'}}
318+
let j = min(Int(3), Float(2.5)) // expected-error{{conflicting arguments to generic parameter 'T' ('Int' vs. 'Float')}}
319319
let k = min(A(), A()) // expected-error{{global function 'min' requires that 'A' conform to 'Comparable'}}
320320
let oi : Int? = 5
321321
let l = min(3, oi) // expected-error{{global function 'min' requires that 'Int?' conform to 'Comparable'}}

‎test/Sema/suppress-argument-labels-in-types.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,7 @@ class C0 {
203203
}
204204

205205
// Check diagnostics changes.
206-
let _ = min(Int(3), Float(2.5)) // expected-error{{cannot convert value of type 'Float' to expected argument type 'Int'}}
206+
let _ = min(Int(3), Float(2.5)) // expected-error{{conflicting arguments to generic parameter 'T' ('Int' vs. 'Float')}}
207207

208208
// SR-11429
209209
func testIntermediateCoercions() {

‎test/Sema/type_join.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -98,9 +98,9 @@ func joinFunctions(
9898
// Any to be inferred for the generic type. That's pretty
9999
// arbitrary.
100100
_ = commonSupertype(escaping, x)
101-
// expected-error@-1 {{cannot convert value of type 'Int' to expected argument type '() -> ()'}}
101+
// expected-error@-1 {{conflicting arguments to generic parameter 'T' ('() -> ()' vs. 'Int')}}
102102
_ = commonSupertype(x, escaping)
103-
// expected-error@-1 {{cannot convert value of type '() -> ()' to expected argument type 'Int'}}
103+
// expected-error@-1 {{conflicting arguments to generic parameter 'T' ('Int' vs. '() -> ()')}}
104104

105105
let a: Any = 1
106106
_ = commonSupertype(nonescaping, a)

‎test/decl/func/default-values.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ defaultArgTuplesNotMaterializable(identity(5))
9292

9393
// <rdar://problem/22333090> QoI: Propagate contextual information in a call to operands
9494
defaultArgTuplesNotMaterializable(identity((5, y: 10)))
95-
// expected-error@-1 {{cannot convert value of type '(Int, y: Int)' to expected argument type 'Int'}}
95+
// expected-error@-1 {{conflicting arguments to generic parameter 'T' ('(Int, y: Int)' vs. 'Int')}}
9696

9797

9898
// rdar://problem/21799331

0 commit comments

Comments
 (0)