-
Notifications
You must be signed in to change notification settings - Fork 10.8k
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
release/18.x: [Headers] Don't declare unreachable() from stddef.h in C++ (#86748) #87696
Conversation
@AaronBallman What do you think about merging this PR to the release branch? |
@llvm/pr-subscribers-backend-x86 @llvm/pr-subscribers-clang Author: None (llvmbot) ChangesBackport df69a30 Requested by: @ian-twilightcoder Full diff: https://github.com/llvm/llvm-project/pull/87696.diff 1 Files Affected:
diff --git a/clang/lib/Headers/__stddef_unreachable.h b/clang/lib/Headers/__stddef_unreachable.h
index 518580c92d3f5d..61df43e9732f8a 100644
--- a/clang/lib/Headers/__stddef_unreachable.h
+++ b/clang/lib/Headers/__stddef_unreachable.h
@@ -7,6 +7,8 @@
*===-----------------------------------------------------------------------===
*/
+#ifndef __cplusplus
+
/*
* When -fbuiltin-headers-in-system-modules is set this is a non-modular header
* and needs to behave as if it was textual.
@@ -15,3 +17,5 @@
(__has_feature(modules) && !__building_module(_Builtin_stddef))
#define unreachable() __builtin_unreachable()
#endif
+
+#endif
|
Nominating for backport because this is a C++ regression that was caused by https://reviews.llvm.org/D157757 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM for backporting
@tstellar is there anything else I need to do for this one? |
Even if __need_unreachable is set, stddef.h should not declare unreachable() in C++ because it conflicts with the declaration in \<utility>. (cherry picked from commit df69a30)
Hi @ian-twilightcoder (or anyone else). If you would like to add a note about this fix in the release notes (completely optional). Please reply to this comment with a one or two sentence description of the fix. |
I don't think we need to release note this one, it's just restoring LLVM 17 behavior. |
Backport df69a30
Requested by: @ian-twilightcoder