Skip to content

Commit 599be90

Browse files
committed
[QoI] Refactor TypeChecker::checkGenericArguments. NFC
Refactor TypeChecker::checkGenericArguments to enable it to be used by FailureDiagnosis::diagnoseArgumentGenericRequirements, which consolidates requirement checking in one place.
1 parent 0d0dc79 commit 599be90

File tree

4 files changed

+94
-81
lines changed

4 files changed

+94
-81
lines changed

lib/Sema/CSDiag.cpp

Lines changed: 19 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -5704,9 +5704,6 @@ bool FailureDiagnosis::diagnoseArgumentGenericRequirements(
57045704
/*allowFixes=*/false, listener, bindings))
57055705
return false;
57065706

5707-
auto loc = argExpr->getLoc();
5708-
auto noteLoc = AFD->getLoc();
5709-
57105707
TypeSubstitutionMap substitutions;
57115708
// First, let's collect all of the archetypes and their substitutions,
57125709
// that's going to help later on if there are cross-archetype
@@ -5740,72 +5737,32 @@ bool FailureDiagnosis::diagnoseArgumentGenericRequirements(
57405737
if (substitutions.empty())
57415738
return false;
57425739

5743-
auto dc = env->getOwningDeclContext();
5744-
auto module = dc->getParentModule();
5745-
auto signature = env->getGenericSignature();
5746-
5747-
for (const auto &req : signature->getRequirements()) {
5748-
Type firstType = req.getFirstType().subst(module, substitutions);
5749-
if (firstType.isNull())
5750-
continue;
5740+
class RequirementsListener : public GenericRequirementsCheckListener {
5741+
private:
5742+
bool DiagnosedAny = false;
57515743

5752-
Type secondType = req.getSecondType();
5753-
if (secondType) {
5754-
secondType = secondType.subst(module, substitutions);
5755-
if (secondType.isNull())
5756-
continue;
5744+
public:
5745+
bool shouldCheck(RequirementKind kind, Type first, Type second) override {
5746+
// This means that we have encountered requirement which references
5747+
// generic parameter not used in the arguments, we can't diagnose it here.
5748+
return !(first->hasTypeParameter() || first->isTypeVariableOrMember());
57575749
}
57585750

5759-
// This means that we have encountered requirement which references
5760-
// generic parameter not used in the arguments, we can't diagnose it here.
5761-
if (firstType->hasTypeParameter() || firstType->isTypeVariableOrMember())
5762-
continue;
5763-
5764-
switch (req.getKind()) {
5765-
case RequirementKind::Conformance: {
5766-
auto protocol = secondType->castTo<ProtocolType>();
5767-
auto result = TC.conformsToProtocol(
5768-
firstType, protocol->getDecl(), dc,
5769-
ConformanceCheckFlags::SuppressDependencyTracking, loc, nullptr);
5770-
5771-
// Conformance failed and was diagnosed by `conformsToProtocol`.
5772-
if (!result.second)
5773-
return true;
5774-
5775-
break;
5751+
void diagnosed(const Requirement *requirement) override {
5752+
DiagnosedAny = true;
57765753
}
57775754

5778-
case RequirementKind::Superclass:
5779-
if (!TC.isSubtypeOf(firstType, secondType, dc)) {
5780-
TC.diagnose(loc, diag::type_does_not_inherit, AFD->getInterfaceType(),
5781-
firstType, secondType);
5782-
5783-
TC.diagnose(
5784-
noteLoc, diag::type_does_not_inherit_requirement,
5785-
req.getFirstType(), req.getSecondType(),
5786-
signature->gatherGenericParamBindingsText(
5787-
{req.getFirstType(), req.getSecondType()}, substitutions));
5788-
return true;
5789-
}
5790-
break;
5791-
5792-
case RequirementKind::SameType:
5793-
if (!firstType->isEqual(secondType)) {
5794-
TC.diagnose(loc, diag::types_not_equal, AFD->getInterfaceType(),
5795-
firstType, secondType);
5755+
bool foundProblems() const { return DiagnosedAny; }
5756+
};
57965757

5797-
TC.diagnose(
5798-
noteLoc, diag::types_not_equal_requirement, req.getFirstType(),
5799-
req.getSecondType(),
5800-
signature->gatherGenericParamBindingsText(
5801-
{req.getFirstType(), req.getSecondType()}, substitutions));
5802-
return true;
5803-
}
5804-
break;
5805-
}
5806-
}
5758+
RequirementsListener genericReqListener;
5759+
TC.checkGenericArguments(env->getOwningDeclContext(), argExpr->getLoc(),
5760+
AFD->getLoc(), AFD->getInterfaceType(),
5761+
env->getGenericSignature(), substitutions, nullptr,
5762+
ConformanceCheckFlags::SuppressDependencyTracking,
5763+
&genericReqListener);
58075764

5808-
return false;
5765+
return genericReqListener.foundProblems();
58095766
}
58105767

58115768
/// When initializing Unsafe[Mutable]Pointer<T> from Unsafe[Mutable]RawPointer,

lib/Sema/TypeCheckConstraints.cpp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1375,6 +1375,16 @@ Expr *ExprTypeCheckListener::appliedSolution(Solution &solution, Expr *expr) {
13751375
return expr;
13761376
}
13771377

1378+
GenericRequirementsCheckListener::~GenericRequirementsCheckListener() {}
1379+
1380+
bool GenericRequirementsCheckListener::shouldCheck(RequirementKind kind,
1381+
Type first, Type second) {
1382+
return true;
1383+
}
1384+
1385+
void GenericRequirementsCheckListener::diagnosed(
1386+
const Requirement *withRequirement) {}
1387+
13781388
bool TypeChecker::
13791389
solveForExpression(Expr *&expr, DeclContext *dc, Type convertType,
13801390
FreeTypeVariableBinding allowFreeTypeVariables,

lib/Sema/TypeCheckGeneric.cpp

Lines changed: 27 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -844,13 +844,12 @@ void TypeChecker::revertGenericFuncSignature(AbstractFunctionDecl *func) {
844844
}
845845
}
846846

847-
std::pair<bool, bool>
848-
TypeChecker::checkGenericArguments(DeclContext *dc, SourceLoc loc,
849-
SourceLoc noteLoc,
850-
Type owner,
851-
GenericSignature *genericSig,
852-
const TypeSubstitutionMap &substitutions,
853-
UnsatisfiedDependency *unsatisfiedDependency) {
847+
std::pair<bool, bool> TypeChecker::checkGenericArguments(
848+
DeclContext *dc, SourceLoc loc, SourceLoc noteLoc, Type owner,
849+
GenericSignature *genericSig, const TypeSubstitutionMap &substitutions,
850+
UnsatisfiedDependency *unsatisfiedDependency,
851+
ConformanceCheckOptions conformanceOptions,
852+
GenericRequirementsCheckListener *listener) {
854853
// Check each of the requirements.
855854
ModuleDecl *module = dc->getParentModule();
856855
for (const auto &req : genericSig->getRequirements()) {
@@ -871,26 +870,33 @@ TypeChecker::checkGenericArguments(DeclContext *dc, SourceLoc loc,
871870
}
872871
}
873872

874-
switch (req.getKind()) {
873+
auto kind = req.getKind();
874+
if (listener && !listener->shouldCheck(kind, firstType, secondType))
875+
continue;
876+
877+
switch (kind) {
875878
case RequirementKind::Conformance: {
876879
// Protocol conformance requirements.
877880
auto proto = secondType->castTo<ProtocolType>();
878881
// FIXME: This should track whether this should result in a private
879882
// or non-private dependency.
880883
// FIXME: Do we really need "used" at this point?
881884
// FIXME: Poor location information. How much better can we do here?
882-
auto result = conformsToProtocol(
883-
firstType, proto->getDecl(), dc,
884-
ConformanceCheckFlags::Used, loc,
885-
unsatisfiedDependency);
885+
auto result =
886+
conformsToProtocol(firstType, proto->getDecl(), dc,
887+
conformanceOptions, loc, unsatisfiedDependency);
886888

887889
// Unsatisfied dependency case.
888890
if (result.first)
889891
return std::make_pair(true, false);
890892

891893
// Conformance check failure case.
892-
if (!result.second)
894+
if (!result.second) {
895+
if (listener && loc.isValid())
896+
listener->diagnosed(&req);
897+
893898
return std::make_pair(false, false);
899+
}
894900

895901
continue;
896902
}
@@ -913,6 +919,10 @@ TypeChecker::checkGenericArguments(DeclContext *dc, SourceLoc loc,
913919
req.getFirstType(), req.getSecondType(),
914920
genericSig->gatherGenericParamBindingsText(
915921
{req.getFirstType(), req.getSecondType()}, substitutions));
922+
923+
if (listener)
924+
listener->diagnosed(&req);
925+
916926
return std::make_pair(false, false);
917927
}
918928
continue;
@@ -926,6 +936,10 @@ TypeChecker::checkGenericArguments(DeclContext *dc, SourceLoc loc,
926936
req.getSecondType(),
927937
genericSig->gatherGenericParamBindingsText(
928938
{req.getFirstType(), req.getSecondType()}, substitutions));
939+
940+
if (listener)
941+
listener->diagnosed(&req);
942+
929943
return std::make_pair(false, false);
930944
}
931945
continue;

lib/Sema/TypeChecker.h

Lines changed: 38 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -331,6 +331,35 @@ class ExprTypeCheckListener {
331331
Expr *expr);
332332
};
333333

334+
/// An abstract interface that is used by `checkGenericArguments`.
335+
class GenericRequirementsCheckListener {
336+
public:
337+
virtual ~GenericRequirementsCheckListener();
338+
339+
/// Callback invoked before trying to check generic requirement placed
340+
/// between given types. Note: if either of the types assigned to the
341+
/// requirement is generic parameter or dependent member, this callback
342+
/// method is going to get their substitutions.
343+
///
344+
/// \param kind The kind of generic requirement to check.
345+
///
346+
/// \param first The left-hand side type assigned to the requirement,
347+
/// possibly represented by its generic substitute.
348+
///
349+
/// \param second The right-hand side type assinged to the requirement,
350+
/// possibly represented by its generic substitute.
351+
///
352+
///
353+
/// \returns true if it's ok to validate requirement, false otherwise.
354+
virtual bool shouldCheck(RequirementKind kind, Type first, Type second);
355+
356+
/// Callback invoked when given requirement has been diagnosed as invalid.
357+
///
358+
/// \param requirement The requirement which didn't pass the check.
359+
///
360+
virtual void diagnosed(const Requirement *requirement);
361+
};
362+
334363
/// Flags that describe the context of type checking a pattern or
335364
/// type.
336365
enum TypeResolutionFlags : unsigned {
@@ -1156,18 +1185,21 @@ class TypeChecker final : public LazyResolver {
11561185
/// \param substitutions Substitutions from interface types of the signature.
11571186
/// \param unsatisfiedDependency Optional callback for reporting unsatisfied
11581187
/// dependencies.
1188+
/// \param conformanceOptions The flags to use when checking conformance
1189+
/// requirement.
1190+
/// \param listener The generic check listener used to pick requirements and
1191+
/// notify callers about diagnosed errors.
11591192
///
11601193
/// \returns One of the following:
11611194
/// - (true, false) if there was an unsatisfied dependency
11621195
/// - (false, true) on success
11631196
/// - (false, false) on failure
11641197
std::pair<bool, bool> checkGenericArguments(
1165-
DeclContext *dc, SourceLoc loc,
1166-
SourceLoc noteLoc,
1167-
Type owner,
1168-
GenericSignature *genericSig,
1169-
const TypeSubstitutionMap &substitutions,
1170-
UnsatisfiedDependency *unsatisfiedDependency);
1198+
DeclContext *dc, SourceLoc loc, SourceLoc noteLoc, Type owner,
1199+
GenericSignature *genericSig, const TypeSubstitutionMap &substitutions,
1200+
UnsatisfiedDependency *unsatisfiedDependency,
1201+
ConformanceCheckOptions conformanceOptions = ConformanceCheckFlags::Used,
1202+
GenericRequirementsCheckListener *listener = nullptr);
11711203

11721204
/// Resolve the superclass of the given class.
11731205
void resolveSuperclass(ClassDecl *classDecl) override;

0 commit comments

Comments
 (0)