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.

Sign up for GitHub

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

Jump to bottom

STL headers should require C++ and forbid C #2148

Merged
merged 6 commits into from
Aug 31, 2021
Merged

STL headers should require C++ and forbid C #2148

merged 6 commits into from
Aug 31, 2021

Conversation

fsb4000
Copy link
Contributor

@fsb4000 fsb4000 commented Aug 22, 2021

Fixes #1141

@fsb4000 fsb4000 requested a review from a team as a code owner August 22, 2021 13:23
@CaseyCarter CaseyCarter added the enhancement Something can be improved label Aug 23, 2021
@StephanTLavavej
Copy link
Member

There will be a stealth merge conflict with #2140, where <stdatomic.h> wants to emit a specific #error for C. I believe we could simply move that #error above #include <yvals.h>.

@StephanTLavavej StephanTLavavej self-assigned this Aug 25, 2021
@StephanTLavavej
Copy link
Member

@fsb4000 @CaseyCarter I've pushed a series of commits:

  • Merged with main.
  • Added a period to the error message, for consistency with the existing messages:

    STL/stl/inc/yvals_core.h

    Line 585 in 78ff461

    #error STL1001: Unexpected compiler version, expected MSVC 19.30 or newer.
  • In yvals_core.h, moved the check within _STL_COMPILER_PREPROCESSOR. We want to continue to expand to nothing for known non-compiler tools.
  • Change iso646.h to avoid defining _STL_COMPILER_PREPROCESSOR (which may be problematic for header unit scenarios where two header units try to emit the same macro independently) and instead just repeat the logic inline, with a bit of commenting.
  • Resolve the stealth merge conflict in stdatomic.h by repeating the _STL_COMPILER_PREPROCESSOR logic inline, then emitting a specific error before the general one.

@StephanTLavavej StephanTLavavej removed their assignment Aug 28, 2021
@StephanTLavavej StephanTLavavej self-assigned this Aug 30, 2021
@StephanTLavavej
Copy link
Member

I have mirrored this to internal MSVC-PR-347845 (all by itself, as I believe it has a moderate risk of breaking stuff). Please notify me if any further changes are pushed.

@StephanTLavavej StephanTLavavej merged commit 4e8d337 into microsoft:main Aug 31, 2021
@StephanTLavavej
Copy link
Member

Thanks for improving the diagnostics here! 🎉 😸 🚀

@fsb4000 fsb4000 deleted the fix1141 branch August 31, 2021 13:26
@Cheney-W Cheney-W mentioned this pull request Sep 6, 2021
Closed
@danilw danilw mentioned this pull request Apr 24, 2022
Closed
BlurryRoots pushed a commit to think-biq/libunistd that referenced this pull request May 1, 2022
restructured header files for consistent presentation of c interface
13fb958 
moved all c++ specific implementation details, as well as references to any STL header
to their respective compilation units. created a seperate compilation unit for pthread
as to allow for forward declaring the necessary structs in a consistent c fashion, while
implementing it in c++ in the unit. this allows for seperation of header use for c
without pulling in any STL header, as this will lead to compiler errors on windows.
for details checkout ofiwg/libfabric#7041 (comment)
or the STL commit microsoft/STL#2148
BlurryRoots added a commit to think-biq/libunistd that referenced this pull request May 1, 2022
@BlurryRoots
restructured header files for consistent presentation of c interface
1d9e221 
moved all c++ specific implementation details, as well as references to any STL header
to their respective compilation units. created a seperate compilation unit for pthread
as to allow for forward declaring the necessary structs in a consistent c fashion, while
implementing it in c++ in the unit. this allows for seperation of header use for c
without pulling in any STL header, as this will lead to compiler errors on windows.
for details checkout ofiwg/libfabric#7041 (comment)
or the STL commit microsoft/STL#2148
@ahmidou ahmidou mentioned this pull request Jul 19, 2022
Closed
@ahmidou ahmidou mentioned this pull request Jul 19, 2022
Open
@amrezaee amrezaee mentioned this pull request Oct 7, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@miscco miscco miscco left review comments

@cpplearner cpplearner cpplearner left review comments

@CaseyCarter CaseyCarter CaseyCarter approved these changes

@StephanTLavavej StephanTLavavej StephanTLavavej approved these changes

Assignees

@StephanTLavavej StephanTLavavej

Labels
enhancement Something can be improved
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

STL headers should require C++ and forbid C
5 participants
@fsb4000 @StephanTLavavej @CaseyCarter @miscco @cpplearner
  翻译：