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

<system_error>: Use uglified [[clang::require_constant_initialization]] attribute #3380

Merged
merged 1 commit into from Feb 10, 2023

Conversation

JMazurkiewicz
Copy link
Contributor

@JMazurkiewicz JMazurkiewicz commented Feb 3, 2023

Currently <system_error> header uses non-reserved identifiers clang and require_constant_initialization which is incorrect (see #2645):

PS D:\stl-playground> type .\clang_rci.cpp
// Non-reserved identifiers
#define clang
#define require_constant_initialization

#include <system_error> // Whoops!
PS D:\stl-playground> clang .\clang_rci.cpp
In file included from .\clang_rci.cpp:5:
D:\stl\out\build\x64\out\inc\system_error:646:7: error: expected ']'
    [[clang::require_constant_initialization]] static _Ty _Static;
      ^
1 error generated.

This PR simply uglifies this attribute.

@JMazurkiewicz JMazurkiewicz requested a review from a team as a code owner February 3, 2023 00:37
@CaseyCarter CaseyCarter added the bug Something isn't working label Feb 3, 2023
@CaseyCarter CaseyCarter added this to Initial Review in Code Reviews via automation Feb 3, 2023
@CaseyCarter CaseyCarter moved this from Initial Review to Final Review in Code Reviews Feb 3, 2023
@StephanTLavavej
Copy link
Member

Neat, I wasn't aware that this was officially documented:

The attribute scope tokens clang and _Clang are interchangeable, as are the attribute scope tokens gnu and __gnu__. Attribute tokens in either of these namespaces can be specified with a preceding and following __ (double underscore) to avoid interference from a macro with the same name. For instance, gnu::__const__ can be used instead of gnu::const.

@StephanTLavavej StephanTLavavej moved this from Final Review to Ready To Merge in Code Reviews Feb 3, 2023
@StephanTLavavej StephanTLavavej self-assigned this Feb 10, 2023
@StephanTLavavej
Copy link
Member

I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed.

@StephanTLavavej StephanTLavavej merged commit 7dde18b into microsoft:main Feb 10, 2023
Code Reviews automation moved this from Ready To Merge to Done Feb 10, 2023
@StephanTLavavej
Copy link
Member

Thanks for noticing and fixing this minor conformance issue! 🐛 🛠️ ✨

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

4 participants