Merge pull request #73492 from kavon/ncgeneric-avoid-spam

NCGenerics: avoid feature-guarding in some cases

Author: kavon

lib/AST/FeatureSet.cpp

@@ -506,6 +506,21 @@ UNINTERESTING_FEATURE(Embedded)
 UNINTERESTING_FEATURE(Volatile)
 UNINTERESTING_FEATURE(SuppressedAssociatedTypes)
 
+static bool disallowFeatureSuppression(StringRef featureName, Decl *decl);
+
+static bool allBoundTypesAreCopyable(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;
@@ -523,15 +538,29 @@ static bool usesFeatureNoncopyableGenerics(Decl *decl) {
 
     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;
-          })) {
+      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, then we're
+        // only referring to the un-suppressed version if any of the bound types
+        // are noncopyable. (rdar://127389991)
+        if (!disallowFeatureSuppression("NoncopyableGenerics", nominalDecl)
+            && allBoundTypesAreCopyable(type, context)) {
+          return false;
+        }
+
+        return true;
+      });
+      if (usesFeature)
         return true;
-      }
     }
   }
 

test/ModuleInterface/Inputs/NoncopyableGenerics_Misc.swift

@@ -112,3 +112,18 @@ public func borrowsNoncopyable<T: ~Copyable>(_ t: borrowing T) {}
 
 @_disallowFeatureSuppression(NoncopyableGenerics)
 public func suppressesNoncopyableGenerics<T: ~Copyable>(_ t: borrowing T) {}
+
+// coverage for rdar://127389991
+@_disallowFeatureSuppression(NoncopyableGenerics)
+public struct LoudlyNC<T: ~Copyable> {}
+public func _indexHumongousDonuts<TTT, T>(_ aggregate: UnsafePointer<TTT>, _ index: Int) -> T {
+    return UnsafeRawPointer(aggregate).load(
+    fromByteOffset: index * MemoryLayout<T>.stride, as: T.self)
+}
+public func referToLoud(_ t: LoudlyNC<String>) {}
+@_disallowFeatureSuppression(NoncopyableGenerics) public func referToLoudProperGuarding(_ t: LoudlyNC<String>) {}
+public struct NoCopyPls: ~Copyable {}
+public func substCopyable(_ t: String?) {}
+public func substGenericCopyable<T>(_ t: T?) {}
+public func substNC(_ t: borrowing NoCopyPls?) {}
+public func substGenericNC<T: ~Copyable>(_ t: borrowing T?) {}

test/ModuleInterface/features.swift

@@ -90,13 +90,9 @@ public class OldSchool2: MP {
 // CHECK: public struct UsesRP {
 public struct UsesRP {
   // CHECK:     #if compiler(>=5.3) && $RethrowsProtocol
-  // CHECK-NEXT:  #if $NoncopyableGenerics
   // CHECK-NEXT:  public var value: (any FeatureTest.RP)? {
   // CHECK-NOT: #if compiler(>=5.3) && $RethrowsProtocol
   // CHECK:         get
-  // CHECK:     #else
-  // CHECK-NEXT: public var value: (any FeatureTest.RP)? {
-  // CHECK-NEXT:  get
   public var value: RP? {
     nil
   }

test/ModuleInterface/noncopyable_generics.swift

@@ -160,6 +160,39 @@ import NoncopyableGenerics_Misc
 // CHECK-MISC-NEXT: public func suppressesNoncopyableGenerics<T>(_ t: borrowing T) where T : ~Copyable
 // CHECK-MISC-NEXT: #endif
 
+// CHECK-MISC:      #if compiler(>=5.3) && $NoncopyableGenerics
+// CHECK-MISC-NEXT: public struct LoudlyNC<T> where T : ~Copyable {
+// CHECK-MISC-NEXT: }
+// CHECK-MISC-NEXT: #endif
+// CHECK-MISC-NEXT: public func _indexHumongousDonuts<TTT, T>(_ aggregate: Swift.UnsafePointer<TTT>, _ index: Swift.Int) -> T
+// CHECK-MISC-NEXT: #if compiler(>=5.3) && $NoncopyableGenerics
+// CHECK-MISC-NEXT: public func referToLoud(_ t: {{.*}}.LoudlyNC<Swift.String>)
+// CHECK-MISC-NEXT: #else
+// CHECK-MISC-NEXT: public func referToLoud(_ t: {{.*}}.LoudlyNC<Swift.String>)
+// CHECK-MISC-NEXT: #endif
+// CHECK-MISC-NEXT: #if compiler(>=5.3) && $NoncopyableGenerics
+// CHECK-MISC-NEXT: public func referToLoudProperGuarding(_ t: {{.*}}.LoudlyNC<Swift.String>)
+// CHECK-MISC-NEXT: #endif
+// CHECK-MISC-NEXT: public struct NoCopyPls : ~Swift.Copyable {
+// CHECK-MISC-NEXT: }
+// CHECK-MISC-NEXT: public func substCopyable(_ t: Swift.String?)
+// CHECK-MISC-NEXT: public func substGenericCopyable<T>(_ t: T?)
+
+// NOTE: we really shouldn't be emitting the else branch for the two funcs
+// below, since the suppressed version isn't valid. We don't have a good way of
+// fixing that right now, either.
+
+// CHECK-MISC-NEXT: #if compiler(>=5.3) && $NoncopyableGenerics
+// CHECK-MISC-NEXT: public func substNC(_ t: borrowing {{.*}}.NoCopyPls?)
+// CHECK-MISC-NEXT: #else
+// CHECK-MISC-NEXT: public func substNC(_ t: borrowing {{.*}}.NoCopyPls?)
+// CHECK-MISC-NEXT: #endif
+// CHECK-MISC-NEXT: #if compiler(>=5.3) && $NoncopyableGenerics
+// CHECK-MISC-NEXT: public func substGenericNC<T>(_ t: borrowing T?) where T : ~Copyable
+// CHECK-MISC-NEXT: #else
+// CHECK-MISC-NEXT: public func substGenericNC<T>(_ t: borrowing T?)
+// CHECK-MISC-NEXT: #endif
+
 
 import Swiftskell