Skip to content
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

Updates to @_predatesConcurrency #40271

Merged
merged 4 commits into from
Nov 30, 2021

Conversation

DougGregor
Copy link
Member

Implement a few remaining pieces of the @_predatesConcurrency attribute:

  • Adjust the mangled type of @_predatesConcurrency declarations to drop Sendable, @Sendable, and global actor annotations
  • All imported C declarations are @_predatesConcurrency by default

@DougGregor
Copy link
Member Author

@swift-ci please build toolchain

@swift-ci
Copy link
Contributor

Linux Toolchain (Ubuntu 16.04)
Download Toolchain
Git Sha - fdb455d0783973bbb397cde9b843cf546032b422

Install command
tar zxf swift-PR-40271-733-ubuntu16.04.tar.gz
More info

Copy link
Contributor

@beccadax beccadax left a comment

Choose a reason for hiding this comment

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

A nit and a question, but this looks good.

Comment on lines +726 to +728
// Imported C declarations always predate concurrency.
if (isa<ClangModuleUnit>(getDeclContext()->getModuleScopeContext()))
return true;
Copy link
Contributor

Choose a reason for hiding this comment

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

Out of curiosity, what will this match that hasClangNode() wouldn't or vice versa?

Copy link
Member Author

Choose a reason for hiding this comment

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

IIRC, there's some cases involving synthesized extensions for import-as-member that differ.

// Strip off Sendable and (possibly) the global actor.
ASTExtInfo extInfo =
fnType->hasExtInfo() ? fnType->getExtInfo() : ASTExtInfo();
extInfo = extInfo.withConcurrent(false);
Copy link
Contributor

Choose a reason for hiding this comment

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

withConcurrent() should probably get renamed eventually, but not today.

@@ -67,6 +67,9 @@ class ASTMangler : public Mangler {
/// If enabled, marker protocols can be encoded in the mangled name.
bool AllowMarkerProtocols = true;

/// Whether the mangling predates concurrency, and therefore shouldn't
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you didn't finish typing this.

@swift-ci
Copy link
Contributor

macOS Toolchain
Download Toolchain
Git Sha - fdb455d0783973bbb397cde9b843cf546032b422

Install command
tar -zxf swift-PR-40271-1219-osx.tar.gz --directory ~/

…rency decls

This allows some APIs to evolve toward supporting concurrency without
breaking their ABI.
The `@MainActor(unsafe)` attribute could be provided for C declarations
via the Clang `swift_attr` attribute. However, this facility was never
used outside of tests, and has been superceded by `@MainActor` with the
inferred `@_predatesConcurrency`.
@DougGregor
Copy link
Member Author

@swift-ci please smoke test and merge

@DougGregor
Copy link
Member Author

@swift-ci please smoke test

@swift-ci swift-ci merged commit c5fa0cf into swiftlang:main Nov 30, 2021
@DougGregor DougGregor deleted the predates-concurrency-abi-c branch November 30, 2021 05:24
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.

None yet

3 participants