Skip to content

Runtime: correct attribute annotations for newer compilers #61489

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

Closed
wants to merge 1 commit into from

Conversation

compnerd
Copy link
Member

@compnerd compnerd commented Oct 7, 2022

extern "C" must precede any attributes. We were previously incorrectly applying the attributes to the declaration that was marked as extern "C" in C++ contexts. Reorder the attributes to correct this oversight allowing compilation with newer clang releases.

`extern "C"` *must* precede any attributes.  We were previously
incorrectly applying the attributes to the declaration that was
marked as `extern "C"` in C++ contexts.  Reorder the attributes
to correct this oversight allowing compilation with newer clang
releases.
@compnerd
Copy link
Member Author

compnerd commented Oct 7, 2022

@swift-ci please smoke test

@allevato
Copy link
Member

allevato commented Oct 7, 2022

FYI: I had already tried this before the solution I came to in #61473, and it didn't work with our more modern internal Clang. I got the same errors about positioning as before:

In file included from swift/stdlib/public/runtime/ErrorDefaultImpls.cpp:19:
swift/include/swift/Runtime/Metadata.h:316:22: error: 'returns_nonnull' attribute cannot be applied to types
SWIFT_RUNTIME_EXPORT SWIFT_RETURNS_NONNULL SWIFT_NODISCARD
                     ^
swift/stdlib/public/SwiftShims/swift/shims/Visibility.h:240:33: note: expanded from macro 'SWIFT_RETURNS_NONNULL'
#define SWIFT_RETURNS_NONNULL [[gnu::returns_nonnull]]
                                ^
swift/stdlib/public/runtime/ErrorDefaultImpls.cpp:19:
swift/include/swift/Runtime/Metadata.h:316:44: error: 'nodiscard' attribute cannot be applied to types
SWIFT_RUNTIME_EXPORT SWIFT_RETURNS_NONNULL SWIFT_NODISCARD
                                           ^
swift/stdlib/public/SwiftShims/swift/shims/Visibility.h:232:27: note: expanded from macro 'SWIFT_NODISCARD'
#define SWIFT_NODISCARD [[nodiscard]]
                          ^

@compnerd
Copy link
Member Author

compnerd commented Oct 7, 2022

https://godbolt.org/z/eac6PYW73 seems to indicate that it should work with clang trunk and that gcc also behaves identically.

@allevato
Copy link
Member

allevato commented Oct 7, 2022

The full code with the macros in place does not work, however: https://godbolt.org/z/9cc53PocK

@compnerd
Copy link
Member Author

compnerd commented Oct 7, 2022

https://godbolt.org/z/ov9e53KMq reduced test case - seems like a clang issue

@compnerd
Copy link
Member Author

compnerd commented Oct 7, 2022

Verified that this is a rejects-valid case: llvm/llvm-project#58229

@compnerd
Copy link
Member Author

Closing in favour of fixing llvm/llvm-project#58229 (https://reviews.llvm.org/D137979)

@compnerd compnerd closed this Nov 16, 2022
@compnerd compnerd deleted the order branch November 16, 2022 19:05
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