Merge pull request #71995 from slavapestov/ncgenerics-fixes-6

Non-copyable generics fixes

Author: slavapestov

include/swift/AST/RequirementSignature.h

@@ -83,6 +83,9 @@ class RequirementSignature final {
              const PrintOptions &Options = PrintOptions()) const;
   void print(ProtocolDecl *owner, ASTPrinter &Printer,
              const PrintOptions &Opts = PrintOptions()) const;
+
+  static RequirementSignature getPlaceholderRequirementSignature(
+      const ProtocolDecl *proto, GenericSignatureErrors errors);
 };
 
 } // end namespace swift

lib/AST/Decl.cpp

@@ -6858,63 +6858,12 @@ ProtocolDecl::getProtocolDependencies() const {
       std::nullopt);
 }
 
-/// If we hit a request cycle, give the protocol a requirement signature that
-/// still has inherited protocol requirements on Self, and also conformances
-/// to Copyable and Escapable for all associated types. Otherwise, we'll see
-/// invariant violations from the inheritance clause mismatch, as well as
-/// spurious downstream diagnostics concerning move-only types.
-static RequirementSignature getPlaceholderRequirementSignature(
-    const ProtocolDecl *proto) {
-  auto &ctx = proto->getASTContext();
-
-  SmallVector<ProtocolDecl *, 2> inheritedProtos;
-  for (auto *inheritedProto : proto->getInheritedProtocols()) {
-    inheritedProtos.push_back(inheritedProto);
-  }
-
-  if (ctx.LangOpts.hasFeature(Feature::NoncopyableGenerics)) {
-    for (auto ip : InvertibleProtocolSet::full()) {
-      auto *otherProto = ctx.getProtocol(getKnownProtocolKind(ip));
-      inheritedProtos.push_back(otherProto);
-    }
-  }
-
-  ProtocolType::canonicalizeProtocols(inheritedProtos);
-
-  SmallVector<Requirement, 2> requirements;
-
-  for (auto *inheritedProto : inheritedProtos) {
-    requirements.emplace_back(RequirementKind::Conformance,
-                              proto->getSelfInterfaceType(),
-                              inheritedProto->getDeclaredInterfaceType());
-  }
-
-  if (ctx.LangOpts.hasFeature(Feature::NoncopyableGenerics)) {
-    for (auto *assocTypeDecl : proto->getAssociatedTypeMembers()) {
-      for (auto ip : InvertibleProtocolSet::full()) {
-        auto *otherProto = ctx.getProtocol(getKnownProtocolKind(ip));
-        requirements.emplace_back(RequirementKind::Conformance,
-                                  assocTypeDecl->getDeclaredInterfaceType(),
-                                  otherProto->getDeclaredInterfaceType());
-      }
-    }
-  }
-
-  // Maintain invariants.
-  llvm::array_pod_sort(requirements.begin(), requirements.end(),
-                       [](const Requirement *lhs, const Requirement *rhs) -> int {
-                         return lhs->compare(*rhs);
-                       });
-
-  return RequirementSignature(ctx.AllocateCopy(requirements),
-                              ArrayRef<ProtocolTypeAlias>());
-}
-
 RequirementSignature ProtocolDecl::getRequirementSignature() const {
   return getASTContext().evaluator(
-               RequirementSignatureRequest { const_cast<ProtocolDecl *>(this) },
+               RequirementSignatureRequest{const_cast<ProtocolDecl *>(this)},
                [this]() {
-                 return getPlaceholderRequirementSignature(this);
+                 return RequirementSignature::getPlaceholderRequirementSignature(
+                     this, GenericSignatureErrors());
                });
 }
 

lib/AST/GenericEnvironment.cpp

@@ -404,7 +404,7 @@ GenericEnvironment::maybeApplyOuterContextSubstitutions(Type type) const {
 
 Type GenericEnvironment::mapTypeIntoContext(GenericEnvironment *env,
                                             Type type) {
-  assert((!type->hasArchetype() || type->hasOpenedExistential()) &&
+  assert((!type->hasArchetype() || type->hasLocalArchetype()) &&
          "already have a contextual type");
   assert((env || !type->hasTypeParameter()) &&
          "no generic environment provided for type with type parameters");

lib/AST/GenericSignature.cpp

@@ -1291,6 +1291,59 @@ void GenericSignatureImpl::getRequirementsWithInverses(
   }
 }
 
+/// If we we can't build a requirement signature because of a request cycle or
+/// failure in Knuth-Bendix completion, we give the protocol a requirement
+/// signature that still has inherited protocol requirements on Self, and also
+/// conformances to Copyable and Escapable for all associated types. Otherwise,
+/// we'll see invariant violations from the inheritance clause mismatch, as
+/// well as spurious downstream diagnostics concerning move-only types.
+RequirementSignature RequirementSignature::getPlaceholderRequirementSignature(
+    const ProtocolDecl *proto, GenericSignatureErrors errors) {
+  auto &ctx = proto->getASTContext();
+
+  SmallVector<ProtocolDecl *, 2> inheritedProtos;
+  for (auto *inheritedProto : proto->getInheritedProtocols()) {
+    inheritedProtos.push_back(inheritedProto);
+  }
+
+  if (ctx.LangOpts.hasFeature(Feature::NoncopyableGenerics)) {
+    for (auto ip : InvertibleProtocolSet::full()) {
+      auto *otherProto = ctx.getProtocol(getKnownProtocolKind(ip));
+      inheritedProtos.push_back(otherProto);
+    }
+  }
+
+  ProtocolType::canonicalizeProtocols(inheritedProtos);
+
+  SmallVector<Requirement, 2> requirements;
+
+  for (auto *inheritedProto : inheritedProtos) {
+    requirements.emplace_back(RequirementKind::Conformance,
+                              proto->getSelfInterfaceType(),
+                              inheritedProto->getDeclaredInterfaceType());
+  }
+
+  if (ctx.LangOpts.hasFeature(Feature::NoncopyableGenerics)) {
+    for (auto *assocTypeDecl : proto->getAssociatedTypeMembers()) {
+      for (auto ip : InvertibleProtocolSet::full()) {
+        auto *otherProto = ctx.getProtocol(getKnownProtocolKind(ip));
+        requirements.emplace_back(RequirementKind::Conformance,
+                                  assocTypeDecl->getDeclaredInterfaceType(),
+                                  otherProto->getDeclaredInterfaceType());
+      }
+    }
+  }
+
+  // Maintain invariants.
+  llvm::array_pod_sort(requirements.begin(), requirements.end(),
+                       [](const Requirement *lhs, const Requirement *rhs) -> int {
+                         return lhs->compare(*rhs);
+                       });
+
+  return RequirementSignature(ctx.AllocateCopy(requirements),
+                              ArrayRef<ProtocolTypeAlias>());
+}
+
 void RequirementSignature::getRequirementsWithInverses(
     ProtocolDecl *owner,
     SmallVector<Requirement, 2> &reqs,

lib/AST/RequirementMachine/GenericSignatureQueries.cpp

@@ -75,6 +75,8 @@ RequirementMachine::getLocalRequirements(
   for (const auto *proto : props->getConformsTo())
     result.protos.push_back(const_cast<ProtocolDecl *>(proto));
 
+  ProtocolType::canonicalizeProtocols(result.protos);
+
   result.layout = props->getLayoutConstraint();
 
   return result;

lib/AST/RequirementMachine/PropertyUnification.cpp

@@ -156,7 +156,9 @@ void RewriteSystem::recordConflict(unsigned existingRuleID,
     // conflicting rules of the same kind, so we rule that out by
     // marking the shorter rule as the conflict. Otherwise, we just
     // leave both rules in place.
-    if (existingRule.getRHS().size() > newRule.getRHS().size()) {
+    if (existingRule.getRHS().size() > newRule.getRHS().size() ||
+        (existingRule.getRHS().size() == newRule.getRHS().size() &&
+         existingRuleID < newRuleID)) {
       existingRule.markConflicting();
     } else {
       newRule.markConflicting();
@@ -192,6 +194,12 @@ void PropertyMap::addLayoutProperty(
   // If the intersection is invalid, we have a conflict.
   if (!mergedLayout->isKnownLayout()) {
     System.recordConflict(*props->LayoutRule, ruleID);
+
+    // Replace the old layout. Since recordConflict() marks the older rule,
+    // this ensures that if we process multiple conflicting layout
+    // requirements, all but the final one will be marked conflicting.
+    props->Layout = newLayout;
+    props->LayoutRule = ruleID;
     return;
   }
 

lib/AST/RequirementMachine/RequirementBuilder.cpp

@@ -192,50 +192,31 @@ void RequirementBuilder::addRequirementRules(ArrayRef<unsigned> rules) {
                           prop->getLayoutConstraint());
         return;
 
-      case Symbol::Kind::Superclass: {
-        // Requirements containing unresolved name symbols originate from
-        // invalid code and should not appear in the generic signature.
-        for (auto term : prop->getSubstitutions()) {
-          if (term.containsUnresolvedSymbols())
-            return;
-        }
-
-        Type superclassType = Map.getTypeFromSubstitutionSchema(
-                                prop->getConcreteType(),
-                                prop->getSubstitutions(),
-                                GenericParams, MutableTerm());
-        if (rule.isRecursive())
-          superclassType = replaceTypeParametersWithErrorTypes(superclassType);
-
-        if (ReconstituteSugar)
-          superclassType = superclassType->reconstituteSugar(/*recursive=*/true);
-
-        Reqs.emplace_back(RequirementKind::Superclass,
-                          subjectType, superclassType);
-        return;
-      }
-
+      case Symbol::Kind::Superclass:
       case Symbol::Kind::ConcreteType: {
-        // Requirements containing unresolved name symbols originate from
-        // invalid code and should not appear in the generic signature.
+        bool containsUnresolvedSymbols = false;
         for (auto term : prop->getSubstitutions()) {
-          if (term.containsUnresolvedSymbols())
-            return;
+          containsUnresolvedSymbols |= term.containsUnresolvedSymbols();
         }
 
         Type concreteType = Map.getTypeFromSubstitutionSchema(
                                 prop->getConcreteType(),
                                 prop->getSubstitutions(),
                                 GenericParams, MutableTerm());
-        if (rule.isRecursive())
+        if (containsUnresolvedSymbols || rule.isRecursive())
           concreteType = replaceTypeParametersWithErrorTypes(concreteType);
 
         if (ReconstituteSugar)
           concreteType = concreteType->reconstituteSugar(/*recursive=*/true);
 
-        auto &component = Components[rule.getRHS()];
-        assert(!component.ConcreteType);
-        component.ConcreteType = concreteType;
+        if (prop->getKind() == Symbol::Kind::Superclass) {
+          Reqs.emplace_back(RequirementKind::Superclass,
+                            subjectType, concreteType);
+        } else {
+          auto &component = Components[rule.getRHS()];
+          assert(!component.ConcreteType);
+          component.ConcreteType = concreteType;
+        }
         return;
       }
 

lib/AST/RequirementMachine/RequirementMachineRequests.cpp

@@ -342,8 +342,8 @@ RequirementSignatureRequest::evaluate(Evaluator &evaluator,
 
   unsigned attempt = 0;
   for (;;) {
-    for (const auto *proto : component) {
-      auto &requirements = protos[proto];
+    for (const auto *otherProto : component) {
+      auto &requirements = protos[otherProto];
 
       // Preprocess requirements to eliminate conformances on type parameters
       // which are made concrete.
@@ -384,11 +384,13 @@ RequirementSignatureRequest::evaluate(Evaluator &evaluator,
         if (otherProto != proto) {
           ctx.evaluator.cacheOutput(
             RequirementSignatureRequest{const_cast<ProtocolDecl *>(otherProto)},
-            RequirementSignature(GenericSignatureErrorFlags::CompletionFailed));
+            RequirementSignature::getPlaceholderRequirementSignature(
+                otherProto, GenericSignatureErrorFlags::CompletionFailed));
         }
       }
 
-      return RequirementSignature(GenericSignatureErrorFlags::CompletionFailed);
+      return RequirementSignature::getPlaceholderRequirementSignature(
+          proto, GenericSignatureErrorFlags::CompletionFailed);
     }
 
     auto minimalRequirements = machine->computeMinimalProtocolRequirements();

lib/AST/Type.cpp

@@ -1118,20 +1118,6 @@ bool TypeBase::isAnyObject() {
   return canTy.getExistentialLayout().isAnyObject();
 }
 
-// Distinguish between class-bound types that might be AnyObject vs other
-// class-bound types. Only types that are potentially AnyObject might have a
-// transparent runtime type wrapper like __SwiftValue. This must look through
-// all optional types because dynamic casting sees through them.
-bool TypeBase::isPotentiallyAnyObject() {
-  Type unwrappedTy = lookThroughAllOptionalTypes();
-  if (auto archetype = unwrappedTy->getAs<ArchetypeType>()) {
-    // Does archetype have any requirements that contradict AnyObject?
-    // 'T : AnyObject' requires a layout constraint, not a conformance.
-    return archetype->getConformsTo().empty() && !archetype->getSuperclass();
-  }
-  return unwrappedTy->isAnyObject();
-}
-
 bool ExistentialLayout::isErrorExistential() const {
   auto protocols = getProtocols();
   return (!hasExplicitAnyObject &&

lib/SIL/Utils/DynamicCasts.cpp

@@ -225,6 +225,22 @@ static CanType getHashableExistentialType(ModuleDecl *M) {
   return hashable->getDeclaredInterfaceType()->getCanonicalType();
 }
 
+// Distinguish between class-bound types that might be AnyObject vs other
+// class-bound types. Only types that are potentially AnyObject might have a
+// transparent runtime type wrapper like __SwiftValue. This must look through
+// all optional types because dynamic casting sees through them.
+static bool isPotentiallyAnyObject(Type type) {
+  Type unwrappedTy = type->lookThroughAllOptionalTypes();
+  if (auto archetype = unwrappedTy->getAs<ArchetypeType>()) {
+    for (auto *proto : archetype->getConformsTo()) {
+      if (!proto->getInvertibleProtocolKind())
+        return false;
+    }
+    return !archetype->getSuperclass();
+  }
+  return unwrappedTy->isAnyObject();
+}
+
 // Returns true if casting \p sourceFormalType to \p targetFormalType preserves
 // ownership.
 //
@@ -321,11 +337,11 @@ bool swift::doesCastPreserveOwnershipForTypes(SILModule &module,
     return false;
 
   // (B2) unwrapping
-  if (sourceType->isPotentiallyAnyObject())
+  if (isPotentiallyAnyObject(sourceType))
     return false;
 
   // (B1) wrapping
-  if (targetType->isPotentiallyAnyObject()) {
+  if (isPotentiallyAnyObject(targetType)) {
     // A class type cannot be wrapped in __SwiftValue, so casting
     // from a class to AnyObject preserves ownership.
     return