Merge pull request swiftlang#74744 from kavon/combo-fixes-6-26-2024

[6.0🍒] NCGenerics reverse condfail fix + bonus content!

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

@@ -1912,6 +1912,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;
+
   /// Returns the name of the category specified by the \c \@_objcImplementation
   /// attribute, or \c None if the name is invalid or
   /// \c isObjCImplementation() is false.

include/swift/AST/DiagnosticsSema.def

@@ -7672,8 +7672,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'",

include/swift/Basic/Features.def

@@ -387,6 +387,8 @@ EXPERIMENTAL_FEATURE_EXCLUDED_FROM_MODULE_INTERFACE(DebugDescriptionMacro, true)
 
 EXPERIMENTAL_FEATURE(ReinitializeConsumeInMultiBlockDefer, false)
 
+EXPERIMENTAL_FEATURE(SE427NoInferenceOnExtension, false)
+
 #undef EXPERIMENTAL_FEATURE_EXCLUDED_FROM_MODULE_INTERFACE
 #undef EXPERIMENTAL_FEATURE
 #undef UPCOMING_FEATURE

lib/AST/ASTPrinter.cpp

@@ -973,7 +973,6 @@ class PrintAST : public ASTVisitor<PrintAST> {
     SwapSelfAndDependentMemberType = 8,
     PrintInherited = 16,
     PrintInverseRequirements = 32,
-    IncludeOuterInverses = 64,
   };
 
   /// The default generic signature flags for printing requirements.
@@ -1003,7 +1002,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,
@@ -1684,9 +1684,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.
@@ -1704,16 +1708,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());
@@ -1722,17 +1745,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();
 
@@ -2769,8 +2781,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,
@@ -2779,7 +2792,8 @@ void PrintAST::printDeclGenericRequirements(GenericContext *decl) {
                           if (parentSig)
                             return !parentSig->isRequirementSatisfied(req);
                           return true;
-                        });
+                        },
+                        inverseFilter);
   Printer.printStructurePost(PrintStructureKind::DeclGenericParameterClause);
 }
 
@@ -2913,20 +2927,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 &&
@@ -3051,24 +3051,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

@@ -1894,6 +1894,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/AST/FeatureSet.cpp

@@ -771,6 +771,7 @@ static bool usesFeatureGlobalActorIsolatedTypesUsability(Decl *decl) {
 }
 
 UNINTERESTING_FEATURE(ReinitializeConsumeInMultiBlockDefer)
+UNINTERESTING_FEATURE(SE427NoInferenceOnExtension)
 
 // ----------------------------------------------------------------------------
 // MARK: - FeatureSet

lib/Sema/TypeCheckDeclPrimary.cpp

@@ -3345,7 +3345,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

@@ -785,33 +785,18 @@ 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();
+
+    // FIXME: to workaround a reverse condfail, always infer the requirements if
+    //  the extension is in a swiftinterface file. This is temporary and should
+    //  be removed soon. (rdar://130424971)
+    if (auto *sf = ext->getOutermostParentSourceFile()) {
+      if (sf->Kind == SourceFileKind::Interface
+          && !ctx.LangOpts.hasFeature(Feature::SE427NoInferenceOnExtension))
+        inferInvertibleReqs = true;
     }
 
     collectAdditionalExtensionRequirements(ext->getExtendedType(), extraReqs);