Merge pull request swiftlang#17168 from huonw/validate-conditional-type-in-expr

[Sema] More aggressive consideration of conditionally defined types in expressions

Author: huonw

lib/Sema/ConstraintSystem.cpp

@@ -481,36 +481,74 @@ Type ConstraintSystem::openUnboundGenericType(UnboundGenericType *unbound,
       /*unsatisfiedDependency*/nullptr);
 }
 
-Type ConstraintSystem::openUnboundGenericType(
-       Type type,
-       ConstraintLocatorBuilder locator) {
-  assert(!type->hasTypeParameter());
+static void checkNestedTypeConstraints(ConstraintSystem &cs, Type type,
+                                       ConstraintLocatorBuilder locator) {
+  // If this is a type defined inside of constrainted extension, let's add all
+  // of the generic requirements to the constraint system to make sure that it's
+  // something we can use.
+  GenericTypeDecl *decl = nullptr;
+  Type parentTy;
+  SubstitutionMap subMap;
 
-  // If this is a generic typealias defined inside of constrainted extension,
-  // let's add all of the generic requirements to the constraint system to
-  // make sure that it's something we can use.
   if (auto *NAT = dyn_cast<NameAliasType>(type.getPointer())) {
-    auto *decl = NAT->getDecl();
-    auto *extension = dyn_cast<ExtensionDecl>(decl->getDeclContext());
-    auto parentTy = NAT->getParent();
-
+    decl = NAT->getDecl();
+    parentTy = NAT->getParent();
+    subMap = NAT->getSubstitutionMap();
+  } else if (auto *AGT = type->getAs<AnyGenericType>()) {
+    decl = AGT->getDecl();
+    parentTy = AGT->getParent();
+    // the context substitution map is fine here, since we can't be adding more
+    // info than that, unlike a typealias
+  }
+
+  // If this decl is generic, the constraints are handled when the generic
+  // parameters are applied, so we don't have to handle them here (which makes
+  // getting the right substitution maps easier).
+  if (decl && !decl->isGeneric()) {
+    auto extension = dyn_cast<ExtensionDecl>(decl->getDeclContext());
     if (parentTy && extension && extension->isConstrainedExtension()) {
-      auto subMap = NAT->getSubstitutionMap();
       auto contextSubMap = parentTy->getContextSubstitutionMap(
           extension->getParentModule(),
           extension->getAsNominalTypeOrNominalTypeExtensionContext());
+      if (!subMap) {
+        // The substitution map wasn't set above, meaning we should grab the map
+        // for the extension itself.
+        subMap = parentTy->getContextSubstitutionMap(
+            extension->getParentModule(), extension);
+      }
 
-      if (auto *signature = NAT->getGenericSignature()) {
-        openGenericRequirements(
+      if (auto *signature = decl->getGenericSignature()) {
+        cs.openGenericRequirements(
             extension, signature, /*skipProtocolSelfConstraint*/ true, locator,
             [&](Type type) {
+              // Why do we look in two substitution maps? We have to use the
+              // context substitution map to find types, because we need to
+              // avoid thinking about them when handling the constraints, or all
+              // the requirements in the signature become tautologies (if the
+              // extension has 'T == Int', subMap will map T -> Int, so the
+              // requirement becomes Int == Int no matter what the actual types
+              // are here). However, we need the conformances for the extension
+              // because the requirements might look like `T: P, T.U: Q`, where
+              // U is an associated type of protocol P.
               return type.subst(QuerySubstitutionMap{contextSubMap},
                                 LookUpConformanceInSubstitutionMap(subMap),
                                 SubstFlags::UseErrorType);
             });
       }
     }
+
+    // And now make sure sure the parent is okay, for things like X<T>.Y.Z.
+    if (parentTy) {
+      checkNestedTypeConstraints(cs, parentTy, locator);
+    }
   }
+}
+
+Type ConstraintSystem::openUnboundGenericType(
+    Type type, ConstraintLocatorBuilder locator) {
+  assert(!type->hasTypeParameter());
+
+  checkNestedTypeConstraints(*this, type, locator);
 
   if (!type->hasUnboundGenericType())
     return type;

lib/Sema/TypeCheckConstraints.cpp

@@ -1215,9 +1215,9 @@ TypeExpr *PreCheckExpression::simplifyNestedTypeExpr(UnresolvedDotExpr *UDE) {
       // If there is no nested type with this name, we have a lookup of
       // a non-type member, so leave the expression as-is.
       if (Result.size() == 1) {
-        return TypeExpr::createForMemberDecl(
-          DRE->getNameLoc().getBaseNameLoc(), TD,
-          NameLoc, Result.front().first);
+        return TypeExpr::createForMemberDecl(DRE->getNameLoc().getBaseNameLoc(),
+                                             TD, NameLoc,
+                                             Result.front().Member);
       }
     }
 
@@ -1273,7 +1273,7 @@ TypeExpr *PreCheckExpression::simplifyNestedTypeExpr(UnresolvedDotExpr *UDE) {
       // a non-type member, so leave the expression as-is.
       if (Result.size() == 1) {
         return TypeExpr::createForMemberDecl(ITR, NameLoc,
-                                             Result.front().first);
+                                             Result.front().Member);
       }
     }
   }

lib/Sema/TypeCheckNameLookup.cpp

@@ -447,7 +447,7 @@ LookupTypeResult TypeChecker::lookupMemberType(DeclContext *dc,
       // Add the type to the result set, so that we can diagnose the
       // reference instead of just saying the member does not exist.
       if (types.insert(memberType->getCanonicalType()).second)
-        result.Results.push_back({typeDecl, memberType});
+        result.Results.push_back({typeDecl, memberType, nullptr});
 
       continue;
     }
@@ -492,7 +492,7 @@ LookupTypeResult TypeChecker::lookupMemberType(DeclContext *dc,
 
     // If we haven't seen this type result yet, add it to the result set.
     if (types.insert(memberType->getCanonicalType()).second)
-      result.Results.push_back({typeDecl, memberType});
+      result.Results.push_back({typeDecl, memberType, nullptr});
   }
 
   if (result.Results.empty()) {
@@ -521,19 +521,21 @@ LookupTypeResult TypeChecker::lookupMemberType(DeclContext *dc,
 
       // Use the type witness.
       auto concrete = conformance->getConcrete();
-      Type memberType = concrete->getTypeWitness(assocType, this);
 
       // This is the only case where NormalProtocolConformance::
       // getTypeWitnessAndDecl() returns a null type.
       if (concrete->getState() ==
           ProtocolConformanceState::CheckingTypeWitnesses)
         continue;
 
-      assert(memberType && "Missing type witness?");
+      auto typeDecl = concrete->getTypeWitnessAndDecl(assocType, this).second;
 
-      // If we haven't seen this type result yet, add it to the result set.
+      assert(typeDecl && "Missing type witness?");
+
+      auto memberType =
+          substMemberTypeWithBase(dc->getParentModule(), typeDecl, type);
       if (types.insert(memberType->getCanonicalType()).second)
-        result.Results.push_back({assocType, memberType});
+        result.Results.push_back({typeDecl, memberType, assocType});
     }
   }
 

lib/Sema/TypeCheckProtocol.cpp

@@ -3098,23 +3098,23 @@ ResolveWitnessResult ConformanceChecker::resolveTypeWitnessViaLookup(
   }
 
   // Determine which of the candidates is viable.
-  SmallVector<std::pair<TypeDecl *, Type>, 2> viable;
+  SmallVector<LookupTypeResultEntry, 2> viable;
   SmallVector<std::pair<TypeDecl *, CheckTypeWitnessResult>, 2> nonViable;
   for (auto candidate : candidates) {
     // Skip nested generic types.
-    if (auto *genericDecl = dyn_cast<GenericTypeDecl>(candidate.first))
+    if (auto *genericDecl = dyn_cast<GenericTypeDecl>(candidate.Member))
       if (genericDecl->getGenericParams())
         continue;
 
     // Skip typealiases with an unbound generic type as their underlying type.
-    if (auto *typeAliasDecl = dyn_cast<TypeAliasDecl>(candidate.first))
+    if (auto *typeAliasDecl = dyn_cast<TypeAliasDecl>(candidate.Member))
       if (typeAliasDecl->getDeclaredInterfaceType()->is<UnboundGenericType>())
         continue;
 
     // Check this type against the protocol requirements.
-    if (auto checkResult = checkTypeWitness(TC, DC, Proto, assocType,
-                                            candidate.second)) {
-      nonViable.push_back({candidate.first, checkResult});
+    if (auto checkResult =
+            checkTypeWitness(TC, DC, Proto, assocType, candidate.MemberType)) {
+      nonViable.push_back({candidate.Member, checkResult});
     } else {
       viable.push_back(candidate);
     }
@@ -3132,10 +3132,10 @@ ResolveWitnessResult ConformanceChecker::resolveTypeWitnessViaLookup(
 
   // If there is a single viable candidate, form a substitution for it.
   if (viable.size() == 1) {
-    auto interfaceType = viable.front().second;
+    auto interfaceType = viable.front().MemberType;
     if (interfaceType->hasArchetype())
       interfaceType = interfaceType->mapTypeOutOfContext();
-    recordTypeWitness(assocType, interfaceType, viable.front().first, true);
+    recordTypeWitness(assocType, interfaceType, viable.front().Member, true);
     return ResolveWitnessResult::Success;
   }
 
@@ -3151,7 +3151,7 @@ ResolveWitnessResult ConformanceChecker::resolveTypeWitnessViaLookup(
                        assocType->getName());
 
         for (auto candidate : viable)
-          diags.diagnose(candidate.first, diag::protocol_witness_type);
+          diags.diagnose(candidate.Member, diag::protocol_witness_type);
       });
 
     return ResolveWitnessResult::ExplicitFailed;

lib/Sema/TypeCheckType.cpp

@@ -123,7 +123,7 @@ static Type getStdlibType(TypeChecker &TC, Type &cached, DeclContext *dc,
                                                   TC.Context.getIdentifier(
                                                     name));
     if (lookup)
-      cached = lookup.back().second;
+      cached = lookup.back().MemberType;
   }
   return cached;
 }
@@ -203,7 +203,7 @@ static Type getObjectiveCNominalType(TypeChecker &TC,
   if (auto result = TC.lookupMemberType(dc, ModuleType::get(module), TypeName,
                                         lookupOptions)) {
     for (auto pair : result) {
-      if (auto nominal = dyn_cast<NominalTypeDecl>(pair.first)) {
+      if (auto nominal = dyn_cast<NominalTypeDecl>(pair.Member)) {
         cache = nominal->getDeclaredType();
         return cache;
       }
@@ -934,14 +934,14 @@ static Type diagnoseUnknownType(TypeChecker &tc, DeclContext *dc,
                                                  relookupOptions);
   if (inaccessibleMembers) {
     // FIXME: What if the unviable candidates have different levels of access?
-    const TypeDecl *first = inaccessibleMembers.front().first;
+    const TypeDecl *first = inaccessibleMembers.front().Member;
     tc.diagnose(comp->getIdLoc(), diag::candidate_inaccessible,
                 comp->getIdentifier(), first->getFormalAccess());
 
     // FIXME: If any of the candidates (usually just one) are in the same module
     // we could offer a fix-it.
     for (auto lookupResult : inaccessibleMembers)
-      tc.diagnose(lookupResult.first, diag::kind_declared_here,
+      tc.diagnose(lookupResult.Member, diag::kind_declared_here,
                   DescriptiveDeclKind::Type);
 
     // Don't try to recover here; we'll get more access-related diagnostics
@@ -1254,7 +1254,8 @@ static Type resolveNestedIdentTypeComponent(
               bool diagnoseErrors,
               GenericTypeResolver *resolver,
               UnsatisfiedDependency *unsatisfiedDependency) {
-  auto maybeDiagnoseBadMemberType = [&](TypeDecl *member, Type memberType) {
+  auto maybeDiagnoseBadMemberType = [&](TypeDecl *member, Type memberType,
+                                        AssociatedTypeDecl *inferredAssocType) {
     // Diagnose invalid cases.
     if (TC.isUnsupportedMemberTypeAccess(parentTy, member)) {
       if (diagnoseErrors) {
@@ -1285,20 +1286,21 @@ static Type resolveNestedIdentTypeComponent(
       }
     }
 
+    // Diagnose a bad conformance reference if we need to.
+    if (inferredAssocType && diagnoseErrors && memberType &&
+        memberType->hasError()) {
+      maybeDiagnoseBadConformanceRef(TC, DC, parentTy, comp->getLoc(),
+                                     inferredAssocType);
+    }
+
     // If we found a reference to an associated type or other member type that
     // was marked invalid, just return ErrorType to silence downstream errors.
     if (member->isInvalid())
       return ErrorType::get(TC.Context);
 
-    // Diagnose a bad conformance reference if we need to.
-    if (isa<AssociatedTypeDecl>(member) && diagnoseErrors &&
-        memberType && memberType->hasError()) {
-      maybeDiagnoseBadConformanceRef(TC, DC, parentTy, comp->getLoc(),
-                                     cast<AssociatedTypeDecl>(member));
-    }
-
     // At this point, we need to have resolved the type of the member.
-    if (!memberType || memberType->hasError()) return memberType;
+    if (!memberType || memberType->hasError())
+      return memberType;
 
     // If there are generic arguments, apply them now.
     if (!options.contains(TypeResolutionFlags::ResolveStructure)) {
@@ -1335,7 +1337,7 @@ static Type resolveNestedIdentTypeComponent(
   if (auto *typeDecl = comp->getBoundDecl()) {
     auto memberType = TC.substMemberTypeWithBase(DC->getParentModule(),
                                                  typeDecl, parentTy);
-    return maybeDiagnoseBadMemberType(typeDecl, memberType);
+    return maybeDiagnoseBadMemberType(typeDecl, memberType, nullptr);
   }
 
   // Phase 1: Find and bind the component decl.
@@ -1390,6 +1392,7 @@ static Type resolveNestedIdentTypeComponent(
   // If we didn't find anything, complain.
   Type memberType;
   TypeDecl *member = nullptr;
+  AssociatedTypeDecl *inferredAssocType = nullptr;
   if (!memberTypes) {
     // If we're not allowed to complain or we couldn't fix the
     // source, bail out.
@@ -1403,12 +1406,13 @@ static Type resolveNestedIdentTypeComponent(
     if (!member)
       return ErrorType::get(TC.Context);
   } else {
-    memberType = memberTypes.back().second;
-    member = memberTypes.back().first;
+    memberType = memberTypes.back().MemberType;
+    member = memberTypes.back().Member;
+    inferredAssocType = memberTypes.back().InferredAssociatedType;
     comp->setValue(member, nullptr);
   }
 
-  return maybeDiagnoseBadMemberType(member, memberType);
+  return maybeDiagnoseBadMemberType(member, memberType, inferredAssocType);
 }
 
 static Type resolveIdentTypeComponent(

lib/Sema/TypeChecker.cpp

@@ -942,8 +942,7 @@ void TypeChecker::diagnoseAmbiguousMemberType(Type baseTy,
       .highlight(baseRange);
   }
   for (const auto &member : lookup) {
-    diagnose(member.first, diag::found_candidate_type,
-             member.second);
+    diagnose(member.Member, diag::found_candidate_type, member.MemberType);
   }
 }
 

lib/Sema/TypeChecker.h

@@ -151,30 +151,37 @@ class LookupResult {
   filter(llvm::function_ref<bool(LookupResultEntry, /*isOuter*/ bool)> pred);
 };
 
+/// An individual result of a name lookup for a type.
+struct LookupTypeResultEntry {
+  TypeDecl *Member;
+  Type MemberType;
+  /// The associated type that the Member/MemberType were inferred for, but only
+  /// if inference happened when creating this entry.
+  AssociatedTypeDecl *InferredAssociatedType;
+};
+
 /// The result of name lookup for types.
 class LookupTypeResult {
   /// The set of results found.
-  SmallVector<std::pair<TypeDecl *, Type>, 4> Results;
+  SmallVector<LookupTypeResultEntry, 4> Results;
 
   friend class TypeChecker;
 
 public:
-  using iterator = SmallVectorImpl<std::pair<TypeDecl *, Type>>::iterator;
+  using iterator = SmallVectorImpl<LookupTypeResultEntry>::iterator;
   iterator begin() { return Results.begin(); }
   iterator end() { return Results.end(); }
   unsigned size() const { return Results.size(); }
 
-  std::pair<TypeDecl *, Type> operator[](unsigned index) const {
+  LookupTypeResultEntry operator[](unsigned index) const {
     return Results[index];
   }
 
-  std::pair<TypeDecl *, Type> front() const { return Results.front(); }
-  std::pair<TypeDecl *, Type> back() const { return Results.back(); }
+  LookupTypeResultEntry front() const { return Results.front(); }
+  LookupTypeResultEntry back() const { return Results.back(); }
 
   /// Add a result to the set of results.
-  void addResult(std::pair<TypeDecl *, Type> result) {
-    Results.push_back(result);
-  }
+  void addResult(LookupTypeResultEntry result) { Results.push_back(result); }
 
   /// \brief Determine whether this result set is ambiguous.
   bool isAmbiguous() const {