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

Solutions for ClangCL intrin0.h header for _STL_INTRIN_HEADER macro #4282

Closed
wants to merge 4 commits into from
Closed

Solutions for ClangCL intrin0.h header for _STL_INTRIN_HEADER macro #4282

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
76a6330
Solutions for ClangCL msvc intrin header
MaxEW707 Dec 21, 2023
5399a28
fix formatting
MaxEW707 Dec 21, 2023
386c0ca
fix missing `>`
MaxEW707 Dec 21, 2023
dc397a3
Fix header name
MaxEW707 Jan 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 7 additions & 1 deletion stl/inc/yvals_core.h
View file
Open in desktop
Expand Up @@ -2034,8 +2034,14 @@ compiler option, or define _ALLOW_RTCc_IN_STL to suppress this error.
#endif // ^^^ !defined(__cpp_noexcept_function_type) ^^^

#ifdef __clang__
#if __clang_major__ >= 18
Copy link
Member

Choose a reason for hiding this comment

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

I have a mild preference for detection via __clang_major__ over using a distinct header name. We don't support compilers older than most-recently-shipped-with-VS, so this will let us replace the whole mess with #include <intrin0.h> in a release cycle or two.

We'll see if other maintainers have different opinions. (STL is notably on vacation for another week.)

Copy link
Author

@MaxEW707 MaxEW707 Jan 14, 2024 •
edited

Choose a reason for hiding this comment

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

llvm/llvm-project@f39e4aa for reference.

Quick update. I added an interposing yvals_core.h header to my PR which ensures that when we introduce this change we do not reduce throughput for users upgrading clang but still referring to an older version of MSVC STL.

If that solution is accepted and when that PR gets merged and shipped with some release of clang you should be able to just replace _STL_INTRIN_HEADER with <intrin0.h> once MSVC STL targets that release of clang with the intrin0.h header.

#define _STL_INTRIN_HEADER <intrin0.h>
#elif __has_include(<intrin_msvc_stl.h>)
#define _STL_INTRIN_HEADER <intrin_msvc_stl.h>
MaxEW707 marked this conversation as resolved.
Show resolved Hide resolved
#else
#define _STL_INTRIN_HEADER <intrin.h>
#define _STL_UNREACHABLE __builtin_unreachable()
#endif
#define _STL_UNREACHABLE __builtin_unreachable()
#else // ^^^ defined(__clang__) / !defined(__clang__) vvv
#define _STL_INTRIN_HEADER <intrin0.h>
#define _STL_UNREACHABLE __assume(false)
Expand Down