Merge pull request #72112 from slavapestov/ncgenerics-fixes-8

Non-copyable generics fixes, part 8

Author: slavapestov

include/swift/AST/DiagnosticsSema.def

@@ -7630,10 +7630,6 @@ NOTE(add_inverse,none,
 NOTE(note_inverse_preventing_conformance,none,
      "%0 has '~%1' constraint preventing implicit '%1' conformance",
      (Type, StringRef))
-NOTE(note_inverse_preventing_conformance_implicit,none,
-     "%kind0 has '~%1' constraint on a generic parameter, "
-     "making its '%1' conformance conditional",
-     (const ValueDecl *, StringRef))
 NOTE(note_inverse_preventing_conformance_explicit,none,
      "%kind0 has '~%1' constraint preventing '%1' conformance",
      (const ValueDecl *, StringRef))

include/swift/AST/InverseMarking.h

@@ -24,7 +24,6 @@ namespace swift {
 struct InverseMarking {
   enum class Kind : uint8_t {
     None,            // No inverse marking is present
-    Inferred,        // Inverse is inferred based on generic parameters.
     Explicit,        // Inverse is explicitly present.
     LegacyExplicit,   // An equivalent, explicit legacy annotation is present.
   };
@@ -84,25 +83,6 @@ struct InverseMarking {
       return Mark(k, loc);
     }
   };
-
-private:
-  Mark inverse;
-  Mark positive;
-
-public:
-
-  // Creates an empty marking.
-  InverseMarking() {}
-
-  Mark const& getInverse() const { return inverse; }
-  Mark const& getPositive() const { return positive; }
-
-  static InverseMarking forInverse(Kind kind, SourceLoc loc = SourceLoc()) {
-    InverseMarking marking;
-    marking.inverse.set(kind, loc);
-    marking.positive.set(Kind::None);
-    return marking;
-  }
 };
 
 }

lib/AST/ConformanceLookup.cpp

@@ -421,7 +421,6 @@ getBuiltinInvertibleProtocolConformance(NominalTypeDecl *nominal,
         // An inverse ~Copyable prevents conformance.
         return ProtocolConformanceRef::forInvalid();
 
-      case InverseMarking::Kind::Inferred: // ignore "inferred" inverse marking
       case InverseMarking::Kind::None:
         break;
       }

lib/AST/Decl.cpp

@@ -6694,9 +6694,9 @@ NominalTypeDecl::hasInverseMarking(InvertibleProtocolKind target) const {
   if (!ctx.LangOpts.hasFeature(Feature::NoncopyableGenerics))
     return InverseMarking::Mark(InverseMarking::Kind::None);
 
-  // Claim that the tuple decl has an inferred ~TARGET marking.
+  // Claim that the tuple decl has an explicit ~TARGET marking.
   if (isa<BuiltinTupleDecl>(this))
-    return InverseMarking::Mark(InverseMarking::Kind::Inferred);
+    return InverseMarking::Mark(InverseMarking::Kind::Explicit);
 
   if (auto P = dyn_cast<ProtocolDecl>(this))
     return P->hasInverseMarking(target);
@@ -6705,57 +6705,6 @@ NominalTypeDecl::hasInverseMarking(InvertibleProtocolKind target) const {
   if (auto inverse = findInverseInInheritance(getInherited(), target))
     return inverse;
 
-  // Check the generic parameters for an explicit ~TARGET marking
-  // which would result in an Inferred ~TARGET marking for this context.
-  auto *gpList = getParsedGenericParams();
-  if (!gpList)
-    return InverseMarking::Mark();
-
-  llvm::SmallSet<GenericTypeParamDecl *, 4> params;
-
-  // Scan the inheritance clauses of generic parameters only for an inverse.
-  for (GenericTypeParamDecl *param : gpList->getParams()) {
-    auto inverse = findInverseInInheritance(param->getInherited(), target);
-
-    // Inverse is inferred from one of the generic parameters.
-    if (inverse)
-      return inverse.with(InverseMarking::Kind::Inferred);
-
-    params.insert(param);
-  }
-
-  // Next, scan the where clause and return the result.
-  auto whereClause = getTrailingWhereClause();
-  if (!whereClause)
-    return InverseMarking::Mark();
-
-  auto requirements = whereClause->getRequirements();
-  for (unsigned i : indices(requirements)) {
-    auto requirementRepr = requirements[i];
-    if (requirementRepr.getKind() != RequirementReprKind::TypeConstraint)
-      continue;
-
-    auto *subjectRepr =
-        dyn_cast<UnqualifiedIdentTypeRepr>(requirementRepr.getSubjectRepr());
-
-    if (!(subjectRepr && subjectRepr->isBound()))
-      continue;
-
-    auto *subjectGP =
-        dyn_cast<GenericTypeParamDecl>(subjectRepr->getBoundDecl());
-    if (!subjectGP || !params.contains(subjectGP))
-      continue;
-
-    auto *constraintRepr =
-        dyn_cast<InverseTypeRepr>(requirementRepr.getConstraintRepr());
-    if (!constraintRepr || constraintRepr->isInvalid())
-      continue;
-
-    if (constraintRepr->isInverseOf(target, getDeclContext()))
-      return InverseMarking::Mark(InverseMarking::Kind::Inferred,
-                                  constraintRepr->getLoc());
-  }
-
   return InverseMarking::Mark();
 }
 

lib/AST/FeatureSet.cpp

@@ -14,7 +14,7 @@
 
 #include "swift/AST/Decl.h"
 #include "swift/AST/ExistentialLayout.h"
-#include "swift/AST/InverseMarking.h"
+#include "swift/AST/NameLookup.h"
 #include "swift/AST/ParameterList.h"
 #include "swift/AST/Pattern.h"
 #include "clang/AST/DeclObjC.h"
@@ -33,62 +33,6 @@ static bool usesTypeMatching(Decl *decl, llvm::function_ref<bool(Type)> fn) {
   return false;
 }
 
-/// \param isRelevantInverse the function used to inspect a mark corresponding
-/// to an inverse to determine whether it "has" an inverse that we care about.
-static bool hasInverse(
-    Decl *decl, InvertibleProtocolKind ip,
-    std::function<bool(InverseMarking::Mark const &)> isRelevantInverse) {
-
-  auto getTypeDecl = [](Type type) -> TypeDecl * {
-    if (auto genericTy = type->getAnyGeneric())
-      return genericTy;
-    if (auto gtpt = dyn_cast<GenericTypeParamType>(type))
-      return gtpt->getDecl();
-    return nullptr;
-  };
-
-  if (auto *extension = dyn_cast<ExtensionDecl>(decl)) {
-    if (auto *nominal = extension->getSelfNominalTypeDecl())
-      return hasInverse(nominal, ip, isRelevantInverse);
-    return false;
-  }
-
-  auto hasInverseInType = [&](Type type) {
-    return type.findIf([&](Type type) -> bool {
-      if (auto *typeDecl = getTypeDecl(type))
-        return hasInverse(typeDecl, ip, isRelevantInverse);
-      return false;
-    });
-  };
-
-  if (auto *TD = dyn_cast<TypeDecl>(decl)) {
-    if (auto *alias = dyn_cast<TypeAliasDecl>(TD))
-      return hasInverseInType(alias->getUnderlyingType());
-
-    if (auto *NTD = dyn_cast<NominalTypeDecl>(TD)) {
-      if (isRelevantInverse(NTD->hasInverseMarking(ip)))
-        return true;
-    }
-
-    if (auto *P = dyn_cast<ProtocolDecl>(TD)) {
-      // Check the protocol's associated types too.
-      return llvm::any_of(
-          P->getAssociatedTypeMembers(), [&](AssociatedTypeDecl *ATD) {
-            return isRelevantInverse(ATD->hasInverseMarking(ip));
-          });
-    }
-
-    return false;
-  }
-
-  if (auto *VD = dyn_cast<ValueDecl>(decl)) {
-    if (VD->hasInterfaceType())
-      return hasInverseInType(VD->getInterfaceType());
-  }
-
-  return false;
-}
-
 // ----------------------------------------------------------------------------
 // MARK: - Standard Features
 // ----------------------------------------------------------------------------
@@ -255,10 +199,39 @@ static bool usesFeatureExtensionMacros(Decl *decl) {
 }
 
 static bool usesFeatureMoveOnly(Decl *decl) {
-  return hasInverse(decl, InvertibleProtocolKind::Copyable,
-                    [](auto &marking) -> bool {
-                      return marking.is(InverseMarking::Kind::LegacyExplicit);
-                    });
+  if (auto *extension = dyn_cast<ExtensionDecl>(decl)) {
+    if (auto *nominal = extension->getExtendedNominal())
+      return usesFeatureMoveOnly(nominal);
+    return false;
+  }
+
+  auto hasInverseInType = [&](Type type) {
+    return type.findIf([&](Type type) -> bool {
+      if (auto *NTD = type->getAnyNominal()) {
+        if (NTD->getAttrs().hasAttribute<MoveOnlyAttr>())
+          return true;
+      }
+      return false;
+    });
+  };
+
+  if (auto *TD = dyn_cast<TypeDecl>(decl)) {
+    if (auto *alias = dyn_cast<TypeAliasDecl>(TD))
+      return hasInverseInType(alias->getUnderlyingType());
+
+    if (auto *NTD = dyn_cast<NominalTypeDecl>(TD)) {
+      if (NTD->getAttrs().hasAttribute<MoveOnlyAttr>())
+        return true;
+    }
+
+    return false;
+  }
+
+  if (auto *VD = dyn_cast<ValueDecl>(decl)) {
+    return hasInverseInType(VD->getInterfaceType());
+  }
+
+  return false;
 }
 
 static bool usesFeatureMoveOnlyResilientTypes(Decl *decl) {
@@ -525,22 +498,49 @@ static bool usesFeatureRawLayout(Decl *decl) {
 UNINTERESTING_FEATURE(Embedded)
 
 static bool usesFeatureNoncopyableGenerics(Decl *decl) {
-  auto checkInverseMarking = [](auto &marking) -> bool {
-    switch (marking.getKind()) {
-    case InverseMarking::Kind::None:
-    case InverseMarking::Kind::LegacyExplicit: // covered by other checks.
-      return false;
+  if (auto *valueDecl = dyn_cast<ValueDecl>(decl)) {
+    if (isa<StructDecl, EnumDecl, ClassDecl>(decl)) {
+      auto *nominalDecl = cast<NominalTypeDecl>(valueDecl);
 
-    case InverseMarking::Kind::Explicit:
-    case InverseMarking::Kind::Inferred:
-      return true;
+      InvertibleProtocolSet inverses;
+      bool anyObject = false;
+      getDirectlyInheritedNominalTypeDecls(nominalDecl, inverses, anyObject);
+      if (!inverses.empty())
+        return true;
     }
-  };
 
-  return hasInverse(decl, InvertibleProtocolKind::Copyable,
-                    checkInverseMarking) ||
-         hasInverse(decl, InvertibleProtocolKind::Escapable,
-                    checkInverseMarking);
+    if (isa<AbstractFunctionDecl>(valueDecl) ||
+        isa<AbstractStorageDecl>(valueDecl)) {
+      if (valueDecl->getInterfaceType().findIf([&](Type type) -> bool {
+            if (auto *nominalDecl = type->getAnyNominal()) {
+              if (isa<StructDecl, EnumDecl, ClassDecl>(nominalDecl))
+                return usesFeatureNoncopyableGenerics(nominalDecl);
+            }
+            return false;
+          })) {
+        return true;
+      }
+    }
+  }
+
+  if (auto *ext = dyn_cast<ExtensionDecl>(decl)) {
+    if (auto *nominal = ext->getExtendedNominal())
+      if (usesFeatureNoncopyableGenerics(nominal))
+        return true;
+  }
+
+  SmallVector<Requirement, 2> reqs;
+  SmallVector<InverseRequirement, 2> inverseReqs;
+
+  if (auto *proto = dyn_cast<ProtocolDecl>(decl)) {
+    proto->getRequirementSignature().getRequirementsWithInverses(
+        proto, reqs, inverseReqs);
+  } else if (auto *genCtx = decl->getAsGenericContext()) {
+    if (auto genericSig = genCtx->getGenericSignature())
+      genericSig->getRequirementsWithInverses(reqs, inverseReqs);
+  }
+
+  return !inverseReqs.empty();
 }
 
 static bool usesFeatureStructLetDestructuring(Decl *decl) {

lib/AST/RequirementMachine/KnuthBendix.cpp

@@ -325,10 +325,8 @@ RewriteSystem::computeCriticalPair(ArrayRef<Symbol>::const_iterator from,
   return true;
 }
 
-/// Computes the confluent completion using the Knuth-Bendix algorithm and
-/// returns a status code.
-///
-/// The first element of the pair is a status.
+/// Runs the Knuth-Bendix algorithm and returns a pair consisting of a
+/// status code and code-specific result.
 ///
 /// The status is CompletionResult::MaxRuleCount if we add more than
 /// \p maxRuleCount rules.
@@ -341,8 +339,8 @@ RewriteSystem::computeCriticalPair(ArrayRef<Symbol>::const_iterator from,
 /// Otherwise, the status is CompletionResult::Success and the second element
 /// is zero.
 std::pair<CompletionResult, unsigned>
-RewriteSystem::computeConfluentCompletion(unsigned maxRuleCount,
-                                          unsigned maxRuleLength) {
+RewriteSystem::performKnuthBendix(unsigned maxRuleCount,
+                                  unsigned maxRuleLength) {
   assert(Initialized);
   assert(!Minimized);
   assert(!Frozen);

lib/AST/RequirementMachine/RequirementMachine.cpp

@@ -449,7 +449,7 @@ RequirementMachine::computeCompletion(RewriteSystem::ValidityPolicy policy) {
       unsigned ruleCount = System.getRules().size();
 
       // First, run the Knuth-Bendix algorithm to resolve overlapping rules.
-      auto result = System.computeConfluentCompletion(MaxRuleCount, MaxRuleLength);
+      auto result = System.performKnuthBendix(MaxRuleCount, MaxRuleLength);
 
       unsigned rulesAdded = (System.getRules().size() - ruleCount);
 
@@ -492,7 +492,7 @@ RequirementMachine::computeCompletion(RewriteSystem::ValidityPolicy policy) {
           return std::make_pair(CompletionResult::MaxRuleLength,
                                 ruleCount + i);
         }
-        if (newRule.getNesting() > MaxConcreteNesting) {
+        if (newRule.getNesting() > MaxConcreteNesting + System.getDeepestInitialRule()) {
           return std::make_pair(CompletionResult::MaxConcreteNesting,
                                 ruleCount + i);
         }

lib/AST/RequirementMachine/RewriteSystem.cpp

@@ -33,6 +33,7 @@ RewriteSystem::RewriteSystem(RewriteContext &ctx)
   Frozen = 0;
   RecordLoops = 0;
   LongestInitialRule = 0;
+  DeepestInitialRule = 0;
 }
 
 RewriteSystem::~RewriteSystem() {
@@ -88,6 +89,7 @@ void RewriteSystem::initialize(
 
   for (const auto &rule : getLocalRules()) {
     LongestInitialRule = std::max(LongestInitialRule, rule.getDepth());
+    DeepestInitialRule = std::max(DeepestInitialRule, rule.getNesting());
   }
 }