Merge pull request #71613 from nate-chandler/bitwise-copyable/no-infer-public

nate-chandler · web-flow · commit 4d8c84270df6 · 2024-02-14T15:21:12.000-08:00
[BitwiseCopyable] Don't ever infer for public.
diff --git a/lib/SIL/IR/TypeLowering.cpp b/lib/SIL/IR/TypeLowering.cpp
@@ -54,6 +54,10 @@ llvm::cl::opt<bool> TypeLoweringForceOpaqueValueLowering(
     llvm::cl::desc("Force TypeLowering to behave as if building with opaque "
                    "values enabled"));
 
+llvm::cl::opt<bool> TypeLoweringDisableVerification(
+    "type-lowering-disable-verification", llvm::cl::init(false),
+    llvm::cl::desc("Disable the asserts-only verification of lowerings"));
+
 namespace {
   /// A CRTP type visitor for deciding whether the metatype for a type
   /// is a singleton type, i.e. whether there can only ever be one
@@ -2945,6 +2949,9 @@ void TypeConverter::verifyLowering(const TypeLowering &lowering,
                                    AbstractionPattern origType,
                                    CanType substType,
                                    TypeExpansionContext forExpansion) {
+  if (TypeLoweringDisableVerification) {
+    return;
+  }
   verifyLexicalLowering(lowering, origType, substType, forExpansion);
   verifyTrivialLowering(lowering, origType, substType, forExpansion);
 }
@@ -3049,7 +3056,7 @@ void TypeConverter::verifyTrivialLowering(const TypeLowering &lowering,
 
   if (lowering.isTrivial() && !conformance) {
     // A trivial type can lack a conformance in a few cases:
-    // (1) containing or being a resilient type
+    // (1) containing or being a public, non-frozen type
     // (2) containing or being a generic type which doesn't conform
     //     unconditionally but in this particular instantiation is trivial
     // (3) being a special type that's not worth forming a conformance for
@@ -3064,6 +3071,7 @@ void TypeConverter::verifyTrivialLowering(const TypeLowering &lowering,
     //               unowned(unsafe) var o: AnyObject
     //             }
     // (5) being defined in a different module
+    // (6) being defined in a module built from interface
     bool hasNoNonconformingNode = visitAggregateLeaves(
         origType, substType, forExpansion,
         /*isLeafAggregate=*/
@@ -3081,12 +3089,22 @@ void TypeConverter::verifyTrivialLowering(const TypeLowering &lowering,
             return true;
           }
 
-          // Resilient trivial types may not conform (case (1)).
-          if (nominal->isResilient())
+          // Public, non-frozen trivial types may not conform (case (1)).
+          if (nominal
+                  ->getFormalAccessScope(/*useDC=*/nullptr,
+                                         /*treatUsableFromInlineAsPublic=*/true)
+                  .isPublic())
             return true;
 
+          auto *module = nominal->getModuleContext();
+
+          // Types in modules built from interfaces may not conform (case (6)).
+          if (module && module->isBuiltFromInterface()) {
+            return false;
+          }
+
           // Trivial types from other modules may not conform (case (5)).
-          return nominal->getModuleContext() != &M;
+          return module != &M;
         },
         /*visit=*/
         [&](auto ty, auto origTy, auto *field, auto index) -> bool {
@@ -3141,12 +3159,22 @@ void TypeConverter::verifyTrivialLowering(const TypeLowering &lowering,
             return false;
           }
 
-          // Resilient trivial types may not conform (case (1)).
-          if (nominal->isResilient())
+          // Public, non-frozen trivial types may not conform (case (1)).
+          if (nominal
+                  ->getFormalAccessScope(/*useDC=*/nullptr,
+                                         /*treatUsableFromInlineAsPublic=*/true)
+                  .isPublic())
+            return false;
+
+          auto *module = nominal->getModuleContext();
+
+          // Types in modules built from interfaces may not conform (case (6)).
+          if (module && module->isBuiltFromInterface()) {
             return false;
+          }
 
           // Trivial types from other modules may not conform (case (5)).
-          return nominal->getModuleContext() == &M;
+          return module == &M;
         });
     if (hasNoNonconformingNode) {
       llvm::errs() << "Trivial type without a BitwiseCopyable conformance!?:\n"
@@ -3159,10 +3187,19 @@ void TypeConverter::verifyTrivialLowering(const TypeLowering &lowering,
   if (!lowering.isTrivial() && conformance) {
     // A non-trivial type can have a conformance in one case:
     // (1) contains a conforming archetype
+    // (2) is resilient with minimal expansion
     bool hasNoConformingArchetypeNode = visitAggregateLeaves(
         origType, substType, forExpansion,
         /*isLeaf=*/
         [&](auto ty, auto origTy, auto *field, auto index) -> bool {
+          // A resilient type that's with minimal expansion may be non-trivial
+          // but still conform (case (2)).
+          auto *nominal = ty.getAnyNominal();
+          if (nominal && nominal->isResilient() &&
+              forExpansion.getResilienceExpansion() ==
+                  ResilienceExpansion::Minimal) {
+            return true;
+          }
           // Walk into every aggregate.
           return false;
         },
@@ -3173,7 +3210,16 @@ void TypeConverter::verifyTrivialLowering(const TypeLowering &lowering,
                  "leaf of non-trivial BitwiseCopyable type that doesn't "
                  "conform to BitwiseCopyable!?");
 
-          // An archetype may conform but be non-trivial (case (2)).
+          // A resilient type that's with minimal expansion may be non-trivial
+          // but still conform (case (2)).
+          auto *nominal = ty.getAnyNominal();
+          if (nominal && nominal->isResilient() &&
+              forExpansion.getResilienceExpansion() ==
+                  ResilienceExpansion::Minimal) {
+            return false;
+          }
+
+          // An archetype may conform but be non-trivial (case (1)).
           if (origTy.isTypeParameter())
             return false;
 
diff --git a/lib/Sema/TypeCheckBitwise.cpp b/lib/Sema/TypeCheckBitwise.cpp
@@ -57,8 +57,7 @@ getImplicitCheckForNominal(NominalTypeDecl *nominal) {
   if (!nominal
            ->getFormalAccessScope(
                /*useDC=*/nullptr, /*treatUsableFromInlineAsPublic=*/true)
-           .isPublic() ||
-      !nominal->isResilient())
+           .isPublic())
     return {BitwiseCopyableCheck::Implicit};
 
   if (nominal->hasClangNode() ||
diff --git a/test/IRGen/bitwise_copyable_resilient.swift b/test/IRGen/bitwise_copyable_resilient.swift
@@ -0,0 +1,55 @@
+// RUN: %empty-directory(%t)
+// RUN: split-file %s %t
+
+// RUN: %target-swift-frontend                           \
+// RUN:     %t/Library.swift                             \
+// RUN:     -emit-module                                 \
+// RUN:     -enable-library-evolution                    \
+// RUN:     -enable-experimental-feature BitwiseCopyable \
+// RUN:     -module-name Library                         \
+// RUN:     -emit-module-path %t/Library.swiftmodule
+
+// RUN: %target-swift-frontend                           \
+// RUN:     %t/Downstream.swift                          \
+// RUN:     -typecheck -verify                           \
+// RUN:     -debug-diagnostic-names                      \
+// RUN:     -enable-experimental-feature BitwiseCopyable \
+// RUN:     -I %t
+
+//--- Library.swift
+public enum Oopsional<T> {
+case someone(T)
+case nobody
+}
+
+@frozen public enum Woopsional<T> {
+case somebody(T)
+case noone
+}
+
+public struct Integer {
+  var value: Int
+}
+
+//--- Downstream.swift
+import Library
+
+func take<T: _BitwiseCopyable>(_ t: T) {}
+
+struct S_Explicit_With_Oopsional<T> : _BitwiseCopyable {
+  var o: Oopsional<T> // expected-error{{non_bitwise_copyable_type_member}}
+}
+
+func passOopsional<T>(_ t: Oopsional<T>) { take(t) } // expected-error   {{type_does_not_conform_decl_owner}}
+                                                     // expected-note@-7 {{where_requirement_failure_one_subst}}
+
+
+struct S_Explicit_With_Woopsional<T> : _BitwiseCopyable {
+  var o: Woopsional<T> // expected-error{{non_bitwise_copyable_type_member}}
+}
+
+func passWoopsional<T>(_ t: Woopsional<T>) { take(t) } // expected-error    {{type_does_not_conform_decl_owner}}
+                                                       // expected-note@-15 {{where_requirement_failure_one_subst}}
+
+extension Integer : @retroactive _BitwiseCopyable {} // expected-error {{bitwise_copyable_outside_module}}
+
diff --git a/test/SILGen/bitwise_copyable.swift b/test/SILGen/bitwise_copyable.swift
@@ -40,7 +40,7 @@ enum Context<T> {
 
 func doit() -> Context<Int>.Here { .init(t: 0) }
 
-public enum E {
+public enum E : _BitwiseCopyable {
   case a
 }
 
diff --git a/test/Sema/bitwise_copyable.swift b/test/Sema/bitwise_copyable.swift
@@ -62,7 +62,8 @@ struct S_Explicit_With_Function_C : _BitwiseCopyable {
 public struct S_Public {}
 
 struct S_Explicit_With_S_Public : _BitwiseCopyable {
-  var s: S_Public
+  var s: S_Public // expected-error   {{non_bitwise_copyable_type_member}}
+                  // expected-note@-4 {{add_nominal_bitwise_copyable_conformance}}
 }
 
 struct S_Explicit_With_Generic<T> : _BitwiseCopyable {
diff --git a/test/Sema/bitwise_copyable_2.swift b/test/Sema/bitwise_copyable_2.swift
@@ -25,3 +25,13 @@ indirect enum E_Explicit_Indirect : _BitwiseCopyable { // expected-error {{non_b
 enum E_Explicit_Indirect_Case : _BitwiseCopyable { // expected-error {{non_bitwise_copyable_type_indirect_enum_element}}
   indirect case s(S) // expected-note {{note_non_bitwise_copyable_type_indirect_enum_element}}
 }
+
+func take<T : _BitwiseCopyable>(_ t: T) {}
+
+// public (effectively) => !conforms
+@usableFromInline internal struct InternalUsableStruct {
+  public var int: Int
+}
+
+func passInternalUsableStruct(_ s: InternalUsableStruct) { take(s) } // expected-error{{type_does_not_conform_decl_owner}}
+                                                                     // expected-note@-8{{where_requirement_failure_one_subst}}
diff --git a/test/Sema/bitwise_copyable_nonresilient.swift b/test/Sema/bitwise_copyable_nonresilient.swift
@@ -32,15 +32,17 @@ import Library
 func take<T: _BitwiseCopyable>(_ t: T) {}
 
 struct S_Explicit_With_Oopsional<T> : _BitwiseCopyable {
-  var o: Oopsional<T>
+  var o: Oopsional<T> // expected-error{{non_bitwise_copyable_type_member}}
 }
 
-func passOopsional<T>(_ t: Oopsional<T>) { take(t) }
+func passOopsional<T>(_ t: Oopsional<T>) { take(t) } // expected-error{{type_does_not_conform_decl_owner}}
+                                                     // expected-note@-7{{where_requirement_failure_one_subst}}
 
 
 struct S_Explicit_With_Woopsional<T> : _BitwiseCopyable {
-  var o: Woopsional<T>
+  var o: Woopsional<T> // expected-error{{non_bitwise_copyable_type_member}}
 }
 
-func passWoopsional<T>(_ t: Woopsional<T>) { take(t) }
+func passWoopsional<T>(_ t: Woopsional<T>) { take(t) } // expected-error{{type_does_not_conform_decl_owner}}
+                                                       // expected-note@-15{{where_requirement_failure_one_subst}}
 
diff --git a/test/Sema/bitwise_copyable_package_resilience.swift b/test/Sema/bitwise_copyable_package_resilience.swift
@@ -0,0 +1,42 @@
+// RUN: %empty-directory(%t)
+// RUN: split-file %s %t
+
+// RUN: %target-swift-frontend                           \
+// RUN:     %t/Library.swift                             \
+// RUN:     -emit-module                                 \
+// RUN:     -package-name Package                        \
+// RUN:     -enable-library-evolution                    \
+// RUN:     -enable-experimental-feature BitwiseCopyable \
+// RUN:     -module-name Library                         \
+// RUN:     -emit-module-path %t/Library.swiftmodule     \
+// RUN:     -emit-module-interface-path %t/Library.swiftinterface
+
+// RUN: %target-swift-frontend                           \
+// RUN:     %t/Downstream.swift                          \
+// RUN:     -typecheck -verify                           \
+// RUN:     -package-name Package                        \
+// RUN:     -debug-diagnostic-names                      \
+// RUN:     -enable-experimental-feature BitwiseCopyable \
+// RUN:     -I %t
+
+//--- Library.swift
+
+// !public => conforms
+package struct PackageStruct {
+  package var int: Int
+}
+
+// Public => !conforms
+public struct PublicStruct {
+  public var int: Int
+}
+
+//--- Downstream.swift
+import Library
+
+func take<T : _BitwiseCopyable>(_ t: T) {}
+
+func passPackageStruct(_ s: PackageStruct) { take(s) }
+
+func passPublicStruct(_ s: PublicStruct) { take(s) } // expected-error{{type_does_not_conform_decl_owner}}
+                                                     // expected-note@-5{{where_requirement_failure_one_subst}}

Original file line number	Diff line number	Diff line change
`@@ -40,7 +40,7 @@ enum Context<T> {`
`40`	`40`
`41`	`41`	`func doit() -> Context<Int>.Here { .init(t: 0) }`
`42`	`42`
`43`		`-public enum E {`
	`43`	`+public enum E : _BitwiseCopyable {`
`44`	`44`	`case a`
`45`	`45`	`}`
`46`	`46`
Original file line number	Diff line number	Diff line change
`@@ -62,7 +62,8 @@ struct S_Explicit_With_Function_C : _BitwiseCopyable {`
`62`	`62`	`public struct S_Public {}`
`63`	`63`
`64`	`64`	`struct S_Explicit_With_S_Public : _BitwiseCopyable {`
`65`		`- var s: S_Public`
	`65`	`+ var s: S_Public // expected-error {{non_bitwise_copyable_type_member}}`
	`66`	`+ // expected-note@-4 {{add_nominal_bitwise_copyable_conformance}}`
`66`	`67`	`}`
`67`	`68`
`68`	`69`	`struct S_Explicit_With_Generic<T> : _BitwiseCopyable {`