Skip to content

Support __available__((swift_attr("@Sendable"))) #40170

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 8 commits into from
Nov 19, 2021

Conversation

beccadax
Copy link
Contributor

@beccadax beccadax commented Nov 13, 2021

This PR implements most of the sendability auditing support for ClangImporter. Specifically:

  • __attribute__((swift_attr("@Sendable"))) is now allowed on Clang decls, even when it would not be allowed on equivalent Swift decls.
  • A Clang type declaration is imported with a synthesized @unchecked Sendable conformance if it:
    • Has @Sendable, but not plain @_nonSendable.
    • Has the enum_extensibility, flag_enum, swift_newtype, or swift_wrapper Clang attributes, but not plain @_nonSendable.
    • Has the ns_error_domain Clang attribute, regardless of the presence of plain @_nonSendable. (Error is always Sendable, so error codes must also be Sendable.)
  • We now force implicit Sendable conformances before printing the contents of a module, and we print the extensions generated by @_nonSendable even if we would normally skip unavailable or implicit extensions.
  • When ClangImporter is using a clang that supports https://reviews.llvm.org/D112773 (see [20210726] Allow __attribute__((swift_attr)) in attribute push pragmas llvm-project#3547), it will define __SWIFT_ATTR_SUPPORTS_SENDABLE_DECLS to be 1.

It includes a number of related refactorings:

  • SynthesizedProtocolAttrs can now create @unchecked conformances.
  • SynthesizedFileUnits can now be attached to any FileUnit, not just SourceFile.
  • ClangImporter::Implementation::diagnose() has been overloaded so that, if a HeaderLoc (wrapping a clang::SourceLocation and related info) is passed in place of a SourceLoc, it will emit a diagnostic in the header file. This replaces several lines of boilerplate per use site.

Fixes rdar://84431708. It does not implement two things that I will defer to 85569247:

  • @_nonSendable in parameter position.
  • Implicit @Sendable (probably @_unsafeSendable in practice) on completion handler parameters.

Some tests will not pass without https://reviews.llvm.org/D112773, which I am in the process of cherry-picking to our clang branches.

This should only cause language behavior changes if you consume headers that include the currently-invalid __attribute__((swift_attr("@Sendable"))), __attribute__((swift_attr("@_nonSendable"))), or __attribute__((swift_attr("@_nonSendable(_assumed)"))) attributes.

This is not yet used by anything.
This change applies SwiftAttr attributes as soon as possible after creating an instance of a Decl, rather than waiting until the declaration is "finished". That makes sure the attributes can influence the declaration very early in its lifecycle, and in particular, before its conformance table is initialized.

Mostly NFC in this commit (other than affecting the order that attributes are printed in), but necessary for future changes in this PR.
...by using `__attribute__((swift_attr("@sendable")))`. `@_nonSendable` will "beat" `@Sendable`, while `@_nonSendable(_assumed)` will not.

This commit also checks if `SwiftAttr` supports `#pragma clang attribute` and, if it does, defines `__SWIFT_ATTR_SUPPORTS_SENDABLE_DECLS` in imported headers so they know they can apply these attributes in an auditing style.
Gives us a place to stuff synthesized declarations for, among other things, imported Clang decls.
An explicit swift_attr("@_nonSendable") will override it (except for ns_error_domain where the type is embedded in another type that's forced to be Sendable), but swift_attr("@_nonSendable(_assumed)") will not.
@beccadax
Copy link
Contributor Author

With swiftlang/llvm-project#3547

@swift-ci please smoke test

@beccadax beccadax marked this pull request as ready for review November 18, 2021 22:41
@beccadax beccadax requested review from DougGregor and kavon November 18, 2021 22:47
@beccadax
Copy link
Contributor Author

With swiftlang/llvm-project#3547

@swift-ci please smoke test

Copy link
Member

@DougGregor DougGregor left a comment

Choose a reason for hiding this comment

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

Some minor comments, but this looks great!

// GetImplicitSendableRequest::evaluate() creates its extension with the
// attribute's AtLoc, so this is a good way to quickly check if the extension
// was synthesized for an '@_nonSendable' attribute.
return ED->getLocFromSource() == nonSendable->AtLoc;
Copy link
Member

Choose a reason for hiding this comment

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

This feels a little... brittle... but okay.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah. :/ I couldn’t think of anything better that would be quick, but maybe I can revisit this; arguably we should just print any unavailable Sendable conformance even if it doesn’t come from @_nonSendable.

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