Skip to content

Sema: Inherit SPI groups when synthesizing modify accessor #65542

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 2, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions lib/Sema/CodeSynthesis.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
4 changes: 4 additions & 0 deletions lib/Sema/CodeSynthesis.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
21 changes: 14 additions & 7 deletions lib/Sema/ResilienceDiagnostics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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));
Expand All @@ -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));

Expand All @@ -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;

Expand Down
6 changes: 6 additions & 0 deletions lib/Sema/TypeCheckStorage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
7 changes: 7 additions & 0 deletions test/SPI/local_spi_decls.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
16 changes: 16 additions & 0 deletions test/SPI/private_swiftinterface.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
26 changes: 22 additions & 4 deletions test/api-digester/ignore-spi-by-given-group-name.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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()