-
Notifications
You must be signed in to change notification settings - Fork 14.9k
[libc] Add _Returns_twice to C++ code #153602
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
Conversation
Fixes issue with <csetjmp> which requires _Returns_twice but in C++ mode
@llvm/pr-subscribers-libc Author: William Huynh (saturn691) ChangesFixes issue with Full diff: https://github.com/llvm/llvm-project/pull/153602.diff 1 Files Affected:
diff --git a/libc/include/__llvm-libc-common.h b/libc/include/__llvm-libc-common.h
index c6fd33a55532c..b376b05df1678 100644
--- a/libc/include/__llvm-libc-common.h
+++ b/libc/include/__llvm-libc-common.h
@@ -47,6 +47,9 @@
#define __NOEXCEPT throw()
#endif
+#undef _Returns_twice
+#define _Returns_twice [[gnu::returns_twice]]
+
// This macro serves as a generic cast implementation for use in both C and C++,
// similar to `__BIONIC_CAST` in Android.
#undef __LLVM_LIBC_CAST
|
libc/include/__llvm-libc-common.h
Outdated
#undef _Returns_twice | ||
#define _Returns_twice [[gnu::returns_twice]] | ||
|
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.
given that this is on both sides of the #ifdef __cplusplus
branch is there a way we can unify it instead of having two copies? Also is there a reason you used the gnu attribute instead of the generic one?
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.
returns_twice
needs to be namespaced (https://en.cppreference.com/w/cpp/language/attributes.html)
❯ clang test.c
test.c:3:3: warning: unknown attribute 'returns_twice' ignored [-Wunknown-attributes]
3 | [[returns_twice]] int f() {
| ^~~~~~~~~~~~~
1 warning generated.
[[attribute]]
syntax is only supported in C++11. It seems to compile for older versions and this has even been back-ported to C code in clang. I'll push an update to unify this.
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.
The [[attribute]]
syntax was only introduced in C20 so we shouldn't be using it in C code since we want our headers are compatible with older C standards.
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.
Good point, I've settled with preferring to generate the new attribute syntax if the compiler supports it (C++11) onwards.
f196fc0
to
8e7da2c
Compare
Fixes issue with
<csetjmp>
which requires_Returns_twice
but in C++ mode