Merge pull request #6770 from xedin/cgr-refactor

[QoI] Refactor TypeChecker::checkGenericArguments. NFC

Author: slavapestov

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<ProtocolType>();
-      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<T> from Unsafe[Mutable]RawPointer,

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,

lib/Sema/TypeCheckGeneric.cpp

@@ -843,13 +843,12 @@ void TypeChecker::revertGenericFuncSignature(AbstractFunctionDecl *func) {
   }
 }
 
-std::pair<bool, bool>
-TypeChecker::checkGenericArguments(DeclContext *dc, SourceLoc loc,
-                                   SourceLoc noteLoc,
-                                   Type owner,
-                                   GenericSignature *genericSig,
-                                   const TypeSubstitutionMap &substitutions,
-                                   UnsatisfiedDependency *unsatisfiedDependency) {
+std::pair<bool, bool> 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()) {
@@ -870,26 +869,33 @@ 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<ProtocolType>();
       // FIXME: This should track whether this should result in a private
       // 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;
     }
@@ -912,6 +918,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;
@@ -925,6 +935,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;

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<bool, bool> 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;