Skip to content

Commit becbcf3

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 (cherry picked from commit 2420067)
1 parent 27ea284 commit becbcf3

File tree

8 files changed

+180
-45
lines changed

8 files changed

+180
-45
lines changed

‎include/swift/AST/DiagnosticsSema.def

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1387,6 +1387,14 @@ NOTE(type_does_not_inherit_requirement,none,
13871387
ERROR(types_not_equal,none,
13881388
"%0 requires the types %1 and %2 be equivalent",
13891389
(Type, Type, Type))
1390+
NOTE(candidate_types_equal_requirement,none,
1391+
"candidate requires that the types %0 and %1 be equivalent "
1392+
"(requirement specified as %2 == %3%4)",
1393+
(Type, Type, Type, Type, StringRef))
1394+
NOTE(candidate_types_inheritance_requirement,none,
1395+
"candidate requires that %1 inherit from %2 "
1396+
"(requirement specified as %2 : %3%4)",
1397+
(Type, Type, Type, Type, StringRef))
13901398
NOTE(types_not_equal_requirement,none,
13911399
"requirement specified as %0 == %1%2", (Type, Type, StringRef))
13921400
ERROR(non_class_cannot_conform_to_class_protocol,none,

‎lib/Sema/CSDiag.cpp

Lines changed: 92 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2152,8 +2152,8 @@ class FailureDiagnosis :public ASTVisitor<FailureDiagnosis, /*exprresult*/bool>{
21522152

21532153
/// Produce diagnostic for failures related to unfulfilled requirements
21542154
/// of the generic parameters used as arguments.
2155-
bool diagnoseArgumentGenericRequirements(TypeChecker &TC, Expr *fnExpr,
2156-
Expr *argExpr,
2155+
bool diagnoseArgumentGenericRequirements(TypeChecker &TC, Expr *callExpr,
2156+
Expr *fnExpr, Expr *argExpr,
21572157
CalleeCandidateInfo &candidates,
21582158
ArrayRef<Identifier> argLabels);
21592159

@@ -5886,16 +5886,18 @@ bool FailureDiagnosis::diagnoseMethodAttributeFailures(
58865886
}
58875887

58885888
bool FailureDiagnosis::diagnoseArgumentGenericRequirements(
5889-
TypeChecker &TC, Expr *fnExpr, Expr *argExpr,
5889+
TypeChecker &TC, Expr *callExpr, Expr *fnExpr, Expr *argExpr,
58905890
CalleeCandidateInfo &candidates, ArrayRef<Identifier> argLabels) {
58915891
if (candidates.closeness != CC_ExactMatch || candidates.size() != 1)
58925892
return false;
58935893

5894-
auto DRE = dyn_cast<DeclRefExpr>(fnExpr);
5895-
if (!DRE)
5896-
return false;
5894+
AbstractFunctionDecl *AFD = nullptr;
5895+
if (auto *DRE = dyn_cast<DeclRefExpr>(fnExpr)) {
5896+
AFD = dyn_cast<AbstractFunctionDecl>(DRE->getDecl());
5897+
} else if (auto *candidate = candidates[0].getDecl()) {
5898+
AFD = dyn_cast<AbstractFunctionDecl>(candidate);
5899+
}
58975900

5898-
auto AFD = dyn_cast<AbstractFunctionDecl>(DRE->getDecl());
58995901
if (!AFD || !AFD->isGeneric() || !AFD->hasInterfaceType())
59005902
return false;
59015903

@@ -5920,7 +5922,9 @@ bool FailureDiagnosis::diagnoseArgumentGenericRequirements(
59205922
// requirements e.g. <A, B where A.Element == B.Element>.
59215923
for (unsigned i = 0, e = bindings.size(); i != e; ++i) {
59225924
auto param = params[i];
5923-
auto archetype = param.Ty->getAs<ArchetypeType>();
5925+
auto paramType = param.Ty->getInOutObjectType();
5926+
5927+
auto archetype = paramType->getAs<ArchetypeType>();
59245928
if (!archetype)
59255929
continue;
59265930

@@ -5948,18 +5952,88 @@ bool FailureDiagnosis::diagnoseArgumentGenericRequirements(
59485952
return false;
59495953

59505954
class RequirementsListener : public GenericRequirementsCheckListener {
5955+
TypeChecker &TC;
5956+
AbstractFunctionDecl *Candidate;
5957+
TypeSubstitutionFn Substitutions;
5958+
5959+
Expr *CallExpr;
5960+
Expr *FnExpr;
5961+
Expr *ArgExpr;
5962+
59515963
public:
5964+
RequirementsListener(TypeChecker &TC, AbstractFunctionDecl *AFD,
5965+
TypeSubstitutionFn subs,
5966+
Expr *callExpr, Expr *fnExpr, Expr *argExpr)
5967+
: TC(TC), Candidate(AFD), Substitutions(subs), CallExpr(callExpr),
5968+
FnExpr(fnExpr), ArgExpr(argExpr) {}
5969+
59525970
bool shouldCheck(RequirementKind kind, Type first, Type second) override {
59535971
// This means that we have encountered requirement which references
59545972
// generic parameter not used in the arguments, we can't diagnose it here.
59555973
return !(first->hasTypeParameter() || first->isTypeVariableOrMember());
59565974
}
5975+
5976+
bool diagnoseUnsatisfiedRequirement(const Requirement &req, Type first,
5977+
Type second) override {
5978+
Diag<Type, Type, Type, Type, StringRef> note;
5979+
switch (req.getKind()) {
5980+
case RequirementKind::Conformance:
5981+
case RequirementKind::Layout:
5982+
return false;
5983+
5984+
case RequirementKind::Superclass:
5985+
note = diag::candidate_types_inheritance_requirement;
5986+
break;
5987+
5988+
case RequirementKind::SameType:
5989+
note = diag::candidate_types_equal_requirement;
5990+
break;
5991+
}
5992+
5993+
SmallVector<char, 8> scratch;
5994+
auto overloadName = Candidate->getFullName().getString(scratch);
5995+
5996+
if (isa<BinaryExpr>(CallExpr) && isa<TupleExpr>(ArgExpr)) {
5997+
auto argTuple = cast<TupleExpr>(ArgExpr);
5998+
auto lhsExpr = argTuple->getElement(0),
5999+
rhsExpr = argTuple->getElement(1);
6000+
auto lhsType = lhsExpr->getType()->getRValueType();
6001+
auto rhsType = rhsExpr->getType()->getRValueType();
6002+
6003+
TC.diagnose(FnExpr->getLoc(), diag::cannot_apply_binop_to_args,
6004+
overloadName, lhsType, rhsType)
6005+
.highlight(lhsExpr->getSourceRange())
6006+
.highlight(rhsExpr->getSourceRange());
6007+
} else if (isa<PrefixUnaryExpr>(CallExpr) ||
6008+
isa<PostfixUnaryExpr>(CallExpr)) {
6009+
TC.diagnose(ArgExpr->getLoc(), diag::cannot_apply_unop_to_arg,
6010+
overloadName, ArgExpr->getType());
6011+
} else {
6012+
bool isInitializer = isa<ConstructorDecl>(Candidate);
6013+
TC.diagnose(ArgExpr->getLoc(), diag::cannot_call_with_params,
6014+
overloadName, getTypeListString(ArgExpr->getType()),
6015+
isInitializer);
6016+
}
6017+
6018+
auto rawFirstType = req.getFirstType();
6019+
auto rawSecondType = req.getSecondType();
6020+
auto *genericSig = Candidate->getGenericSignature();
6021+
6022+
TC.diagnose(Candidate, note, first, second,
6023+
rawFirstType, rawSecondType,
6024+
genericSig->gatherGenericParamBindingsText(
6025+
{rawFirstType, rawSecondType}, Substitutions));
6026+
return true;
6027+
}
59576028
};
59586029

5959-
auto dc = env->getOwningDeclContext();
5960-
RequirementsListener genericReqListener;
6030+
auto *dc = env->getOwningDeclContext();
6031+
auto substitutionFn = QueryTypeSubstitutionMap{substitutions};
6032+
RequirementsListener genericReqListener(TC, AFD, substitutionFn,
6033+
callExpr, fnExpr, argExpr);
6034+
59616035
auto result = TC.checkGenericArguments(
5962-
dc, argExpr->getLoc(), AFD->getLoc(), AFD->getInterfaceType(),
6036+
dc, callExpr->getLoc(), fnExpr->getLoc(), AFD->getInterfaceType(),
59636037
env->getGenericSignature(), QueryTypeSubstitutionMap{substitutions},
59646038
LookUpConformanceInModule{dc->getParentModule()}, nullptr,
59656039
ConformanceCheckFlags::SuppressDependencyTracking, &genericReqListener);
@@ -6662,6 +6736,10 @@ bool FailureDiagnosis::visitApplyExpr(ApplyExpr *callExpr) {
66626736
}
66636737
}
66646738

6739+
if (diagnoseArgumentGenericRequirements(CS->TC, callExpr, fnExpr, argExpr,
6740+
calleeInfo, argLabels))
6741+
return true;
6742+
66656743
if (isContextualConversionFailure(argTuple))
66666744
return false;
66676745

@@ -6695,13 +6773,13 @@ bool FailureDiagnosis::visitApplyExpr(ApplyExpr *callExpr) {
66956773
return true;
66966774
}
66976775

6698-
// If all of the arguments are a perfect match, so let's check if there
6776+
// If all of the arguments are a perfect match, let's check if there
66996777
// are problems with requirements placed on generic parameters, because
67006778
// CalleeCandidateInfo validates only conformance of the parameters
67016779
// to their protocol types (if any) but it doesn't check additional
67026780
// requirements placed on e.g. nested types or between parameters.
6703-
if (diagnoseArgumentGenericRequirements(CS->TC, fnExpr, argExpr, calleeInfo,
6704-
argLabels))
6781+
if (diagnoseArgumentGenericRequirements(CS->TC, callExpr, fnExpr, argExpr,
6782+
calleeInfo, argLabels))
67056783
return true;
67066784

67076785
if (isContextualConversionFailure(argExpr))

‎lib/Sema/TypeCheckConstraints.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1639,6 +1639,11 @@ void GenericRequirementsCheckListener::satisfiedConformance(
16391639
ProtocolConformanceRef conformance) {
16401640
}
16411641

1642+
bool GenericRequirementsCheckListener::diagnoseUnsatisfiedRequirement(
1643+
const Requirement &req, Type first, Type second) {
1644+
return false;
1645+
}
1646+
16421647
bool TypeChecker::
16431648
solveForExpression(Expr *&expr, DeclContext *dc, Type convertType,
16441649
FreeTypeVariableBinding allowFreeTypeVariables,

‎lib/Sema/TypeCheckGeneric.cpp

Lines changed: 30 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1300,9 +1300,13 @@ RequirementCheckResult TypeChecker::checkGenericArguments(
13001300
secondType = req->getSecondType();
13011301
}
13021302

1303+
bool requirementFailure = false;
13031304
if (listener && !listener->shouldCheck(kind, firstType, secondType))
13041305
continue;
13051306

1307+
Diag<Type, Type, Type> diagnostic;
1308+
Diag<Type, Type, StringRef> diagnosticNote;
1309+
13061310
switch (kind) {
13071311
case RequirementKind::Conformance: {
13081312
// Protocol conformance requirements.
@@ -1311,6 +1315,8 @@ RequirementCheckResult TypeChecker::checkGenericArguments(
13111315
// or non-private dependency.
13121316
// FIXME: Do we really need "used" at this point?
13131317
// FIXME: Poor location information. How much better can we do here?
1318+
// FIXME: This call should support listener to be able to properly
1319+
// diagnose problems with conformances.
13141320
auto result =
13151321
conformsToProtocol(firstType, proto->getDecl(), dc,
13161322
conformanceOptions, loc, unsatisfiedDependency);
@@ -1343,37 +1349,37 @@ RequirementCheckResult TypeChecker::checkGenericArguments(
13431349
case RequirementKind::Superclass:
13441350
// Superclass requirements.
13451351
if (!isSubclassOf(firstType, secondType, dc)) {
1346-
if (loc.isValid()) {
1347-
// FIXME: Poor source-location information.
1348-
diagnose(loc, diag::type_does_not_inherit, owner, firstType,
1349-
secondType);
1350-
1351-
diagnose(noteLoc, diag::type_does_not_inherit_requirement,
1352-
rawFirstType, rawSecondType,
1353-
genericSig->gatherGenericParamBindingsText(
1354-
{rawFirstType, rawSecondType}, substitutions));
1355-
}
1356-
1357-
return RequirementCheckResult::Failure;
1352+
diagnostic = diag::type_does_not_inherit;
1353+
diagnosticNote = diag::type_does_not_inherit_requirement;
1354+
requirementFailure = true;
13581355
}
1359-
continue;
1356+
break;
13601357

13611358
case RequirementKind::SameType:
13621359
if (!firstType->isEqual(secondType)) {
1363-
if (loc.isValid()) {
1364-
// FIXME: Better location info for both diagnostics.
1365-
diagnose(loc, diag::types_not_equal, owner, firstType, secondType);
1366-
1367-
diagnose(noteLoc, diag::types_not_equal_requirement, rawFirstType,
1368-
rawSecondType,
1369-
genericSig->gatherGenericParamBindingsText(
1370-
{rawFirstType, rawSecondType}, substitutions));
1371-
}
1372-
1373-
return RequirementCheckResult::Failure;
1360+
diagnostic = diag::types_not_equal;
1361+
diagnosticNote = diag::types_not_equal_requirement;
1362+
requirementFailure = true;
13741363
}
1364+
break;
1365+
}
1366+
1367+
if (!requirementFailure)
13751368
continue;
1369+
1370+
if (listener &&
1371+
listener->diagnoseUnsatisfiedRequirement(rawReq, firstType, secondType))
1372+
return RequirementCheckResult::Failure;
1373+
1374+
if (loc.isValid()) {
1375+
// FIXME: Poor source-location information.
1376+
diagnose(loc, diagnostic, owner, firstType, secondType);
1377+
diagnose(noteLoc, diagnosticNote, rawFirstType, rawSecondType,
1378+
genericSig->gatherGenericParamBindingsText(
1379+
{rawFirstType, rawSecondType}, substitutions));
13761380
}
1381+
1382+
return RequirementCheckResult::Failure;
13771383
}
13781384

13791385
if (valid)

‎lib/Sema/TypeChecker.h

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

377391
/// 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)