Skip to content

Commit 2420067

Browse files
xedinxedin
committed
[QoI] Improve diagnostics of unsatisfied generic requirements
Fixes a problem related to presence of InOutType in function parameters which diagnostics related to generic parameter requirements didn't handle correctly, and improves diagnostics for unsatisfied generic requirements in operator applications, which we didn't attempt to diagnose at all. Resolves: rdar://problem/33477726
1 parent d018ecc commit 2420067

File tree

8 files changed

+185
-46
lines changed

8 files changed

+185
-46
lines changed

‎include/swift/AST/DiagnosticsSema.def

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1378,6 +1378,14 @@ NOTE(type_does_not_inherit_requirement,none,
13781378
ERROR(types_not_equal,none,
13791379
"%0 requires the types %1 and %2 be equivalent",
13801380
(Type, Type, Type))
1381+
NOTE(candidate_types_equal_requirement,none,
1382+
"candidate requires that the types %0 and %1 be equivalent "
1383+
"(requirement specified as %2 == %3%4)",
1384+
(Type, Type, Type, Type, StringRef))
1385+
NOTE(candidate_types_inheritance_requirement,none,
1386+
"candidate requires that %1 inherit from %2 "
1387+
"(requirement specified as %2 : %3%4)",
1388+
(Type, Type, Type, Type, StringRef))
13811389
NOTE(types_not_equal_requirement,none,
13821390
"requirement specified as %0 == %1%2", (Type, Type, StringRef))
13831391
ERROR(non_class_cannot_conform_to_class_protocol,none,

‎lib/Sema/CSDiag.cpp

Lines changed: 97 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2143,8 +2143,8 @@ class FailureDiagnosis :public ASTVisitor<FailureDiagnosis, /*exprresult*/bool>{
21432143

21442144
/// Produce diagnostic for failures related to unfulfilled requirements
21452145
/// of the generic parameters used as arguments.
2146-
bool diagnoseArgumentGenericRequirements(TypeChecker &TC, Expr *fnExpr,
2147-
Expr *argExpr,
2146+
bool diagnoseArgumentGenericRequirements(TypeChecker &TC, Expr *callExpr,
2147+
Expr *fnExpr, Expr *argExpr,
21482148
CalleeCandidateInfo &candidates,
21492149
ArrayRef<Identifier> argLabels);
21502150

@@ -5914,16 +5914,18 @@ bool FailureDiagnosis::diagnoseMethodAttributeFailures(
59145914
}
59155915

59165916
bool FailureDiagnosis::diagnoseArgumentGenericRequirements(
5917-
TypeChecker &TC, Expr *fnExpr, Expr *argExpr,
5917+
TypeChecker &TC, Expr *callExpr, Expr *fnExpr, Expr *argExpr,
59185918
CalleeCandidateInfo &candidates, ArrayRef<Identifier> argLabels) {
59195919
if (candidates.closeness != CC_ExactMatch || candidates.size() != 1)
59205920
return false;
59215921

5922-
auto DRE = dyn_cast<DeclRefExpr>(fnExpr);
5923-
if (!DRE)
5924-
return false;
5922+
AbstractFunctionDecl *AFD = nullptr;
5923+
if (auto *DRE = dyn_cast<DeclRefExpr>(fnExpr)) {
5924+
AFD = dyn_cast<AbstractFunctionDecl>(DRE->getDecl());
5925+
} else if (auto *candidate = candidates[0].getDecl()) {
5926+
AFD = dyn_cast<AbstractFunctionDecl>(candidate);
5927+
}
59255928

5926-
auto AFD = dyn_cast<AbstractFunctionDecl>(DRE->getDecl());
59275929
if (!AFD || !AFD->isGeneric() || !AFD->hasInterfaceType())
59285930
return false;
59295931

@@ -5952,14 +5954,19 @@ bool FailureDiagnosis::diagnoseArgumentGenericRequirements(
59525954
// requirements e.g. <A, B where A.Element == B.Element>.
59535955
for (unsigned i = 0, e = bindings.size(); i != e; ++i) {
59545956
auto param = params[i];
5955-
auto archetype = param.getType()->getAs<ArchetypeType>();
5957+
auto paramType = param.getType()->getInOutObjectType();
5958+
5959+
auto archetype = paramType->getAs<ArchetypeType>();
59565960
if (!archetype)
59575961
continue;
59585962

59595963
// Bindings specify the arguments that source the parameter. The only case
59605964
// this returns a non-singular value is when there are varargs in play.
59615965
for (auto argNo : bindings[i]) {
5962-
auto argType = args[argNo].getType()->getWithoutSpecifierType();
5966+
auto argType = args[argNo]
5967+
.getType()
5968+
->getWithoutSpecifierType()
5969+
->getRValueObjectType();
59635970

59645971
if (argType->is<ArchetypeType>()) {
59655972
diagnoseUnboundArchetype(archetype, fnExpr);
@@ -5980,18 +5987,89 @@ bool FailureDiagnosis::diagnoseArgumentGenericRequirements(
59805987
return false;
59815988

59825989
class RequirementsListener : public GenericRequirementsCheckListener {
5990+
ConstraintSystem &CS;
5991+
AbstractFunctionDecl *Candidate;
5992+
TypeSubstitutionFn Substitutions;
5993+
5994+
Expr *CallExpr;
5995+
Expr *FnExpr;
5996+
Expr *ArgExpr;
5997+
59835998
public:
5999+
RequirementsListener(ConstraintSystem &cs, AbstractFunctionDecl *AFD,
6000+
TypeSubstitutionFn subs,
6001+
Expr *callExpr, Expr *fnExpr, Expr *argExpr)
6002+
: CS(cs), Candidate(AFD), Substitutions(subs), CallExpr(callExpr),
6003+
FnExpr(fnExpr), ArgExpr(argExpr) {}
6004+
59846005
bool shouldCheck(RequirementKind kind, Type first, Type second) override {
59856006
// This means that we have encountered requirement which references
59866007
// generic parameter not used in the arguments, we can't diagnose it here.
59876008
return !(first->hasTypeParameter() || first->isTypeVariableOrMember());
59886009
}
6010+
6011+
bool diagnoseUnsatisfiedRequirement(const Requirement &req, Type first,
6012+
Type second) override {
6013+
Diag<Type, Type, Type, Type, StringRef> note;
6014+
switch (req.getKind()) {
6015+
case RequirementKind::Conformance:
6016+
case RequirementKind::Layout:
6017+
return false;
6018+
6019+
case RequirementKind::Superclass:
6020+
note = diag::candidate_types_inheritance_requirement;
6021+
break;
6022+
6023+
case RequirementKind::SameType:
6024+
note = diag::candidate_types_equal_requirement;
6025+
break;
6026+
}
6027+
6028+
TypeChecker &TC = CS.TC;
6029+
SmallVector<char, 8> scratch;
6030+
auto overloadName = Candidate->getFullName().getString(scratch);
6031+
6032+
if (isa<BinaryExpr>(CallExpr) && isa<TupleExpr>(ArgExpr)) {
6033+
auto argTuple = cast<TupleExpr>(ArgExpr);
6034+
auto lhsExpr = argTuple->getElement(0),
6035+
rhsExpr = argTuple->getElement(1);
6036+
auto lhsType = CS.getType(lhsExpr)->getRValueType();
6037+
auto rhsType = CS.getType(rhsExpr)->getRValueType();
6038+
6039+
TC.diagnose(FnExpr->getLoc(), diag::cannot_apply_binop_to_args,
6040+
overloadName, lhsType, rhsType)
6041+
.highlight(lhsExpr->getSourceRange())
6042+
.highlight(rhsExpr->getSourceRange());
6043+
} else if (isa<PrefixUnaryExpr>(CallExpr) ||
6044+
isa<PostfixUnaryExpr>(CallExpr)) {
6045+
TC.diagnose(ArgExpr->getLoc(), diag::cannot_apply_unop_to_arg,
6046+
overloadName, CS.getType(ArgExpr));
6047+
} else {
6048+
bool isInitializer = isa<ConstructorDecl>(Candidate);
6049+
TC.diagnose(ArgExpr->getLoc(), diag::cannot_call_with_params,
6050+
overloadName, getTypeListString(CS.getType(ArgExpr)),
6051+
isInitializer);
6052+
}
6053+
6054+
auto rawFirstType = req.getFirstType();
6055+
auto rawSecondType = req.getSecondType();
6056+
auto *genericSig = Candidate->getGenericSignature();
6057+
6058+
TC.diagnose(Candidate, note, first, second,
6059+
rawFirstType, rawSecondType,
6060+
genericSig->gatherGenericParamBindingsText(
6061+
{rawFirstType, rawSecondType}, Substitutions));
6062+
return true;
6063+
}
59896064
};
59906065

5991-
auto dc = env->getOwningDeclContext();
5992-
RequirementsListener genericReqListener;
6066+
auto *dc = env->getOwningDeclContext();
6067+
auto substitutionFn = QueryTypeSubstitutionMap{substitutions};
6068+
RequirementsListener genericReqListener(CS, AFD, substitutionFn,
6069+
callExpr, fnExpr, argExpr);
6070+
59936071
auto result = TC.checkGenericArguments(
5994-
dc, argExpr->getLoc(), AFD->getLoc(), AFD->getInterfaceType(),
6072+
dc, callExpr->getLoc(), fnExpr->getLoc(), AFD->getInterfaceType(),
59956073
env->getGenericSignature(), QueryTypeSubstitutionMap{substitutions},
59966074
LookUpConformanceInModule{dc->getParentModule()}, nullptr,
59976075
ConformanceCheckFlags::SuppressDependencyTracking, &genericReqListener);
@@ -6682,6 +6760,10 @@ bool FailureDiagnosis::visitApplyExpr(ApplyExpr *callExpr) {
66826760
}
66836761
}
66846762

6763+
if (diagnoseArgumentGenericRequirements(CS.TC, callExpr, fnExpr, argExpr,
6764+
calleeInfo, argLabels))
6765+
return true;
6766+
66856767
if (isContextualConversionFailure(argTuple))
66866768
return false;
66876769

@@ -6715,13 +6797,13 @@ bool FailureDiagnosis::visitApplyExpr(ApplyExpr *callExpr) {
67156797
return true;
67166798
}
67176799

6718-
// If all of the arguments are a perfect match, so let's check if there
6800+
// If all of the arguments are a perfect match, let's check if there
67196801
// are problems with requirements placed on generic parameters, because
67206802
// CalleeCandidateInfo validates only conformance of the parameters
67216803
// to their protocol types (if any) but it doesn't check additional
67226804
// requirements placed on e.g. nested types or between parameters.
6723-
if (diagnoseArgumentGenericRequirements(CS.TC, fnExpr, argExpr, calleeInfo,
6724-
argLabels))
6805+
if (diagnoseArgumentGenericRequirements(CS.TC, callExpr, fnExpr, argExpr,
6806+
calleeInfo, argLabels))
67256807
return true;
67266808

67276809
if (isContextualConversionFailure(argExpr))

‎lib/Sema/TypeCheckConstraints.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1644,6 +1644,11 @@ void GenericRequirementsCheckListener::satisfiedConformance(
16441644
ProtocolConformanceRef conformance) {
16451645
}
16461646

1647+
bool GenericRequirementsCheckListener::diagnoseUnsatisfiedRequirement(
1648+
const Requirement &req, Type first, Type second) {
1649+
return false;
1650+
}
1651+
16471652
bool TypeChecker::
16481653
solveForExpression(Expr *&expr, DeclContext *dc, Type convertType,
16491654
FreeTypeVariableBinding allowFreeTypeVariables,

‎lib/Sema/TypeCheckGeneric.cpp

Lines changed: 30 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1231,9 +1231,13 @@ RequirementCheckResult TypeChecker::checkGenericArguments(
12311231
secondType = req->getSecondType();
12321232
}
12331233

1234+
bool requirementFailure = false;
12341235
if (listener && !listener->shouldCheck(kind, firstType, secondType))
12351236
continue;
12361237

1238+
Diag<Type, Type, Type> diagnostic;
1239+
Diag<Type, Type, StringRef> diagnosticNote;
1240+
12371241
switch (kind) {
12381242
case RequirementKind::Conformance: {
12391243
// Protocol conformance requirements.
@@ -1242,6 +1246,8 @@ RequirementCheckResult TypeChecker::checkGenericArguments(
12421246
// or non-private dependency.
12431247
// FIXME: Do we really need "used" at this point?
12441248
// FIXME: Poor location information. How much better can we do here?
1249+
// FIXME: This call should support listener to be able to properly
1250+
// diagnose problems with conformances.
12451251
auto result =
12461252
conformsToProtocol(firstType, proto->getDecl(), dc,
12471253
conformanceOptions, loc, unsatisfiedDependency);
@@ -1274,37 +1280,37 @@ RequirementCheckResult TypeChecker::checkGenericArguments(
12741280
case RequirementKind::Superclass:
12751281
// Superclass requirements.
12761282
if (!isSubclassOf(firstType, secondType, dc)) {
1277-
if (loc.isValid()) {
1278-
// FIXME: Poor source-location information.
1279-
diagnose(loc, diag::type_does_not_inherit, owner, firstType,
1280-
secondType);
1281-
1282-
diagnose(noteLoc, diag::type_does_not_inherit_requirement,
1283-
rawFirstType, rawSecondType,
1284-
genericSig->gatherGenericParamBindingsText(
1285-
{rawFirstType, rawSecondType}, substitutions));
1286-
}
1287-
1288-
return RequirementCheckResult::Failure;
1283+
diagnostic = diag::type_does_not_inherit;
1284+
diagnosticNote = diag::type_does_not_inherit_requirement;
1285+
requirementFailure = true;
12891286
}
1290-
continue;
1287+
break;
12911288

12921289
case RequirementKind::SameType:
12931290
if (!firstType->isEqual(secondType)) {
1294-
if (loc.isValid()) {
1295-
// FIXME: Better location info for both diagnostics.
1296-
diagnose(loc, diag::types_not_equal, owner, firstType, secondType);
1297-
1298-
diagnose(noteLoc, diag::types_not_equal_requirement, rawFirstType,
1299-
rawSecondType,
1300-
genericSig->gatherGenericParamBindingsText(
1301-
{rawFirstType, rawSecondType}, substitutions));
1302-
}
1303-
1304-
return RequirementCheckResult::Failure;
1291+
diagnostic = diag::types_not_equal;
1292+
diagnosticNote = diag::types_not_equal_requirement;
1293+
requirementFailure = true;
13051294
}
1295+
break;
1296+
}
1297+
1298+
if (!requirementFailure)
13061299
continue;
1300+
1301+
if (listener &&
1302+
listener->diagnoseUnsatisfiedRequirement(rawReq, firstType, secondType))
1303+
return RequirementCheckResult::Failure;
1304+
1305+
if (loc.isValid()) {
1306+
// FIXME: Poor source-location information.
1307+
diagnose(loc, diagnostic, owner, firstType, secondType);
1308+
diagnose(noteLoc, diagnosticNote, rawFirstType, rawSecondType,
1309+
genericSig->gatherGenericParamBindingsText(
1310+
{rawFirstType, rawSecondType}, substitutions));
13071311
}
1312+
1313+
return RequirementCheckResult::Failure;
13081314
}
13091315

13101316
if (valid)

‎lib/Sema/TypeChecker.h

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -375,6 +375,20 @@ class GenericRequirementsCheckListener {
375375
/// \param conformance The conformance itself.
376376
virtual void satisfiedConformance(Type depTy, Type replacementTy,
377377
ProtocolConformanceRef conformance);
378+
379+
/// Callback to diagnose problem with unsatisfied generic requirement.
380+
///
381+
/// \param req The unsatisfied generic requirement.
382+
///
383+
/// \param first The left-hand side type assigned to the requirement,
384+
/// possibly represented by its generic substitute.
385+
///
386+
/// \param second The right-hand side type assigned to the requirement,
387+
/// possibly represented by its generic substitute.
388+
///
389+
/// \returns true if problem has been diagnosed, false otherwise.
390+
virtual bool diagnoseUnsatisfiedRequirement(const Requirement &req,
391+
Type first, Type second);
378392
};
379393

380394
/// The result of `checkGenericRequirement`.

‎test/Constraints/generics.swift

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -248,10 +248,12 @@ struct S27515965 : P27515965 {
248248

249249
struct V27515965 {
250250
init<T : P27515965>(_ tp: T) where T.R == Float {}
251+
// expected-note@-1 {{candidate requires that the types 'Any' and 'Float' be equivalent (requirement specified as 'T.R' == 'Float' [with T = S27515965])}}
251252
}
252253

253254
func test(x: S27515965) -> V27515965 {
254-
return V27515965(x) // expected-error {{generic parameter 'T' could not be inferred}}
255+
return V27515965(x)
256+
// expected-error@-1 {{cannot invoke initializer for type 'init(_:)' with an argument list of type '(S27515965)'}}
255257
}
256258

257259
protocol BaseProto {}

‎test/Generics/deduction.swift

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -216,11 +216,12 @@ func callRangeOfIsBefore(_ ia: [Int], da: [Double]) {
216216
}
217217

218218
func testEqualIterElementTypes<A: IteratorProtocol, B: IteratorProtocol>(_ a: A, _ b: B) where A.Element == B.Element {}
219-
// expected-note@-1 {{requirement specified as 'A.Element' == 'B.Element' [with A = IndexingIterator<[Int]>, B = IndexingIterator<[Double]>]}}
219+
// expected-note@-1 {{candidate requires that the types 'Int' and 'Double' be equivalent (requirement specified as 'A.Element' == 'B.Element' [with A = IndexingIterator<[Int]>, B = IndexingIterator<[Double]>])}}
220220
func compareIterators() {
221221
var a: [Int] = []
222222
var b: [Double] = []
223-
testEqualIterElementTypes(a.makeIterator(), b.makeIterator()) // expected-error {{'<A, B where A : IteratorProtocol, B : IteratorProtocol, A.Element == B.Element> (A, B) -> ()' requires the types 'Int' and 'Double' be equivalent}}
223+
testEqualIterElementTypes(a.makeIterator(), b.makeIterator())
224+
// expected-error@-1 {{cannot invoke 'testEqualIterElementTypes(_:_:)' with an argument list of type '(IndexingIterator<[Int]>, IndexingIterator<[Double]>)'}}
224225
}
225226

226227
protocol P_GI {
@@ -233,9 +234,9 @@ class C_GI : P_GI {
233234

234235
class GI_Diff {}
235236
func genericInheritsA<T>(_ x: T) where T : P_GI, T.Y : GI_Diff {}
236-
// expected-note@-1 {{requirement specified as 'T.Y' : 'GI_Diff' [with T = C_GI]}}
237-
genericInheritsA(C_GI()) // expected-error {{<T where T : P_GI, T.Y : GI_Diff> (T) -> ()' requires that 'C_GI.Y' (aka 'Double') inherit from 'GI_Diff'}}
238-
237+
// expected-note@-1 {{candidate requires that 'GI_Diff' inherit from 'T.Y' (requirement specified as 'T.Y' : 'GI_Diff' [with T = C_GI])}}
238+
genericInheritsA(C_GI())
239+
// expected-error@-1 {{cannot invoke 'genericInheritsA(_:)' with an argument list of type '(C_GI)'}}
239240

240241
//===----------------------------------------------------------------------===//
241242
// Deduction for member operators
@@ -318,3 +319,24 @@ func foo() {
318319
let l = min(3, oi) // expected-error{{value of optional type 'Int?' not unwrapped; did you mean to use '!' or '?'?}}
319320
}
320321

322+
infix operator +&
323+
func +&<R, S>(lhs: inout R, rhs: S) where R : RangeReplaceableCollection, S : Sequence, R.Element == S.Element {}
324+
// expected-note@-1 {{candidate requires that the types 'String' and 'String.Element' (aka 'Character') be equivalent (requirement specified as 'R.Element' == 'S.Element' [with R = [String], S = String])}}
325+
326+
func rdar33477726_1() {
327+
var arr: [String] = []
328+
arr +& "hello"
329+
// expected-error@-1 {{binary operator '+&(_:_:)' cannot be applied to operands of type '[String]' and 'String'}}
330+
}
331+
332+
func rdar33477726_2<R, S>(_: R, _: S) where R: Sequence, S == R.Element {}
333+
// expected-note@-1 {{candidate requires that the types 'Int' and 'String.Element' (aka 'Character') be equivalent (requirement specified as 'S' == 'R.Element' [with R = String, S = Int])}}
334+
rdar33477726_2("answer", 42)
335+
// expected-error@-1 {{cannot invoke 'rdar33477726_2(_:_:)' with an argument list of type '(String, Int)'}}
336+
337+
prefix operator +-
338+
prefix func +-<T>(_: T) where T: Sequence, T.Element == Int {}
339+
// expected-note@-1 {{candidate requires that the types 'String.Element' (aka 'Character') and 'Int' be equivalent (requirement specified as 'T.Element' == 'Int' [with T = String])}}
340+
341+
+-"hello"
342+
// expected-error@-1 {{unary operator '+-(_:)' cannot be applied to an operand of type 'String'}}

‎test/decl/protocol/req/recursion.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ public struct S<A: P> where A.T == S<A> {
4848
// expected-error@-2 {{generic struct 'S' references itself}}
4949
func f(a: A.T) {
5050
g(a: id(t: a))
51-
// expected-error@-1 {{cannot convert value of type 'A.T' to expected argument type 'S<_>'}}
51+
// expected-error@-1 {{generic parameter 'T' could not be inferred}}
5252
_ = A.T.self
5353
}
5454

0 commit comments

Comments
 (0)