Merge pull request #74672 from kavon/post-vacation-misc-cleanups

Handful of small NoncopyableGenerics fixes / cleanups.

Author: kavon

docs/ReferenceGuides/UnderscoredAttributes.md

@@ -917,6 +917,46 @@ More generally, multiple availabilities can be specified, like so:
 enum Toast { ... }
 ```
 
+## `@_preInverseGenerics`
+
+By default when mangling a generic signature, the presence of a conformance 
+requirement for an invertible protocol, like Copyable and Escapable, is not
+explicitly mangled. Only the _absence_ of those conformance requirements for
+each generic parameter appears in the mangled name.
+
+This attribute changes the way generic signatures are mangled, by ignoring
+even the absences of those conformance requirements for invertible protocols.
+So, the following functions would have the same mangling because of the
+attribute:
+
+```swift
+@_preInverseGenerics
+func foo<T: ~Copyable>(_ t: borrowing T) {}
+
+// In 'bug.swift', the function above without the attribute would be:
+//
+//   $s3bug3fooyyxRi_zlF ---> bug.foo<A where A: ~Swift.Copyable>(A) -> ()
+//
+// With the attribute, the above becomes:
+//
+//   $s3bug3fooyyxlF ---> bug.foo<A>(A) -> ()
+//
+// which is exactly the same symbol for the function below.
+
+func foo<T>(_ t: T) {}
+```
+
+The purpose of this attribute is to aid in adopting noncopyable generics
+(SE-427) in existing libraries without breaking ABI; it is for advanced users
+only.
+
+> **WARNING:** Before applying this attribute, you _must manually verify_ that
+> there never were any implementations of `foo` that contained a copy of `t`, 
+> to ensure correctness. There is no way to prove this by simply inspecting the
+> Swift source code! You actually have to **check the assembly code** in all of
+> your existing libraries containing `foo`, because an older version of the
+> Swift compiler could have decided to insert a copy of `t` as an optimization!
+
 ## `@_private(sourceFile: "FileName.swift")`
 
 Fully bypasses access control, allowing access to private declarations

include/swift/AST/ASTPrinter.h

@@ -107,6 +107,29 @@ enum class PrintStructureKind {
   FunctionParameterType,
 };
 
+/// ---------------------------------
+/// MARK: inverse filtering functors
+
+/// An inverse filter is just a function-object. Use one of the functors below
+/// to create such a filter.
+using InverseFilter = std::function<bool(const InverseRequirement &)>;
+
+/// Include all of them!
+class AllInverses {
+public:
+  bool operator()(const InverseRequirement &) const { return true; }
+};
+
+/// Only prints inverses on generic parameters defined in the specified
+/// generic context.
+class InversesAtDepth {
+  std::optional<unsigned> includedDepth;
+public:
+  InversesAtDepth(GenericContext *level);
+  bool operator()(const InverseRequirement &) const;
+};
+/// ---------------------------------
+
 /// An abstract class used to print an AST.
 class ASTPrinter {
   unsigned CurrentIndentation = 0;

include/swift/AST/Decl.h

@@ -1919,6 +1919,10 @@ class ExtensionDecl final : public GenericContext, public Decl,
   /// \endcode
   bool isWrittenWithConstraints() const;
 
+  /// Does this extension add conformance to an invertible protocol for the
+  /// extended type?
+  bool isAddingConformanceToInvertible() const;
+
   /// If this extension represents an imported Objective-C category, returns the
   /// category's name. Otherwise returns the empty identifier.
   Identifier getObjCCategoryName() const;

include/swift/AST/DiagnosticsSema.def

@@ -7719,8 +7719,8 @@ ERROR(inverse_associatedtype_restriction, none,
       "cannot suppress '%0' requirement of an associated type",
       (StringRef))
 ERROR(inverse_on_class, none,
-      "classes cannot be '~%0'",
-      (StringRef))
+      "%select{classes|actors}1 cannot be '~%0'",
+      (StringRef, bool))
 ERROR(inverse_with_class_constraint, none,
       "composition involving %select{class requirement %2|'AnyObject'}0 "
       "cannot contain '~%1'",

lib/AST/ASTPrinter.cpp

@@ -974,7 +974,6 @@ class PrintAST : public ASTVisitor<PrintAST> {
     SwapSelfAndDependentMemberType = 8,
     PrintInherited = 16,
     PrintInverseRequirements = 32,
-    IncludeOuterInverses = 64,
   };
 
   /// The default generic signature flags for printing requirements.
@@ -1004,7 +1003,8 @@ class PrintAST : public ASTVisitor<PrintAST> {
   void
   printGenericSignature(GenericSignature genericSig,
                         unsigned flags,
-                        llvm::function_ref<bool(const Requirement &)> filter);
+                        llvm::function_ref<bool(const Requirement &)> filter,
+                        InverseFilter inverseFilter);
   void printSingleDepthOfGenericSignature(
       ArrayRef<GenericTypeParamType *> genericParams,
       ArrayRef<Requirement> requirements,
@@ -1693,9 +1693,13 @@ static unsigned getDepthOfRequirement(const Requirement &req) {
 
 void PrintAST::printGenericSignature(GenericSignature genericSig,
                                      unsigned flags) {
+  ASSERT(!((flags & InnermostOnly) && (flags & PrintInverseRequirements))
+         && "InnermostOnly + PrintInverseRequirements is not handled");
+
   printGenericSignature(genericSig, flags,
                         // print everything
-                        [&](const Requirement &) { return true; });
+                        [&](const Requirement &) { return true; },
+                        AllInverses());
 }
 
 // Erase any requirements involving invertible protocols.
@@ -1710,16 +1714,35 @@ static void eraseInvertibleProtocolConformances(
     });
 }
 
+InversesAtDepth::InversesAtDepth(GenericContext *level) {
+  includedDepth = std::nullopt;
+  // Does this generic context have its own generic parameters?
+  if (auto *list = level->getGenericParams()) {
+    includedDepth = list->getParams().back()->getDepth(); // use this depth.
+  }
+}
+bool InversesAtDepth::operator()(const InverseRequirement &inverse) const {
+  if (includedDepth) {
+    auto d = inverse.subject->castTo<GenericTypeParamType>()->getDepth();
+    return d == includedDepth.value();
+  }
+  return false;
+}
+
 void PrintAST::printGenericSignature(
     GenericSignature genericSig,
     unsigned flags,
-    llvm::function_ref<bool(const Requirement &)> filter) {
+    llvm::function_ref<bool(const Requirement &)> filter,
+    InverseFilter inverseFilter) {
 
   SmallVector<Requirement, 2> requirements;
   SmallVector<InverseRequirement, 2> inverses;
 
   if (flags & PrintInverseRequirements) {
     genericSig->getRequirementsWithInverses(requirements, inverses);
+    llvm::erase_if(inverses, [&](InverseRequirement inverse) -> bool {
+      return !inverseFilter(inverse);
+    });
   } else {
     requirements.append(genericSig.getRequirements().begin(),
                         genericSig.getRequirements().end());
@@ -1728,17 +1751,6 @@ void PrintAST::printGenericSignature(
       eraseInvertibleProtocolConformances(requirements);
   }
 
-  // Unless `IncludeOuterInverses` is enabled, limit inverses to the
-  // innermost generic parameters.
-  if (!(flags & IncludeOuterInverses) && !inverses.empty()) {
-    auto innerParams = genericSig.getInnermostGenericParams();
-    SmallPtrSet<TypeBase *, 4> innerParamSet(innerParams.begin(),
-                                             innerParams.end());
-    llvm::erase_if(inverses, [&](InverseRequirement inverse) -> bool {
-      return !innerParamSet.contains(inverse.subject.getPointer());
-    });
-  }
-
   if (flags & InnermostOnly) {
     auto genericParams = genericSig.getInnermostGenericParams();
 
@@ -2772,8 +2784,9 @@ void PrintAST::printDeclGenericRequirements(GenericContext *decl) {
   // In many cases, inverses should not be printed for outer generic parameters.
   // Exceptions to that include extensions, as it's valid to write an inverse
   // on the generic parameters they get from the extended nominal.
-  if (isa<ExtensionDecl>(decl))
-    flags |= IncludeOuterInverses;
+  InverseFilter inverseFilter = AllInverses();
+  if (!isa<ExtensionDecl>(decl))
+    inverseFilter = InversesAtDepth(decl);
 
   Printer.printStructurePre(PrintStructureKind::DeclGenericParameterClause);
   printGenericSignature(genericSig,
@@ -2782,7 +2795,8 @@ void PrintAST::printDeclGenericRequirements(GenericContext *decl) {
                           if (parentSig)
                             return !parentSig->isRequirementSatisfied(req);
                           return true;
-                        });
+                        },
+                        inverseFilter);
   Printer.printStructurePost(PrintStructureKind::DeclGenericParameterClause);
 }
 
@@ -2916,20 +2930,6 @@ void PrintAST::printExtendedTypeName(TypeLoc ExtendedTypeLoc) {
   printTypeLoc(TypeLoc(ExtendedTypeLoc.getTypeRepr(), Ty));
 }
 
-/// If an extension adds a conformance for an invertible protocol, then we
-/// should not print inverses for its generic signature, because no conditional
-/// requirements are inferred by default for such an extension.
-static bool isExtensionAddingInvertibleConformance(const ExtensionDecl *ext) {
-  auto conformances = ext->getLocalConformances();
-  for (auto *conf : conformances) {
-    if (conf->getProtocol()->getInvertibleProtocolKind()) {
-      assert(conformances.size() == 1 && "expected solo conformance");
-      return true;
-    }
-  }
-  return false;
-}
-
 void PrintAST::printSynthesizedExtension(Type ExtendedType,
                                          ExtensionDecl *ExtDecl) {
   if (Options.PrintCompatibilityFeatureChecks &&
@@ -3054,24 +3054,22 @@ void PrintAST::printExtension(ExtensionDecl *decl) {
       assert(baseGenericSig &&
              "an extension can't be generic if the base type isn't");
 
-      auto genSigFlags = defaultGenericRequirementFlags()
-                       | IncludeOuterInverses;
+      auto genSigFlags = defaultGenericRequirementFlags();
 
       // Disable printing inverses if the extension is adding a conformance
       // for an invertible protocol itself, as we do not infer any requirements
       // in such an extension. We need to print the whole signature:
       //     extension S: Copyable where T: Copyable
-      if (isExtensionAddingInvertibleConformance(decl)) {
+      if (decl->isAddingConformanceToInvertible())
         genSigFlags &= ~PrintInverseRequirements;
-        genSigFlags &= ~IncludeOuterInverses;
-      }
 
       printGenericSignature(genericSig,
                             genSigFlags,
                             [baseGenericSig](const Requirement &req) -> bool {
         // Only include constraints that are not satisfied by the base type.
         return !baseGenericSig->isRequirementSatisfied(req);
-      });
+      },
+      AllInverses());
     }
   }
   if (Options.TypeDefinitions) {

lib/AST/Decl.cpp

@@ -1892,6 +1892,30 @@ Type ExtensionDecl::getExtendedType() const {
   return ErrorType::get(ctx);
 }
 
+bool ExtensionDecl::isAddingConformanceToInvertible() const {
+  const unsigned numEntries = getInherited().size();
+  for (unsigned i = 0; i < numEntries; ++i) {
+    InheritedTypeRequest request{this, i, TypeResolutionStage::Structural};
+    auto result = evaluateOrDefault(getASTContext().evaluator, request,
+                                    InheritedTypeResult::forDefault());
+    Type inheritedTy;
+    switch (result) {
+    case InheritedTypeResult::Inherited:
+      inheritedTy = result.getInheritedType();
+      break;
+    case InheritedTypeResult::Suppressed:
+    case InheritedTypeResult::Default:
+      continue;
+    }
+
+    if (inheritedTy)
+      if (auto kp = inheritedTy->getKnownProtocol())
+        if (getInvertibleProtocolKind(*kp))
+          return true;
+  }
+  return false;
+}
+
 bool Decl::isObjCImplementation() const {
   return getAttrs().hasAttribute<ObjCImplementationAttr>(/*AllowInvalid=*/true);
 }

lib/Sema/TypeCheckDeclPrimary.cpp

@@ -3349,7 +3349,8 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
 
       ctx.Diags.diagnose(decl->getLoc(),
                          diag::inverse_on_class,
-                         getProtocolName(getKnownProtocolKind(ip)));
+                         getProtocolName(getKnownProtocolKind(ip)),
+                         decl->isAnyActor());
     }
   }
 

lib/Sema/TypeCheckGeneric.cpp

@@ -783,34 +783,10 @@ GenericSignatureRequest::evaluate(Evaluator &evaluator,
   } else if (auto *ext = dyn_cast<ExtensionDecl>(GC)) {
     loc = ext->getLoc();
 
-    // The inherited entries influence the generic signature of the extension,
-    // because if it introduces conformance to invertible protocol IP, we do not
-    // we do not infer any requirements that the generic parameters to conform
+    // If the extension introduces conformance to invertible protocol IP, do not
+    // infer any conditional requirements that the generic parameters to conform
     // to invertible protocols. This forces people to write out the conditions.
-    const unsigned numEntries = ext->getInherited().size();
-    for (unsigned i = 0; i < numEntries; ++i) {
-      InheritedTypeRequest request{ext, i, TypeResolutionStage::Structural};
-      auto result = evaluateOrDefault(ctx.evaluator, request,
-                                      InheritedTypeResult::forDefault());
-      Type inheritedTy;
-      switch (result) {
-      case InheritedTypeResult::Inherited:
-        inheritedTy = result.getInheritedType();
-        break;
-      case InheritedTypeResult::Suppressed:
-      case InheritedTypeResult::Default:
-        continue;
-      }
-
-      if (inheritedTy) {
-        if (auto kp = inheritedTy->getKnownProtocol()) {
-          if (getInvertibleProtocolKind(*kp)) {
-            inferInvertibleReqs = false;
-            break;
-          }
-        }
-      }
-    }
+    inferInvertibleReqs = !ext->isAddingConformanceToInvertible();
 
     collectAdditionalExtensionRequirements(ext->getExtendedType(), extraReqs);
 

test/Generics/inverse_classes2.swift

@@ -1,13 +1,12 @@
-// RUN: %target-typecheck-verify-swift \
-// RUN:   -parse-stdlib -module-name Swift
-
-// NOTE: -parse-stdlib is a transitional workaround and should not be required.
+// RUN: %target-typecheck-verify-swift -disable-availability-checking
 
 @_moveOnly // expected-error {{'@_moveOnly' attribute is only valid on structs or enums}}
 class KlassLegacy {}
 
 class KlassModern: ~Copyable {} // expected-error {{classes cannot be '~Copyable'}}
 
+actor FamousPerson: ~Copyable {} // expected-error{{actors cannot be '~Copyable'}}
+
 class Konditional<T: ~Copyable> {}
 
 func checks<T: ~Copyable, C>(

test/ModuleInterface/Inputs/NoncopyableGenerics_Misc.swift

@@ -137,3 +137,8 @@ public struct Generic<T: Publik & ~Copyable> : (P & ~Copyable) {}
 public struct VeryNested: (P & (Q & ~Copyable & Publik) & (P & ~Copyable)) {}
 public struct Twice: P & ~Copyable, Q & ~Copyable {}
 public struct RegularTwice: ~Copyable, ~Copyable {}
+
+// coverage for rdar://130179698
+public struct Continuation<T: ~Copyable, E: Error> {
+  public func resume(returning value: consuming T) where E == Never {}
+}

test/ModuleInterface/noncopyable_generics.swift

@@ -215,6 +215,11 @@ import NoncopyableGenerics_Misc
 // CHECK-MISC-NEXT: public struct RegularTwice : ~Swift.Copyable, ~Swift.Copyable {
 // CHECK-MISC-NEXT: }
 
+// CHECK-MISC-NEXT: #if compiler(>=5.3) && $NoncopyableGenerics
+// CHECK-MISC-NEXT: public struct Continuation<T, E> where E : Swift.Error, T : ~Copyable {
+// CHECK-MISC-NOT:  ~
+// CHECK-MISC:      #endif
+
 // NOTE: below are extensions emitted at the end of NoncopyableGenerics_Misc.swift
 // CHECK-MISC: extension {{.*}}.VeryNested : {{.*}}.Publik {}