Merge pull request #71659 from xedin/noncopyable-circularity-fixes

[AST/Sema] NonCopyableGenerics: Address some of the request circularity issues

Author: xedin

include/swift/AST/Decl.h

@@ -32,6 +32,7 @@
 #include "swift/AST/IfConfigClause.h"
 #include "swift/AST/Import.h"
 #include "swift/AST/Initializer.h"
+#include "swift/AST/InverseMarking.h"
 #include "swift/AST/LayoutConstraint.h"
 #include "swift/AST/LifetimeAnnotation.h"
 #include "swift/AST/ReferenceCounting.h"
@@ -3219,30 +3220,6 @@ class TypeDecl : public ValueDecl {
     };
   };
 
-  /// "Does a conformance for Copyable exist for this type declaration?"
-  ///
-  /// This doesn't mean that all instance of this type are Copyable, because
-  /// if a conditional conformance to Copyable exists, this method will return
-  /// true.
-  ///
-  /// If you need a more precise answer, ask this Decl's corresponding
-  /// Type if it `isCopyable` instead of using this.
-  CanBeInvertible::Result canBeCopyable() const;
-
-  /// "Does a conformance for Escapable exist for this type declaration?"
-  ///
-  /// This doesn't mean that all instance of this type are Escapable, because
-  /// if a conditional conformance to Escapable exists, this method will return
-  /// true.
-  ///
-  /// If you need a more precise answer, ask this Decl's corresponding
-  /// Type if it `isEscapable` instead of using this.
-  CanBeInvertible::Result canBeEscapable() const;
-
-  /// Determine how the given invertible protocol was written on this TypeDecl,
-  /// if at all.
-  InverseMarking getMarking(InvertibleProtocolKind ip) const;
-
   static bool classof(const Decl *D) {
     return D->getKind() >= DeclKind::First_TypeDecl &&
            D->getKind() <= DeclKind::Last_TypeDecl;
@@ -3260,7 +3237,7 @@ class TypeDecl : public ValueDecl {
   }
 };
 
-/// A type declaration that can have generic parameters attached to it. Because
+/// A type declaration that  have generic parameters attached to it. Because
 /// it has these generic parameters, it is always a DeclContext.
 class GenericTypeDecl : public GenericContext, public TypeDecl {
 public:
@@ -3935,6 +3912,10 @@ class AssociatedTypeDecl : public TypeDecl {
         TypeDecl::getOverriddenDecl());
   }
 
+  /// Determine whether this type has ~<target>` stated as
+  /// one of its inherited types.
+  InverseMarking::Mark hasInverseMarking(InvertibleProtocolKind target) const;
+
   /// Retrieve the set of associated types overridden by this associated
   /// type.
   llvm::TinyPtrVector<AssociatedTypeDecl *> getOverriddenDecls() const;
@@ -4372,6 +4353,34 @@ class NominalTypeDecl : public GenericTypeDecl, public IterableDeclContext {
   /// Returns null if the type is a class, or does not have a declared `deinit`.
   DestructorDecl *getValueTypeDestructor();
 
+  /// "Does a conformance for Copyable exist for this type declaration?"
+  ///
+  /// This doesn't mean that all instance of this type are Copyable, because
+  /// if a conditional conformance to Copyable exists, this method will return
+  /// true.
+  ///
+  /// If you need a more precise answer, ask this Decl's corresponding
+  /// Type if it `isCopyable` instead of using this.
+  CanBeInvertible::Result canBeCopyable() const;
+
+  /// "Does a conformance for Escapable exist for this type declaration?"
+  ///
+  /// This doesn't mean that all instance of this type are Escapable, because
+  /// if a conditional conformance to Escapable exists, this method will return
+  /// true.
+  ///
+  /// If you need a more precise answer, ask this Decl's corresponding
+  /// Type if it `isEscapable` instead of using this.
+  CanBeInvertible::Result canBeEscapable() const;
+
+  /// Determine whether this type has `: <target>` stated explicitly in
+  /// its inheritance clause.
+  bool hasMarking(InvertibleProtocolKind target) const;
+
+  /// Determine whether this type has ~<target>` stated on
+  /// itself, one of its inherited types or `Self` requirements.
+  InverseMarking::Mark hasInverseMarking(InvertibleProtocolKind target) const;
+
   // Implement isa/cast/dyncast/etc.
   static bool classof(const Decl *D) {
     return D->getKind() >= DeclKind::First_NominalTypeDecl &&
@@ -5227,6 +5236,10 @@ class ProtocolDecl final : public NominalTypeDecl {
   /// protocol.
   bool inheritsFrom(const ProtocolDecl *Super) const;
 
+  /// Determine whether this protocol has ~<target>` stated on
+  /// itself, one of its inherited types or `Self` requirements.
+  InverseMarking::Mark hasInverseMarking(InvertibleProtocolKind target) const;
+
   /// Determine whether this protocol requires conformance to `IP`, without
   /// querying a generic signature.
   bool requiresInvertible(InvertibleProtocolKind ip) const;

include/swift/AST/InverseMarking.h

@@ -19,8 +19,6 @@
 
 namespace swift {
 
-class InvertibleAnnotationRequest;
-
 /// Describes the way an inverse and its corresponding positive contraint
 /// appears on a TypeDecl, i.e., the way it was marked.
 struct InverseMarking {
@@ -91,8 +89,6 @@ struct InverseMarking {
   Mark inverse;
   Mark positive;
 
-  // This friend initializes the marks.
-  friend InvertibleAnnotationRequest;
 public:
 
   // Creates an empty marking.

include/swift/AST/TypeCheckRequests.h

@@ -429,28 +429,6 @@ class IsFinalRequest :
   void cacheResult(bool value) const;
 };
 
-/// Determine the kind of invertible protocol markings for this declaration,
-/// for example, if a conformance to IP or ~IP was written on it in the
-/// inheritance clause and/or where clause.
-class InvertibleAnnotationRequest
-    : public SimpleRequest<InvertibleAnnotationRequest,
-                           InverseMarking(TypeDecl *, InvertibleProtocolKind),
-                           RequestFlags::Cached> {
-public:
-  using SimpleRequest::SimpleRequest;
-
-private:
-  friend SimpleRequest;
-
-  // Evaluation.
-  InverseMarking evaluate(Evaluator &evaluator,
-                          TypeDecl *decl, InvertibleProtocolKind ip) const;
-
-public:
-  // Caching.
-  bool isCached() const { return true; }
-};
-
 /// Determine whether the given declaration is 'dynamic''.
 class IsDynamicRequest :
     public SimpleRequest<IsDynamicRequest,

include/swift/AST/TypeCheckerTypeIDZone.def

@@ -217,9 +217,6 @@ SWIFT_REQUEST(TypeChecker, IsDynamicRequest, bool(ValueDecl *),
               SeparatelyCached, NoLocationInfo)
 SWIFT_REQUEST(TypeChecker, IsFinalRequest, bool(ValueDecl *), SeparatelyCached,
               NoLocationInfo)
-SWIFT_REQUEST(TypeChecker, InvertibleAnnotationRequest,
-              InverseMarking(TypeDecl *, InvertibleProtocolKind),
-              Cached, NoLocationInfo)
 SWIFT_REQUEST(TypeChecker, IsGetterMutatingRequest, bool(AbstractStorageDecl *),
               SeparatelyCached, NoLocationInfo)
 SWIFT_REQUEST(TypeChecker, IsImplicitlyUnwrappedOptionalRequest,

include/swift/AST/TypeRepr.h

@@ -137,6 +137,10 @@ class alignas(1 << TypeReprAlignInBits) TypeRepr
   /// Is this type representation a protocol?
   bool isProtocolOrProtocolComposition(DeclContext *dc);
 
+  /// Is this `~<target>` representation.
+  bool isInverseOf(InvertibleProtocolKind target,
+                   DeclContext *dc);
+
   /// Is this type representation known to be invalid?
   bool isInvalid() const { return Bits.TypeRepr.Invalid; }
 

lib/AST/ASTPrinter.cpp

@@ -3407,9 +3407,8 @@ static bool usesFeatureFlowSensitiveConcurrencyCaptures(Decl *decl) {
 /// \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 const&)> isRelevantInverse) {
+    Decl *decl, InvertibleProtocolKind ip,
+    std::function<bool(InverseMarking::Mark const &)> isRelevantInverse) {
 
   auto getTypeDecl = [](Type type) -> TypeDecl* {
     if (auto genericTy = type->getAnyGeneric())
@@ -3421,40 +3420,41 @@ static bool hasInverse(
 
   if (auto *extension = dyn_cast<ExtensionDecl>(decl)) {
     if (auto *nominal = extension->getSelfNominalTypeDecl())
-      if (isRelevantInverse(nominal->getMarking(ip)))
-        return true;
+      return hasInverse(nominal, ip, isRelevantInverse);
+    return false;
   }
 
-  if (auto typeDecl = dyn_cast<TypeDecl>(decl)) {
-    if (isRelevantInverse(typeDecl->getMarking(ip)))
-      return true;
+  auto hasInverseInType = [&](Type type) {
+    return type.findIf([&](Type type) -> bool {
+      if (auto *typeDecl = getTypeDecl(type))
+        return hasInverse(typeDecl, ip, isRelevantInverse);
+      return false;
+    });
+  };
 
-    // Check the protocol's associated types too.
-    if (auto proto = dyn_cast<ProtocolDecl>(decl)) {
-      auto hasInverse =
-          llvm::any_of(proto->getAssociatedTypeMembers(),
-                       [&](AssociatedTypeDecl *assocTyDecl) {
-                         return isRelevantInverse(assocTyDecl->getMarking(ip));
-                       });
-      if (hasInverse)
+  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 value = dyn_cast<ValueDecl>(decl)) {
-    // Check for noncopyable types in the types of this declaration.
-    if (Type type = value->getInterfaceType()) {
-      bool hasInverse = type.findIf([&](Type type) {
-        if (auto *typeDecl = getTypeDecl(type))
-          if (isRelevantInverse(typeDecl->getMarking(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;
-      });
+    return false;
+  }
 
-      if (hasInverse)
-        return true;
-    }
+  if (auto *VD = dyn_cast<ValueDecl>(decl)) {
+    if (VD->hasInterfaceType())
+      return hasInverseInType(VD->getInterfaceType());
   }
 
   return false;
@@ -3463,8 +3463,8 @@ static bool hasInverse(
 static bool usesFeatureMoveOnly(Decl *decl) {
   return hasInverse(decl, InvertibleProtocolKind::Copyable,
                     [](auto &marking) -> bool {
-    return marking.getInverse().is(InverseMarking::Kind::LegacyExplicit);
-  });
+                      return marking.is(InverseMarking::Kind::LegacyExplicit);
+                    });
 }
 
 static bool usesFeatureLazyImmediate(Decl *D) { return false; }
@@ -3508,8 +3508,8 @@ static bool usesFeatureMoveOnlyPartialReinitialization(Decl *decl) {
 }
 
 static bool usesFeatureNoncopyableGenerics(Decl *decl) {
-  auto checkMarking = [](auto &marking) -> bool {
-    switch (marking.getInverse().getKind()) {
+  auto checkInverseMarking = [](auto &marking) -> bool {
+    switch (marking.getKind()) {
     case InverseMarking::Kind::None:
     case InverseMarking::Kind::LegacyExplicit: // covered by other checks.
       return false;
@@ -3520,8 +3520,10 @@ static bool usesFeatureNoncopyableGenerics(Decl *decl) {
     }
   };
 
-  return hasInverse(decl, InvertibleProtocolKind::Copyable, checkMarking)
-      || hasInverse(decl, InvertibleProtocolKind::Escapable, checkMarking);
+  return hasInverse(decl, InvertibleProtocolKind::Copyable,
+                    checkInverseMarking) ||
+         hasInverse(decl, InvertibleProtocolKind::Escapable,
+                    checkInverseMarking);
 }
 
 static bool usesFeatureOneWayClosureParameters(Decl *decl) {

lib/AST/ConformanceLookup.cpp

@@ -415,8 +415,7 @@ getBuiltinInvertibleProtocolConformance(NominalTypeDecl *nominal,
   case InvertibleProtocolKind::Copyable:
     // If move-only classes is enabled, we'll check the markings.
     if (ctx.LangOpts.hasFeature(Feature::MoveOnlyClasses)) {
-      auto marking = nominal->getMarking(*ip);
-      switch (marking.getInverse().getKind()) {
+      switch (nominal->hasInverseMarking(*ip).getKind()) {
       case InverseMarking::Kind::LegacyExplicit:
       case InverseMarking::Kind::Explicit:
         // An inverse ~Copyable prevents conformance.