From 599be9000970bdcf09684082459cf62aaff0ba17 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Thu, 12 Jan 2017 18:40:16 -0800 Subject: [PATCH] [QoI] Refactor TypeChecker::checkGenericArguments. NFC Refactor TypeChecker::checkGenericArguments to enable it to be used by FailureDiagnosis::diagnoseArgumentGenericRequirements, which consolidates requirement checking in one place. --- lib/Sema/CSDiag.cpp | 81 ++++++++----------------------- lib/Sema/TypeCheckConstraints.cpp | 10 ++++ lib/Sema/TypeCheckGeneric.cpp | 40 ++++++++++----- lib/Sema/TypeChecker.h | 44 ++++++++++++++--- 4 files changed, 94 insertions(+), 81 deletions(-) diff --git a/lib/Sema/CSDiag.cpp b/lib/Sema/CSDiag.cpp index b46c8fe6d5d37..260af80e7959d 100644 --- a/lib/Sema/CSDiag.cpp +++ b/lib/Sema/CSDiag.cpp @@ -5704,9 +5704,6 @@ bool FailureDiagnosis::diagnoseArgumentGenericRequirements( /*allowFixes=*/false, listener, bindings)) return false; - auto loc = argExpr->getLoc(); - auto noteLoc = AFD->getLoc(); - TypeSubstitutionMap substitutions; // First, let's collect all of the archetypes and their substitutions, // that's going to help later on if there are cross-archetype @@ -5740,72 +5737,32 @@ bool FailureDiagnosis::diagnoseArgumentGenericRequirements( if (substitutions.empty()) return false; - auto dc = env->getOwningDeclContext(); - auto module = dc->getParentModule(); - auto signature = env->getGenericSignature(); - - for (const auto &req : signature->getRequirements()) { - Type firstType = req.getFirstType().subst(module, substitutions); - if (firstType.isNull()) - continue; + class RequirementsListener : public GenericRequirementsCheckListener { + private: + bool DiagnosedAny = false; - Type secondType = req.getSecondType(); - if (secondType) { - secondType = secondType.subst(module, substitutions); - if (secondType.isNull()) - continue; + public: + bool shouldCheck(RequirementKind kind, Type first, Type second) override { + // This means that we have encountered requirement which references + // generic parameter not used in the arguments, we can't diagnose it here. + return !(first->hasTypeParameter() || first->isTypeVariableOrMember()); } - // This means that we have encountered requirement which references - // generic parameter not used in the arguments, we can't diagnose it here. - if (firstType->hasTypeParameter() || firstType->isTypeVariableOrMember()) - continue; - - switch (req.getKind()) { - case RequirementKind::Conformance: { - auto protocol = secondType->castTo(); - auto result = TC.conformsToProtocol( - firstType, protocol->getDecl(), dc, - ConformanceCheckFlags::SuppressDependencyTracking, loc, nullptr); - - // Conformance failed and was diagnosed by `conformsToProtocol`. - if (!result.second) - return true; - - break; + void diagnosed(const Requirement *requirement) override { + DiagnosedAny = true; } - case RequirementKind::Superclass: - if (!TC.isSubtypeOf(firstType, secondType, dc)) { - TC.diagnose(loc, diag::type_does_not_inherit, AFD->getInterfaceType(), - firstType, secondType); - - TC.diagnose( - noteLoc, diag::type_does_not_inherit_requirement, - req.getFirstType(), req.getSecondType(), - signature->gatherGenericParamBindingsText( - {req.getFirstType(), req.getSecondType()}, substitutions)); - return true; - } - break; - - case RequirementKind::SameType: - if (!firstType->isEqual(secondType)) { - TC.diagnose(loc, diag::types_not_equal, AFD->getInterfaceType(), - firstType, secondType); + bool foundProblems() const { return DiagnosedAny; } + }; - TC.diagnose( - noteLoc, diag::types_not_equal_requirement, req.getFirstType(), - req.getSecondType(), - signature->gatherGenericParamBindingsText( - {req.getFirstType(), req.getSecondType()}, substitutions)); - return true; - } - break; - } - } + RequirementsListener genericReqListener; + TC.checkGenericArguments(env->getOwningDeclContext(), argExpr->getLoc(), + AFD->getLoc(), AFD->getInterfaceType(), + env->getGenericSignature(), substitutions, nullptr, + ConformanceCheckFlags::SuppressDependencyTracking, + &genericReqListener); - return false; + return genericReqListener.foundProblems(); } /// When initializing Unsafe[Mutable]Pointer from Unsafe[Mutable]RawPointer, diff --git a/lib/Sema/TypeCheckConstraints.cpp b/lib/Sema/TypeCheckConstraints.cpp index d9e98ff87c2ef..1ae4d09515c79 100644 --- a/lib/Sema/TypeCheckConstraints.cpp +++ b/lib/Sema/TypeCheckConstraints.cpp @@ -1375,6 +1375,16 @@ Expr *ExprTypeCheckListener::appliedSolution(Solution &solution, Expr *expr) { return expr; } +GenericRequirementsCheckListener::~GenericRequirementsCheckListener() {} + +bool GenericRequirementsCheckListener::shouldCheck(RequirementKind kind, + Type first, Type second) { + return true; +} + +void GenericRequirementsCheckListener::diagnosed( + const Requirement *withRequirement) {} + bool TypeChecker:: solveForExpression(Expr *&expr, DeclContext *dc, Type convertType, FreeTypeVariableBinding allowFreeTypeVariables, diff --git a/lib/Sema/TypeCheckGeneric.cpp b/lib/Sema/TypeCheckGeneric.cpp index 0841a87550d56..563025fe62828 100644 --- a/lib/Sema/TypeCheckGeneric.cpp +++ b/lib/Sema/TypeCheckGeneric.cpp @@ -844,13 +844,12 @@ void TypeChecker::revertGenericFuncSignature(AbstractFunctionDecl *func) { } } -std::pair -TypeChecker::checkGenericArguments(DeclContext *dc, SourceLoc loc, - SourceLoc noteLoc, - Type owner, - GenericSignature *genericSig, - const TypeSubstitutionMap &substitutions, - UnsatisfiedDependency *unsatisfiedDependency) { +std::pair TypeChecker::checkGenericArguments( + DeclContext *dc, SourceLoc loc, SourceLoc noteLoc, Type owner, + GenericSignature *genericSig, const TypeSubstitutionMap &substitutions, + UnsatisfiedDependency *unsatisfiedDependency, + ConformanceCheckOptions conformanceOptions, + GenericRequirementsCheckListener *listener) { // Check each of the requirements. ModuleDecl *module = dc->getParentModule(); for (const auto &req : genericSig->getRequirements()) { @@ -871,7 +870,11 @@ TypeChecker::checkGenericArguments(DeclContext *dc, SourceLoc loc, } } - switch (req.getKind()) { + auto kind = req.getKind(); + if (listener && !listener->shouldCheck(kind, firstType, secondType)) + continue; + + switch (kind) { case RequirementKind::Conformance: { // Protocol conformance requirements. auto proto = secondType->castTo(); @@ -879,18 +882,21 @@ TypeChecker::checkGenericArguments(DeclContext *dc, SourceLoc loc, // or non-private dependency. // FIXME: Do we really need "used" at this point? // FIXME: Poor location information. How much better can we do here? - auto result = conformsToProtocol( - firstType, proto->getDecl(), dc, - ConformanceCheckFlags::Used, loc, - unsatisfiedDependency); + auto result = + conformsToProtocol(firstType, proto->getDecl(), dc, + conformanceOptions, loc, unsatisfiedDependency); // Unsatisfied dependency case. if (result.first) return std::make_pair(true, false); // Conformance check failure case. - if (!result.second) + if (!result.second) { + if (listener && loc.isValid()) + listener->diagnosed(&req); + return std::make_pair(false, false); + } continue; } @@ -913,6 +919,10 @@ TypeChecker::checkGenericArguments(DeclContext *dc, SourceLoc loc, req.getFirstType(), req.getSecondType(), genericSig->gatherGenericParamBindingsText( {req.getFirstType(), req.getSecondType()}, substitutions)); + + if (listener) + listener->diagnosed(&req); + return std::make_pair(false, false); } continue; @@ -926,6 +936,10 @@ TypeChecker::checkGenericArguments(DeclContext *dc, SourceLoc loc, req.getSecondType(), genericSig->gatherGenericParamBindingsText( {req.getFirstType(), req.getSecondType()}, substitutions)); + + if (listener) + listener->diagnosed(&req); + return std::make_pair(false, false); } continue; diff --git a/lib/Sema/TypeChecker.h b/lib/Sema/TypeChecker.h index faae9b738c2b0..3163576dd9ab3 100644 --- a/lib/Sema/TypeChecker.h +++ b/lib/Sema/TypeChecker.h @@ -331,6 +331,35 @@ class ExprTypeCheckListener { Expr *expr); }; +/// An abstract interface that is used by `checkGenericArguments`. +class GenericRequirementsCheckListener { +public: + virtual ~GenericRequirementsCheckListener(); + + /// Callback invoked before trying to check generic requirement placed + /// between given types. Note: if either of the types assigned to the + /// requirement is generic parameter or dependent member, this callback + /// method is going to get their substitutions. + /// + /// \param kind The kind of generic requirement to check. + /// + /// \param first The left-hand side type assigned to the requirement, + /// possibly represented by its generic substitute. + /// + /// \param second The right-hand side type assinged to the requirement, + /// possibly represented by its generic substitute. + /// + /// + /// \returns true if it's ok to validate requirement, false otherwise. + virtual bool shouldCheck(RequirementKind kind, Type first, Type second); + + /// Callback invoked when given requirement has been diagnosed as invalid. + /// + /// \param requirement The requirement which didn't pass the check. + /// + virtual void diagnosed(const Requirement *requirement); +}; + /// Flags that describe the context of type checking a pattern or /// type. enum TypeResolutionFlags : unsigned { @@ -1156,18 +1185,21 @@ class TypeChecker final : public LazyResolver { /// \param substitutions Substitutions from interface types of the signature. /// \param unsatisfiedDependency Optional callback for reporting unsatisfied /// dependencies. + /// \param conformanceOptions The flags to use when checking conformance + /// requirement. + /// \param listener The generic check listener used to pick requirements and + /// notify callers about diagnosed errors. /// /// \returns One of the following: /// - (true, false) if there was an unsatisfied dependency /// - (false, true) on success /// - (false, false) on failure std::pair checkGenericArguments( - DeclContext *dc, SourceLoc loc, - SourceLoc noteLoc, - Type owner, - GenericSignature *genericSig, - const TypeSubstitutionMap &substitutions, - UnsatisfiedDependency *unsatisfiedDependency); + DeclContext *dc, SourceLoc loc, SourceLoc noteLoc, Type owner, + GenericSignature *genericSig, const TypeSubstitutionMap &substitutions, + UnsatisfiedDependency *unsatisfiedDependency, + ConformanceCheckOptions conformanceOptions = ConformanceCheckFlags::Used, + GenericRequirementsCheckListener *listener = nullptr); /// Resolve the superclass of the given class. void resolveSuperclass(ClassDecl *classDecl) override;