Skip to content

Conversation

@zixu-w
Copy link
Contributor

@zixu-w zixu-w commented Sep 4, 2025

Correctly account for access scopes of declarations visited by APIGen:
Mark declarations with non-public access scopes as private SPIs.

Resolves rdar://159701853

@zixu-w zixu-w requested a review from cyndyishida September 4, 2025 23:39
@zixu-w zixu-w requested a review from rjmccall as a code owner September 4, 2025 23:39
Copy link
Contributor

@cyndyishida cyndyishida left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change generally makes sense to me. A couple questions.

  1. What happens if, or is it possible, for S/L/PF libraries to have public access level APIs? I assume so, but what is a resulting sdkdb info?

  2. Why not record non-exported decls? Is it because that information may not be used or it can't actually ever be represented as SPI?

@zixu-w
Copy link
Contributor Author

zixu-w commented Sep 5, 2025

  1. What happens if, or is it possible, for S/L/PF libraries to have public access level APIs? I assume so, but what is a resulting sdkdb info?

Private frameworks and the access scope keywords in Swift are orthogonal. S/L/PF libraries would definitely have declarations with public access scope. Access scope controls the visibility of declarations outside the Swift module, not the public/private in terms of access level. All APIs and SPIs should have public access scope. SPIs would also have a @_spi or @_spi_available attribute.

It's also possible for private frameworks to have public access level APIs (public not as in access scope, but in terms of public/private. e.g., a declaration without the @_spi attribute). And we would want to treat them as private. But that's a different issue that we should address in a separate change. This change only covers the access scope handling.

  1. Why not record non-exported decls? Is it because that information may not be used or it can't actually ever be represented as SPI?

These decls couldn't ever be used as interfaces provided by the module, including API and SPI. From the documentation of isExposedToClients that I used for the filtering, non-exportable decls means there's no way to access the decl from outside the module (also see https://docs.swift.org/swift-book/documentation/the-swift-programming-language/accesscontrol/).

@cyndyishida
Copy link
Contributor

It's also possible for private frameworks to have public access level APIs (public not as in access scope, but in terms of public/private. e.g., a declaration without the @_spi attribute). And we would want to treat them as private. But that's a different issue that we should address in a separate change. This change only covers the access scope handling.

This is precisely my question. what is the resulting sdkdb entry for this case & does it change with this patch independently of whether theres a better fix for this in a future patch.

These decls couldn't ever be used as interfaces provided by the module, including API and SPI. From the documentation of isExposedToClients that I used for the filtering, non-exportable decls means there's no way to access the decl from outside the module (also see https://docs.swift.org/swift-book/documentation/the-swift-programming-language/accesscontrol/).

I don't know that I fully understand the difference between exposed vs exported. e.g. inlined functions could be not-exported symbols but clients could still use it right?

Correctly account for access scopes of declarations visited by APIGen:
Mark declarations with non-public access scopes as private SPIs.

Resolves rdar://159701853
@zixu-w zixu-w force-pushed the pr-159701853-apigen-access-scope branch from 2ada31b to fea5226 Compare October 2, 2025 23:40
Copy link
Contributor

@cyndyishida cyndyishida left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Should the PR description/ultimate merged commit message be updated though?

@zixu-w
Copy link
Contributor Author

zixu-w commented Oct 3, 2025

@swift-ci please test

1 similar comment
@zixu-w
Copy link
Contributor Author

zixu-w commented Oct 6, 2025

@swift-ci please test

@zixu-w
Copy link
Contributor Author

zixu-w commented Oct 7, 2025

@swift-ci please smoke test macOS

@zixu-w
Copy link
Contributor Author

zixu-w commented Oct 10, 2025

@swift-ci please test Windows

@zixu-w zixu-w merged commit d652100 into swiftlang:main Oct 14, 2025
4 of 5 checks passed
@zixu-w zixu-w deleted the pr-159701853-apigen-access-scope branch October 14, 2025 18:05
dendiz pushed a commit to dendiz/swift that referenced this pull request Oct 21, 2025
Correctly account for access scopes of declarations visited by APIGen:
Mark declarations with non-public access scopes as private SPIs.

Resolves rdar://159701853
speednoisemovement pushed a commit to speednoisemovement/swift that referenced this pull request Oct 29, 2025
Correctly account for access scopes of declarations visited by APIGen:
Mark declarations with non-public access scopes as private SPIs.

Resolves rdar://159701853
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants