Merge pull request #75090 from tshortli/no-more-ncg-condfails

AST: Remove `NoncopyableGenerics` feature suppression

Author: tshortli

include/swift/AST/PrintOptions.h

@@ -385,12 +385,6 @@ struct PrintOptions {
   /// Suppress 'sending' on arguments and results.
   bool SuppressSendingArgsAndResults = false;
 
-  /// Suppress Noncopyable generics.
-  bool SuppressNoncopyableGenerics = false;
-
-  /// Suppress printing of `borrowing` and `consuming`.
-  bool SuppressNoncopyableOwnershipModifiers = false;
-
   /// Suppress printing of '~Proto' for suppressible, non-invertible protocols.
   bool SuppressConformanceSuppression = false;
 

include/swift/Basic/Features.def

@@ -193,7 +193,8 @@ SUPPRESSIBLE_LANGUAGE_FEATURE(AssociatedTypeImplements, 0, "@_implements on asso
 LANGUAGE_FEATURE(BuiltinAddressOfRawLayout, 0, "Builtin.addressOfRawLayout")
 LANGUAGE_FEATURE(MoveOnlyPartialConsumption, 429, "Partial consumption of noncopyable values")
 LANGUAGE_FEATURE(BitwiseCopyable, 426, "BitwiseCopyable protocol")
-SUPPRESSIBLE_LANGUAGE_FEATURE(NoncopyableGenerics, 427, "Noncopyable generics")
+BASELINE_LANGUAGE_FEATURE(NoncopyableGenerics, 427, "Noncopyable generics")
+BASELINE_LANGUAGE_FEATURE(NoncopyableGenerics2, 427, "Noncopyable generics alias")
 SUPPRESSIBLE_LANGUAGE_FEATURE(ConformanceSuppression, 426, "Suppressible inferred conformances")
 SUPPRESSIBLE_LANGUAGE_FEATURE(BitwiseCopyable2, 426, "BitwiseCopyable feature")
 LANGUAGE_FEATURE(BodyMacros, 415, "Function body macros")
@@ -348,9 +349,6 @@ EXPERIMENTAL_FEATURE(Embedded, true)
 /// Enables importing the Volatile module
 EXPERIMENTAL_FEATURE(Volatile, true)
 
-// Alias for NoncopyableGenerics
-EXPERIMENTAL_FEATURE(NoncopyableGenerics2, true)
-
 // Enables ~Copyable and ~Escapable annotations on associatedtype declarations.
 EXPERIMENTAL_FEATURE(SuppressedAssociatedTypes, true)
 

lib/AST/ASTPrinter.cpp

@@ -983,12 +983,7 @@ class PrintAST : public ASTVisitor<PrintAST> {
   /// The default generic signature flags for printing requirements.
   static unsigned 
   defaultGenericRequirementFlags(const PrintOptions &options) {
-    unsigned flags = PrintRequirements;
-
-    if (!options.SuppressNoncopyableGenerics)
-      flags |= PrintInverseRequirements;
-
-    return flags;
+    return PrintRequirements | PrintInverseRequirements;
   }
 
   void printInheritedFromRequirementSignature(ProtocolDecl *proto,
@@ -1148,18 +1143,8 @@ class PrintAST : public ASTVisitor<PrintAST> {
     Printer.callPrintDeclPre(D, Options.BracketOptions);
 
     if (Options.PrintCompatibilityFeatureChecks) {
-      printWithCompatibilityFeatureChecks(Printer, Options, D, [&] {
-        // If we are in a scope where non-copyable generics are being suppressed
-        // and we are also printing a decl that has @_preInverseGenerics, make
-        // sure we also suppress printing ownership modifiers that were added
-        // to satisfy the requirements of non-copyability.
-        llvm::SaveAndRestore<bool> scope(
-            Options.SuppressNoncopyableOwnershipModifiers,
-            Options.SuppressNoncopyableGenerics &&
-                D->getAttrs().hasAttribute<PreInverseGenericsAttr>());
-
-        ASTVisitor::visit(D);
-      });
+      printWithCompatibilityFeatureChecks(Printer, Options, D,
+                                          [&] { ASTVisitor::visit(D); });
     } else {
       ASTVisitor::visit(D);
     }
@@ -1642,9 +1627,8 @@ void PrintAST::printInheritedFromRequirementSignature(ProtocolDecl *proto,
   // The invertible protocols themselves do not need to state inverses in their
   // inheritance clause, because they do not gain any default requirements.
   // HACK: also exclude Sendable from getting inverses printed.
-  if (!proto->getInvertibleProtocolKind()
-      && !proto->isSpecificProtocol(KnownProtocolKind::Sendable) &&
-      !Options.SuppressNoncopyableGenerics)
+  if (!proto->getInvertibleProtocolKind() &&
+      !proto->isSpecificProtocol(KnownProtocolKind::Sendable))
     flags |= PrintInverseRequirements;
 
   printRequirementSignature(
@@ -1701,18 +1685,6 @@ void PrintAST::printGenericSignature(GenericSignature genericSig,
                         AllInverses());
 }
 
-// Erase any requirements involving invertible protocols.
-static void eraseInvertibleProtocolConformances(
-    SmallVectorImpl<Requirement> &requirements) {
-  llvm::erase_if(requirements, [&](Requirement req) {
-      if (req.getKind() != RequirementKind::Conformance)
-        return false;
-
-      return req.getProtocolDecl()
-          ->getInvertibleProtocolKind().has_value();
-    });
-}
-
 InversesAtDepth::InversesAtDepth(GenericContext *level) {
   includedDepth = std::nullopt;
   // Does this generic context have its own generic parameters?
@@ -1745,9 +1717,6 @@ void PrintAST::printGenericSignature(
   } else {
     requirements.append(genericSig.getRequirements().begin(),
                         genericSig.getRequirements().end());
-
-    if (Options.SuppressNoncopyableGenerics)
-      eraseInvertibleProtocolConformances(requirements);
   }
 
   if (flags & InnermostOnly) {
@@ -2026,9 +1995,6 @@ void PrintAST::printRequirementSignature(ProtocolDecl *owner,
   } else {
     requirements.append(sig.getRequirements().begin(),
                         sig.getRequirements().end());
-
-    if (Options.SuppressNoncopyableGenerics)
-      eraseInvertibleProtocolConformances(requirements);
   }
 
   if (attachingTo) {
@@ -3172,17 +3138,6 @@ static void suppressingFeatureAssociatedTypeImplements(PrintOptions &options,
   options.ExcludeAttrList.resize(originalExcludeAttrCount);
 }
 
-static void suppressingFeatureNoncopyableGenerics(
-    PrintOptions &options,
-    llvm::function_ref<void()> action) {
-  unsigned originalExcludeAttrCount = options.ExcludeAttrList.size();
-  options.ExcludeAttrList.push_back(DeclAttrKind::PreInverseGenerics);
-  llvm::SaveAndRestore<bool> scope(
-      options.SuppressNoncopyableGenerics, true);
-  action();
-  options.ExcludeAttrList.resize(originalExcludeAttrCount);
-}
-
 static void
 suppressingFeatureConformanceSuppression(PrintOptions &options,
                                          llvm::function_ref<void()> action) {
@@ -3783,14 +3738,10 @@ static void printParameterFlags(ASTPrinter &printer,
     printer.printKeyword("inout", options, " ");
     break;
   case ParamSpecifier::Borrowing:
-    if (!options.SuppressNoncopyableOwnershipModifiers) {
-      printer.printKeyword("borrowing", options, " ");
-    }
+    printer.printKeyword("borrowing", options, " ");
     break;
   case ParamSpecifier::Consuming:
-    if (!options.SuppressNoncopyableOwnershipModifiers) {
-      printer.printKeyword("consuming", options, " ");
-    }
+    printer.printKeyword("consuming", options, " ");
     break;
   case ParamSpecifier::LegacyShared:
     printer.printKeyword("__shared", options, " ");
@@ -7481,7 +7432,7 @@ void GenericSignature::print(ASTPrinter &Printer,
   }
 
   auto flags = PrintAST::PrintParams | PrintAST::PrintRequirements;
-  if (Opts.PrintInverseRequirements && !Opts.SuppressNoncopyableGenerics)
+  if (Opts.PrintInverseRequirements)
     flags |= PrintAST::PrintInverseRequirements;
   PrintAST(Printer, Opts).printGenericSignature(*this, flags);
 }
@@ -7497,7 +7448,7 @@ void RequirementSignature::print(ProtocolDecl *owner,
                                  ASTPrinter &Printer,
                                  const PrintOptions &Opts) const {
   auto flags = PrintAST::PrintParams | PrintAST::PrintRequirements;
-  if (Opts.PrintInverseRequirements && !Opts.SuppressNoncopyableGenerics)
+  if (Opts.PrintInverseRequirements)
     flags |= PrintAST::PrintInverseRequirements;
   PrintAST(Printer, Opts).printRequirementSignature(owner, *this, flags, nullptr);
 }
@@ -7898,10 +7849,6 @@ swift::getInheritedForPrinting(
 
       // Preserve any inverses that appeared in the unprintable type.
       if (foundUnprintable) {
-        if (printableInverses.contains(InvertibleProtocolKind::Copyable)
-            && options.SuppressNoncopyableGenerics)
-          printableInverses.remove(InvertibleProtocolKind::Copyable);
-
         if (!printableInverses.empty()) {
           auto inversesTy = ProtocolCompositionType::get(decl->getASTContext(),
                                                          /*members=*/{},
@@ -7911,26 +7858,6 @@ swift::getInheritedForPrinting(
         }
         continue;
       }
-
-      // Suppress Copyable and ~Copyable.
-      if (options.SuppressNoncopyableGenerics) {
-        if (auto pct = ty->getAs<ProtocolCompositionType>()) {
-          auto inverses = pct->getInverses();
-          if (inverses.contains(InvertibleProtocolKind::Copyable)) {
-            inverses.remove(InvertibleProtocolKind::Copyable);
-            ty = ProtocolCompositionType::get(decl->getASTContext(),
-                                              pct->getMembers(),
-                                              inverses,
-                                              pct->hasExplicitAnyObject());
-            if (ty->isAny())
-              continue;
-          }
-        }
-
-        if (auto protoTy = ty->getAs<ProtocolType>())
-          if (protoTy->getDecl()->isSpecificProtocol(KnownProtocolKind::Copyable))
-            continue;
-      }
     }
     if (options.SuppressConformanceSuppression &&
         inherited.getEntry(i).isSuppressed()) {

lib/AST/FeatureSet.cpp

@@ -504,94 +504,6 @@ UNINTERESTING_FEATURE(SuppressedAssociatedTypes)
 
 static bool disallowFeatureSuppression(StringRef featureName, Decl *decl);
 
-static bool allSubstTypesAreCopyable(Type type, DeclContext *context) {
-  assert(type->getAnyNominal());
-  auto bgt = type->getAs<BoundGenericType>();
-  if (!bgt)
-    return false;  // nothing is bound.
-
-  for (auto argInterfaceTy : bgt->getGenericArgs())
-    if (context->mapTypeIntoContext(argInterfaceTy)->isNoncopyable())
-      return false;
-
-  return true;
-}
-
-static bool usesFeatureNoncopyableGenerics(Decl *decl) {
-  if (decl->getAttrs().hasAttribute<PreInverseGenericsAttr>())
-    return true;
-
-  if (auto *valueDecl = dyn_cast<ValueDecl>(decl)) {
-    if (auto proto = dyn_cast<ProtocolDecl>(decl)) {
-      auto reqSig = proto->getRequirementSignature();
-
-      SmallVector<Requirement, 2> reqs;
-      SmallVector<InverseRequirement, 2> inverses;
-      reqSig.getRequirementsWithInverses(proto, reqs, inverses);
-      if (!inverses.empty())
-        return true;
-    }
-
-    if (isa<AbstractFunctionDecl>(valueDecl) ||
-        isa<AbstractStorageDecl>(valueDecl)) {
-      auto *context = decl->getInnermostDeclContext();
-      auto usesFeature = valueDecl->getInterfaceType().findIf(
-          [&](Type type) -> bool {
-        auto *nominalDecl = type->getAnyNominal();
-        if (!nominalDecl || !isa<StructDecl, EnumDecl, ClassDecl>(nominalDecl))
-          return false;
-
-        if (!usesFeatureNoncopyableGenerics(nominalDecl))
-          return false;
-
-        // If we only _refer_ to a TypeDecl that uses NoncopyableGenerics,
-        // and a suppressed version of that decl is in the interface, and
-        // if we only substitute Copyable types for the generic parameters,
-        // then we can say this decl is not "using" the feature such that
-        // a feature guard is required. In other words, this reference to the
-        // type will always be valid, regardless of whether the feature is
-        // enabled or not. (rdar://127389991)
-        if (!disallowFeatureSuppression("NoncopyableGenerics", nominalDecl)
-            && allSubstTypesAreCopyable(type, context)) {
-          return false;
-        }
-
-        return true;
-      });
-      if (usesFeature)
-        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)) {
-
-    // We have baked-in support for Sendable not needing to state inverses
-    // in its inheritance clause within interface files. So, it technically is
-    // not using the feature with respect to the concerns of an interface file.
-    if (proto->isSpecificProtocol(KnownProtocolKind::Sendable))
-      return false;
-
-    proto->getRequirementSignature().getRequirementsWithInverses(
-        proto, reqs, inverseReqs);
-  } else if (auto *genCtx = decl->getAsGenericContext()) {
-    if (auto genericSig = genCtx->getGenericSignature())
-      genericSig->getRequirementsWithInverses(reqs, inverseReqs);
-  }
-
-  return !inverseReqs.empty();
-}
-
-UNINTERESTING_FEATURE(NoncopyableGenerics2)
-
 static bool usesFeatureStructLetDestructuring(Decl *decl) {
   auto sd = dyn_cast<StructDecl>(decl);
   if (!sd)

stdlib/public/core/Bitset.swift

@@ -316,10 +316,8 @@ extension _UnsafeBitset.Word {
 // iterator, not the original word.
 extension _UnsafeBitset.Word: Sequence, IteratorProtocol {
 
-#if $NoncopyableGenerics
   @usableFromInline
   typealias Element = Int
-#endif
 
   @inlinable
   internal var count: Int {

stdlib/public/core/ExistentialCollection.swift

@@ -1270,10 +1270,7 @@ internal struct _ClosureBasedSequence<Iterator: IteratorProtocol> {
 extension _ClosureBasedSequence: Sendable {}
 
 extension _ClosureBasedSequence: Sequence {
-
-#if $NoncopyableGenerics
   public typealias Element = Iterator.Element
-#endif
 
   @inlinable
   internal func makeIterator() -> Iterator {

stdlib/public/core/Misc.swift

@@ -231,7 +231,7 @@ func _rethrowsViaClosure(_ fn: () throws -> ()) rethrows {
 @_marker public protocol Escapable {}
 
 #if $BitwiseCopyable2
-#if $NoncopyableGenerics && $NonescapableTypes
+#if $NonescapableTypes
 @_marker public protocol BitwiseCopyable: ~Escapable { }
 #else
 @_marker public protocol BitwiseCopyable { }
@@ -240,7 +240,7 @@ func _rethrowsViaClosure(_ fn: () throws -> ()) rethrows {
 @available(*, deprecated, message: "Use BitwiseCopyable")
 public typealias _BitwiseCopyable = BitwiseCopyable
 #else
-#if $NoncopyableGenerics && $NonescapableTypes
+#if $NonescapableTypes
 @_marker public protocol _BitwiseCopyable: ~Escapable { }
 #else
 @_marker public protocol _BitwiseCopyable { }

stdlib/public/core/Optional.swift

@@ -230,16 +230,12 @@ extension Optional where Wrapped: ~Copyable {
   public borrowing func _borrowingMap<U: ~Copyable, E: Error>(
     _ transform: (borrowing Wrapped) throws(E) -> U
   ) throws(E) -> U? {
-    #if compiler(>=6.0) && $NoncopyableGenerics
     switch self {
     case .some(let y):
       return .some(try transform(y))
     case .none:
       return .none
     }
-    #else
-    fatalError("unsupported compiler")
-    #endif
   }
 }
 
@@ -308,16 +304,12 @@ extension Optional where Wrapped: ~Copyable {
   public func _borrowingFlatMap<U: ~Copyable, E: Error>(
     _ transform: (borrowing Wrapped) throws(E) -> U?
   ) throws(E) -> U? {
-    #if compiler(>=6.0) && $NoncopyableGenerics
     switch self {
     case .some(let y):
       return try transform(y)
     case .none:
       return .none
     }
-    #else
-    fatalError("Unsupported compiler")
-    #endif
   }
 }
 

stdlib/public/core/Sendable.swift

@@ -132,13 +132,12 @@
 /// ### Sendable Metatypes
 ///
 /// Metatypes such as `Int.Type` implicitly conform to the `Sendable` protocol.
-#if $NoncopyableGenerics && $NonescapableTypes
+#if $NonescapableTypes
 @_marker public protocol Sendable: ~Copyable, ~Escapable { }
-#elseif $NoncopyableGenerics
-@_marker public protocol Sendable: ~Copyable { }
 #else
-@_marker public protocol Sendable { }
+@_marker public protocol Sendable: ~Copyable { }
 #endif
+
 ///
 /// A type whose values can safely be passed across concurrency domains by copying,
 /// but which disables some safety checking at the conformance site.