Skip to content

rename <stdatomic.h> to <__msvc_cxx_stdatomic.h>#3183

Merged
barcharcraz merged 7 commits intomicrosoft:mainfrom
barcharcraz:dev/chbarto/rename_stdatomic.h
Oct 27, 2022
Merged

rename <stdatomic.h> to <__msvc_cxx_stdatomic.h>#3183
barcharcraz merged 7 commits intomicrosoft:mainfrom
barcharcraz:dev/chbarto/rename_stdatomic.h

Conversation

@barcharcraz
Copy link
Contributor

@barcharcraz barcharcraz commented Oct 26, 2022

we are adding C11 atomics support to the compiler/vcruntime. because of this <stdatomic.h> needs to become part of vcruntime. This PR renames the STL's "compatibility" stdatomic.h to __msvc_cxx_stdatomic.h and updates references to stdatomic.h to instead point to __msvc_cxx_stdatomic.h. The "real" stdatomic.h is added to vcruntime in an internal mirror of this PR: MSVC-PR-423692, and it simply includes either __msvc_cxx_stdatomic.h or vcruntime_c11_stdatomic.h depending on __cplusplus.

The one inclusion of "stdatomic.h" in our tests was updated, so our tests should continue to pass, even if the runtime used to build the STL does not have stdatomic.h.

@barcharcraz barcharcraz requested a review from a team as a code owner October 26, 2022 20:53
@CaseyCarter CaseyCarter added the enhancement Something can be improved label Oct 26, 2022
@StephanTLavavej StephanTLavavej self-assigned this Oct 26, 2022
@CaseyCarter CaseyCarter added the high priority Important! label Oct 26, 2022
@barcharcraz barcharcraz force-pushed the dev/chbarto/rename_stdatomic.h branch from 458ba0a to 43225db Compare October 26, 2022 22:33
CaseyCarter
CaseyCarter reviewed Oct 26, 2022
View reviewed changes
@frederick-vs-ja
Copy link
Contributor

frederick-vs-ja commented Oct 27, 2022

I think this error message will be confusing after the changes. It still says <stdatomic.h>, but if one include <stdatomic.h> in C11/17 mode (after the corresponding changes in vcruntime), this #error won't be encountered.

Perhaps we should say something like <__msvc_cxx_stdatomic.hpp> is an internal header. It is incompatible with C and should not be directly included..

STL/stl/inc/__msvc_cxx_stdatomic.hpp

Lines 13 to 16 in 996ad1f

// provide a specific error message for C compilers, before the general error message in yvals_core.h
#ifndef __cplusplus
#error <stdatomic.h> is not yet supported when compiling as C, but this is planned for a future release.
#endif // __cplusplus

@CaseyCarter CaseyCarter mentioned this pull request Oct 27, 2022
Closed
@CaseyCarter
Copy link
Contributor

CaseyCarter commented Oct 27, 2022

I think this error message will be confusing after the changes.

I agree the error message is now bad. I'd prefer that we fix it in a followup PR rather than this one given that we have a few hours to make 17.5p2 and we're still fighting to get a clean internal test run. I've filed #3184 to ensure we don't forget to follow up.

@barcharcraz barcharcraz merged commit c873cf0 into microsoft:main Oct 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@StephanTLavavej StephanTLavavej StephanTLavavej approved these changes

+1 more reviewer

@CaseyCarter CaseyCarter CaseyCarter approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

enhancement Something can be improved high priority Important!

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@barcharcraz @frederick-vs-ja @CaseyCarter @StephanTLavavej