Merge pull request swiftlang#65542 from tshortli/implicit-modify-inherit-spi

tshortli · web-flow · commit 872f45ded62b · 2023-05-02T13:11:00.000-07:00
Sema: Inherit SPI groups when synthesizing modify accessor
diff --git a/lib/Sema/CodeSynthesis.cpp b/lib/Sema/CodeSynthesis.cpp
@@ -1494,3 +1494,15 @@ bool swift::addNonIsolatedToSynthesized(NominalTypeDecl *nominal,
   value->getAttrs().add(new (ctx) NonisolatedAttr(/*isImplicit=*/true));
   return true;
 }
+
+void swift::applyInferredSPIAccessControlAttr(Decl *decl,
+                                              const Decl *inferredFromDecl,
+                                              ASTContext &ctx) {
+  auto spiGroups = inferredFromDecl->getSPIGroups();
+  if (spiGroups.empty())
+    return;
+
+  auto spiAttr =
+      SPIAccessControlAttr::create(ctx, SourceLoc(), SourceRange(), spiGroups);
+  decl->getAttrs().add(spiAttr);
+}
diff --git a/lib/Sema/CodeSynthesis.h b/lib/Sema/CodeSynthesis.h
@@ -89,6 +89,10 @@ bool hasLetStoredPropertyWithInitialValue(NominalTypeDecl *nominal);
 /// if an attribute was added.
 bool addNonIsolatedToSynthesized(NominalTypeDecl *nominal, ValueDecl *value);
 
+/// Adds the `@_spi` groups from \p inferredFromDecl to \p decl.
+void applyInferredSPIAccessControlAttr(Decl *decl, const Decl *inferredFromDecl,
+                                       ASTContext &ctx);
+
 } // end namespace swift
 
 #endif
diff --git a/lib/Sema/ResilienceDiagnostics.cpp b/lib/Sema/ResilienceDiagnostics.cpp
@@ -222,6 +222,18 @@ static bool diagnoseValueDeclRefExportability(SourceLoc loc, const ValueDecl *D,
   if (originKind == DisallowedOriginKind::None)
     return false;
 
+  auto diagName = D->getName();
+  if (auto accessor = dyn_cast<AccessorDecl>(D)) {
+    // Only diagnose accessors if their disallowed origin kind differs from
+    // that of their storage.
+    if (getDisallowedOriginKind(accessor->getStorage(), where) == originKind)
+      return false;
+
+    // For accessors, diagnose with the name of the storage instead of the
+    // implicit '_'.
+    diagName = accessor->getStorage()->getName();
+  }
+
   ASTContext &ctx = where.getDeclContext()->getASTContext();
 
   auto fragileKind = where.getFragileFunctionKind();
@@ -233,7 +245,7 @@ static bool diagnoseValueDeclRefExportability(SourceLoc loc, const ValueDecl *D,
                               diag::decl_from_hidden_module;
     ctx.Diags.diagnose(loc, errorOrWarning,
                        D->getDescriptiveKind(),
-                       D->getName(),
+                       diagName,
                        static_cast<unsigned>(*reason),
                        definingModule->getName(),
                        static_cast<unsigned>(originKind));
@@ -250,7 +262,7 @@ static bool diagnoseValueDeclRefExportability(SourceLoc loc, const ValueDecl *D,
                               diag::inlinable_decl_ref_from_hidden_module;
 
     ctx.Diags.diagnose(loc, errorOrWarning,
-                       D->getDescriptiveKind(), D->getName(),
+                       D->getDescriptiveKind(), diagName,
                        fragileKind.getSelector(), definingModule->getName(),
                        static_cast<unsigned>(originKind));
 
@@ -265,11 +277,6 @@ static bool diagnoseValueDeclRefExportability(SourceLoc loc, const ValueDecl *D,
 bool TypeChecker::diagnoseDeclRefExportability(SourceLoc loc,
                                                const ValueDecl *D,
                                                const ExportContext &where) {
-  // Accessors cannot have exportability that's different than the storage,
-  // so skip them for now.
-  if (isa<AccessorDecl>(D))
-    return false;
-
   if (!where.mustOnlyReferenceExportedDecls())
     return false;
 
diff --git a/lib/Sema/TypeCheckStorage.cpp b/lib/Sema/TypeCheckStorage.cpp
@@ -2324,6 +2324,12 @@ createCoroutineAccessorPrototype(AbstractStorageDecl *storage,
   AvailabilityInference::applyInferredAvailableAttrs(accessor,
                                                      asAvailableAs, ctx);
 
+  // A modify coroutine should have the same SPI visibility as the setter.
+  if (kind == AccessorKind::Modify) {
+    if (FuncDecl *setter = storage->getParsedAccessor(AccessorKind::Set))
+      applyInferredSPIAccessControlAttr(accessor, setter, ctx);
+  }
+
   finishImplicitAccessor(accessor, ctx);
 
   return accessor;
diff --git a/test/SPI/local_spi_decls.swift b/test/SPI/local_spi_decls.swift
@@ -26,11 +26,18 @@ public func useOfSPITypeInvalid() -> SPIClass { fatalError() } // expected-error
 @_spi(S) public func spiUseOfInternalType() -> InternalClass { fatalError() } // expected-error{{function cannot be declared public because its result uses an internal type}}
 @_spi(S) public func spiUseOfPrivateType(_ a: PrivateClass)  { fatalError() } // expected-error{{function cannot be declared public because its parameter uses a private type}}
 
+public var globalArrayWithSPISetter: [Int] {
+  get { fatalError() }
+  @_spi(S) set {}
+}
+
 @inlinable
 func inlinable() -> SPIClass { // expected-error {{class 'SPIClass' cannot be used in an '@inlinable' function because it is SPI}}
   spiFunc() // expected-error {{global function 'spiFunc()' cannot be used in an '@inlinable' function because it is SPI}}
   _ = SPIClass() // expected-error {{class 'SPIClass' cannot be used in an '@inlinable' function because it is SPI}}
   // expected-error@-1 {{initializer 'init()' cannot be used in an '@inlinable' function because it is SPI}}
+  globalArrayWithSPISetter = [] // expected-error {{setter 'globalArrayWithSPISetter' cannot be used in an '@inlinable' function because it is SPI}}
+  globalArrayWithSPISetter.append(0) // expected-error {{setter 'globalArrayWithSPISetter' cannot be used in an '@inlinable' function because it is SPI}}
 }
 
 @_spi(S) public struct SPIStruct {
diff --git a/test/SPI/private_swiftinterface.swift b/test/SPI/private_swiftinterface.swift
@@ -139,6 +139,22 @@ public class SomeClass {
 }
 
 public struct PublicStruct {
+  @_spi(S) public var spiVar: SomeClass
+  // CHECK-PRIVATE: @_spi(S) public var spiVar: {{.*}}.SomeClass
+  // CHECK-PUBLIC-NOT: spiVar
+
+  public var publicVarWithSPISet: SomeClass {
+    get { SomeClass() }
+    @_spi(S) set {}
+  }
+  // CHECK-PRIVATE: public var publicVarWithSPISet: {{.*}}.SomeClass {
+  // CHECK-PRIVATE-NEXT:   get
+  // CHECK-PRIVATE-NEXT:   @_spi(S) set
+  // CHECK-PRIVATE-NEXT: }
+  // CHECK-PUBLIC: public var publicVarWithSPISet: {{.*}}.SomeClass {
+  // CHECK-PUBLIC-NEXT:   get
+  // CHECK-PUBLIC-NEXT: }
+
   @_spi(S) @Wrapper public var spiWrappedSimple: SomeClass
   // CHECK-PRIVATE: @_spi(S) @{{.*}}.Wrapper public var spiWrappedSimple: {{.*}}.SomeClass
   // CHECK-PUBLIC-NOT: spiWrappedSimple
diff --git a/test/api-digester/ignore-spi-by-given-group-name.swift b/test/api-digester/ignore-spi-by-given-group-name.swift
@@ -2,21 +2,39 @@
 
 // RUN: %target-swift-frontend -emit-module -o %t/Foo.swiftmodule -emit-abi-descriptor-path %t/abi-before.json %s -enable-library-evolution -DBASELINE -emit-tbd-path %t/abi-before.tbd
 // RUN: %target-swift-frontend -emit-module -o %t/Foo.swiftmodule -emit-abi-descriptor-path %t/abi-after.json %s -enable-library-evolution -emit-tbd-path %t/abi-after.tbd
-// RUN: %api-digester -diagnose-sdk --input-paths %t/abi-before.json -input-paths %t/abi-after.json -abi -o %t/result.txt
-// RUN: %FileCheck %s -check-prefix CHECK_INCLUDE_SPI < %t/result.txt
+// RUN: %api-digester -diagnose-sdk --input-paths %t/abi-before.json -input-paths %t/abi-after.json -abi -o %t/include-result.txt
+// RUN: %FileCheck %s -check-prefix CHECK_INCLUDE_SPI < %t/include-result.txt
 
-// RUN: %api-digester -diagnose-sdk --input-paths %t/abi-before.json -input-paths %t/abi-after.json -abi -o %t/result.txt -ignore-spi-group secret
-// RUN: %FileCheck -check-prefix CHECK_EXCLUDE_SPI %s < %t/result.txt
+// RUN: %api-digester -diagnose-sdk --input-paths %t/abi-before.json -input-paths %t/abi-after.json -abi -o %t/exclude-result.txt -ignore-spi-group secret
+// RUN: %FileCheck -check-prefix CHECK_EXCLUDE_SPI %s < %t/exclude-result.txt
 
 #if BASELINE
 
+public struct Struct {
+  public var x: Int {
+    get { 0 }
+    @_spi(secret) set {}
+  }
+}
+
 @_spi(secret)
 public func foo() {}
 
 #else
 
+public struct Struct {
+  public var x: Int {
+    get { 0 }
+  }
+}
 
 #endif
 
+// CHECK_INCLUDE_SPI: Accessor Struct.x.Modify() has been removed
+// CHECK_EXCLUDE_SPI-NOT: Struct.x.Modify()
+
+// CHECK_INCLUDE_SPI: Accessor Struct.x.Set() has been removed
+// CHECK_EXCLUDE_SPI-NOT: Struct.x.Set()
+
 // CHECK_INCLUDE_SPI: Func foo() has been removed
 // CHECK_EXCLUDE_SPI-NOT: foo()
