Sema: Pass the correct substitutions to 'checkGenericArguments' instead of mapping into context in place

AnthonyLatsis · AnthonyLatsis · commit 588603f7954f · 2021-11-19T16:39:37.000+03:00
diff --git a/lib/Sema/TypeCheckGeneric.cpp b/lib/Sema/TypeCheckGeneric.cpp
@@ -800,7 +800,7 @@ GenericSignatureRequest::evaluate(Evaluator &evaluator,
 ///
 
 RequirementCheckResult TypeChecker::checkGenericArguments(
-    DeclContext *dc, SourceLoc loc, SourceLoc noteLoc, Type owner,
+    ModuleDecl *module, SourceLoc loc, SourceLoc noteLoc, Type owner,
     TypeArrayView<GenericTypeParamType> genericParams,
     ArrayRef<Requirement> requirements,
     TypeSubstitutionFn substitutions,
@@ -815,7 +815,6 @@ RequirementCheckResult TypeChecker::checkGenericArguments(
   SmallVector<RequirementSet, 8> pendingReqs;
   pendingReqs.push_back({requirements, {}});
 
-  auto *module = dc->getParentModule();
   ASTContext &ctx = module->getASTContext();
   while (!pendingReqs.empty()) {
     auto current = pendingReqs.pop_back_val();
@@ -824,14 +823,7 @@ RequirementCheckResult TypeChecker::checkGenericArguments(
       auto req = rawReq;
       if (current.Parents.empty()) {
         auto substed = rawReq.subst(
-            [&](SubstitutableType *type) -> Type {
-              auto substType = substitutions(type);
-              if (substType->hasTypeParameter())
-                return dc->mapTypeIntoContext(substType);
-              return substType;
-            },
-            LookUpConformanceInModule(module),
-            options);
+            substitutions, LookUpConformanceInModule(module), options);
         if (!substed) {
           // Another requirement will fail later; just continue.
           valid = false;
diff --git a/lib/Sema/TypeCheckProtocol.cpp b/lib/Sema/TypeCheckProtocol.cpp
@@ -4705,8 +4705,9 @@ ResolveWitnessResult ConformanceChecker::resolveTypeWitnessViaLookup(
     // If the type comes from a constrained extension or has a 'where'
     // clause, check those requirements now.
     if (!skipRequirementCheck &&
-        !TypeChecker::checkContextualRequirements(genericDecl, Adoptee,
-                                                  SourceLoc(), DC)) {
+        !TypeChecker::checkContextualRequirements(
+            genericDecl, Adoptee, SourceLoc(), DC->getParentModule(),
+            DC->getGenericSignatureOfContext())) {
       continue;
     }
 
@@ -4798,7 +4799,7 @@ void ConformanceChecker::ensureRequirementsAreSatisfied() {
       proto, substitutingType, ProtocolConformanceRef(Conformance));
 
   auto result = TypeChecker::checkGenericArguments(
-      DC, Loc, Loc,
+      DC->getParentModule(), Loc, Loc,
       // FIXME: maybe this should be the conformance's type
       proto->getDeclaredInterfaceType(),
       { proto->getSelfInterfaceType() },
diff --git a/lib/Sema/TypeCheckProtocolInference.cpp b/lib/Sema/TypeCheckProtocolInference.cpp
@@ -1086,8 +1086,8 @@ bool AssociatedTypeInference::checkCurrentTypeWitnesses(
   sanitizeProtocolRequirements(proto, proto->getRequirementSignature(),
                                sanitizedRequirements);
   auto result =
-    TypeChecker::checkGenericArguments(dc, SourceLoc(), SourceLoc(),
-                                       typeInContext,
+    TypeChecker::checkGenericArguments(dc->getParentModule(), SourceLoc(),
+                                       SourceLoc(), typeInContext,
                                        { proto->getSelfInterfaceType() },
                                        sanitizedRequirements,
                                        QuerySubstitutionMap{substitutions},
@@ -1134,7 +1134,7 @@ bool AssociatedTypeInference::checkConstrainedExtension(ExtensionDecl *ext) {
 
   SubstOptions options = getSubstOptionsWithCurrentTypeWitnesses();
   switch (TypeChecker::checkGenericArguments(
-                       dc, SourceLoc(), SourceLoc(), adoptee,
+                       dc->getParentModule(), SourceLoc(), SourceLoc(), adoptee,
                        ext->getGenericSignature().getGenericParams(),
                        ext->getGenericSignature().getRequirements(),
                        QueryTypeSubstitutionMap{subs},
diff --git a/lib/Sema/TypeCheckType.cpp b/lib/Sema/TypeCheckType.cpp
@@ -569,17 +569,12 @@ static bool isPointerToVoid(ASTContext &Ctx, Type Ty, bool &IsMutable) {
   return BGT->getGenericArgs().front()->isVoid();
 }
 
-/// Even if the type is not generic, it might be inside of a generic
-/// context or have a free-standing 'where' clause, so we need to
-/// those check requirements too.
-///
-/// Return true on success.
 bool TypeChecker::checkContextualRequirements(GenericTypeDecl *decl,
-                                              Type parentTy,
-                                              SourceLoc loc,
-                                              DeclContext *dc) {
-  if (!parentTy || parentTy->hasUnboundGenericType() ||
-      parentTy->hasTypeVariable()) {
+                                              Type parentTy, SourceLoc loc,
+                                              ModuleDecl *module,
+                                              GenericSignature contextSig) {
+  assert(parentTy && "expected a parent type");
+  if (parentTy->hasUnboundGenericType() || parentTy->hasTypeVariable()) {
     return true;
   }
 
@@ -599,12 +594,16 @@ bool TypeChecker::checkContextualRequirements(GenericTypeDecl *decl,
       noteLoc = loc;
   }
 
+  if (contextSig) {
+    parentTy = contextSig.getGenericEnvironment()->mapTypeIntoContext(parentTy);
+  }
+
   const auto subMap = parentTy->getContextSubstitutions(decl->getDeclContext());
   const auto genericSig = decl->getGenericSignature();
 
   const auto result =
     TypeChecker::checkGenericArguments(
-        dc, loc, noteLoc,
+        module, loc, noteLoc,
         decl->getDeclaredInterfaceType(),
         genericSig.getGenericParams(),
         genericSig.getRequirements(),
@@ -679,7 +678,13 @@ static Type applyGenericArguments(Type type, TypeResolution resolution,
       return type;
     }
 
-    if (TypeChecker::checkContextualRequirements(decl, parentTy, loc, dc))
+    if (!parentTy) {
+      return type;
+    }
+
+    if (TypeChecker::checkContextualRequirements(
+            decl, parentTy, loc, dc->getParentModule(),
+            resolution.getGenericSignature()))
       return type;
 
     return ErrorType::get(resolution.getASTContext());
@@ -872,10 +877,6 @@ Type TypeResolution::applyUnboundGenericArguments(
     }
   }
 
-  SourceLoc noteLoc = decl->getLoc();
-  if (noteLoc.isInvalid())
-    noteLoc = loc;
-
   // Realize the types of the generic arguments and add them to the
   // substitution map.
   for (unsigned i = 0, e = genericArgs.size(); i < e; ++i) {
@@ -895,11 +896,25 @@ Type TypeResolution::applyUnboundGenericArguments(
   auto *module = getDeclContext()->getParentModule();
 
   if (!skipRequirementsCheck && getStage() > TypeResolutionStage::Structural) {
+    TypeSubstitutionMap contextualSubs = subs;
+    if (getStage() == TypeResolutionStage::Interface) {
+      if (const auto contextSig = getGenericSignature()) {
+        auto *genericEnv = contextSig.getGenericEnvironment();
+        for (auto &pair : contextualSubs) {
+          pair.second = genericEnv->mapTypeIntoContext(pair.second);
+        }
+      }
+    }
+
+    SourceLoc noteLoc = decl->getLoc();
+    if (noteLoc.isInvalid())
+      noteLoc = loc;
+
     auto result = TypeChecker::checkGenericArguments(
-        getDeclContext(), loc, noteLoc,
+        module, loc, noteLoc,
         UnboundGenericType::get(decl, parentTy, getASTContext()),
         genericSig.getGenericParams(), genericSig.getRequirements(),
-        QueryTypeSubstitutionMap{subs});
+        QueryTypeSubstitutionMap{contextualSubs});
 
     switch (result) {
     case RequirementCheckResult::Failure:
diff --git a/lib/Sema/TypeCheckType.h b/lib/Sema/TypeCheckType.h
@@ -312,10 +312,6 @@ class TypeResolution {
         placeholderHandler(placeholderHandler),
         genericEnv(nullptr) {}
 
-  /// Retrieves the generic signature for the context, or NULL if there is
-  /// no generic signature to resolve types.
-  GenericSignature getGenericSignature() const;
-
 public:
   /// Form a type resolution for the structure of a type, which does not
   /// attempt to resolve member types of type parameters to a particular
@@ -373,6 +369,10 @@ class TypeResolution {
     return placeholderHandler;
   }
 
+  /// Retrieves the generic signature for the context, or NULL if there is
+  /// no generic signature to resolve types.
+  GenericSignature getGenericSignature() const;
+
   /// Resolves a TypeRepr to a type.
   ///
   /// Performs name lookup, checking of generic arguments, and so on in order
diff --git a/lib/Sema/TypeChecker.h b/lib/Sema/TypeChecker.h
@@ -445,7 +445,7 @@ std::string gatherGenericParamBindingsText(
 /// Check the given set of generic arguments against the requirements in a
 /// generic signature.
 ///
-/// \param dc The context in which the generic arguments should be checked.
+/// \param module The module to use for conformace lookup.
 /// \param loc The location at which any diagnostics should be emitted.
 /// \param noteLoc The location at which any notes will be printed.
 /// \param owner The type that owns the generic signature.
@@ -454,7 +454,7 @@ std::string gatherGenericParamBindingsText(
 /// should be checked.
 /// \param substitutions Substitutions from interface types of the signature.
 RequirementCheckResult checkGenericArguments(
-    DeclContext *dc, SourceLoc loc, SourceLoc noteLoc, Type owner,
+    ModuleDecl *module, SourceLoc loc, SourceLoc noteLoc, Type owner,
     TypeArrayView<GenericTypeParamType> genericParams,
     ArrayRef<Requirement> requirements, TypeSubstitutionFn substitutions,
     SubstOptions options = None);
@@ -465,10 +465,49 @@ RequirementCheckResult checkGenericArguments(
     ArrayRef<Requirement> requirements,
     TypeSubstitutionFn substitutions);
 
-bool checkContextualRequirements(GenericTypeDecl *decl,
-                                 Type parentTy,
-                                 SourceLoc loc,
-                                 DeclContext *dc);
+/// Checks whether the generic requirements imposed on the nested type
+/// declaration \p decl (if present) are in agreement with the substitutions
+/// that are needed to spell it as a member of the given parent type
+/// \p parentTy.
+///
+/// For example, given
+/// \code
+/// struct S<X> {}
+/// extension S where X == Bool {
+///   struct Inner {}
+/// }
+/// \endcode
+/// \c Inner cannot be referenced on \c S<Int>, because its contextual
+/// requirement \c X \c == \c Bool is not satisfied by the substitution
+/// \c [X \c = \c Int].
+///
+/// Similarly, \c typealias \c Y below is a viable type witness in the
+/// conformance of \c S to \c P, because its contextual requirement
+/// \c Self.X \c == \c Bool is satisfied by the substitution
+/// \c [Self \c = \c S].
+/// \code
+/// protocol P {
+///   associatedtype X
+///   associatedtype Y
+/// }
+/// extension P where X == Bool {
+///   typealias Y = Bool
+/// }
+///
+/// struct S: P {
+///   typealias X = Bool
+/// }
+/// \endcode
+///
+/// \param module The module to use for conformace lookup.
+/// \param contextSig The generic signature that should be used to map
+/// \p parentTy into context. We pass a generic signature to secure on-demand
+/// computation of the associated generic enviroment.
+///
+/// \returns \c true on success.
+bool checkContextualRequirements(GenericTypeDecl *decl, Type parentTy,
+                                 SourceLoc loc, ModuleDecl *module,
+                                 GenericSignature contextSig);
 
 /// Add any implicitly-defined constructors required for the given
 /// struct, class or actor.
diff --git a/validation-test/compiler_crashers_2_fixed/0161-sr6569.swift b/validation-test/compiler_crashers_2_fixed/0161-sr6569.swift
@@ -6,8 +6,8 @@ protocol P {
 
 struct Type<Param> {}
 extension Type: P where Param: P, Param.A == Type<Param> {
-  // expected-error@-1 5{{extension of generic struct 'Type' has self-referential generic requirements}}
-  // expected-note@-2 5{{through reference here}}
+  // expected-error@-1 6{{extension of generic struct 'Type' has self-referential generic requirements}}
+  // expected-note@-2 6{{through reference here}}
   // expected-error@-3 {{type 'Type<Param>' does not conform to protocol 'P'}}
   typealias A = Param
   // expected-note@-1 2{{through reference here}}
