-
Notifications
You must be signed in to change notification settings - Fork 14.4k
[libc++] Make bad_function_call::what() existence a matter of availability instead of ABI #127697
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
[libc++] Make bad_function_call::what() existence a matter of availability instead of ABI #127697
Conversation
@llvm/pr-subscribers-libcxx Author: Nikolas Klauser (philnik777) ChangesWe're currently adding Full diff: https://github.com/llvm/llvm-project/pull/127697.diff 3 Files Affected:
diff --git a/libcxx/include/__configuration/availability.h b/libcxx/include/__configuration/availability.h
index 261cf9c1ae9d8..bdb72d86c2603 100644
--- a/libcxx/include/__configuration/availability.h
+++ b/libcxx/include/__configuration/availability.h
@@ -78,6 +78,9 @@
// in all versions of the library are available.
#if !_LIBCPP_HAS_VENDOR_AVAILABILITY_ANNOTATIONS
+# define _LIBCPP_INTRODUCED_IN_LLVM_21 1
+# define _LIBCPP_INTRODUCED_IN_LLVM_21_ATTRIBUTE /* nothing */
+
# define _LIBCPP_INTRODUCED_IN_LLVM_20 1
# define _LIBCPP_INTRODUCED_IN_LLVM_20_ATTRIBUTE /* nothing */
@@ -114,6 +117,11 @@
// clang-format off
+// LLVM 21
+// TODO: Fill this in
+# define _LIBCPP_INTRODUCED_IN_LLVM_21 0
+# define _LIBCPP_INTRODUCED_IN_LLVM_21_ATTRIBUTE __attribute__((unavailable))
+
// LLVM 20
// TODO: Fill this in
# define _LIBCPP_INTRODUCED_IN_LLVM_20 0
@@ -359,6 +367,8 @@
#define _LIBCPP_AVAILABILITY_HAS_FROM_CHARS_FLOATING_POINT _LIBCPP_INTRODUCED_IN_LLVM_20
#define _LIBCPP_AVAILABILITY_FROM_CHARS_FLOATING_POINT _LIBCPP_INTRODUCED_IN_LLVM_20_ATTRIBUTE
+#define _LIBCPP_AVAILABILITY_HAS_BAD_FUNCTION_CALL_GOOD_WHAT_MESSAGE _LIBCPP_INTRODUCED_IN_LLVM_21
+
// Define availability attributes that depend on _LIBCPP_HAS_EXCEPTIONS.
// Those are defined in terms of the availability attributes above, and
// should not be vendor-specific.
diff --git a/libcxx/include/__functional/function.h b/libcxx/include/__functional/function.h
index 2a1293cfcc26b..cd77012173512 100644
--- a/libcxx/include/__functional/function.h
+++ b/libcxx/include/__functional/function.h
@@ -71,7 +71,7 @@ class _LIBCPP_EXPORTED_FROM_ABI bad_function_call : public exception {
_LIBCPP_HIDE_FROM_ABI_VIRTUAL ~bad_function_call() _NOEXCEPT override {}
# endif
-# ifdef _LIBCPP_ABI_BAD_FUNCTION_CALL_GOOD_WHAT_MESSAGE
+# if _LIBCPP_AVAILABILITY_HAS_BAD_FUNCTION_CALL_GOOD_WHAT_MESSAGE
const char* what() const _NOEXCEPT override;
# endif
};
diff --git a/libcxx/src/functional.cpp b/libcxx/src/functional.cpp
index ef53e3e84da0e..f2c70fc54392d 100644
--- a/libcxx/src/functional.cpp
+++ b/libcxx/src/functional.cpp
@@ -12,8 +12,6 @@ _LIBCPP_BEGIN_NAMESPACE_STD
bad_function_call::~bad_function_call() noexcept {}
-#ifdef _LIBCPP_ABI_BAD_FUNCTION_CALL_GOOD_WHAT_MESSAGE
const char* bad_function_call::what() const noexcept { return "std::bad_function_call"; }
-#endif
_LIBCPP_END_NAMESPACE_STD
|
998fb73
to
ba00842
Compare
ba00842
to
6804b1c
Compare
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.
This LGTM. However, since I do expect we might run into some unforeseen issues with this change, let me take it for a spin internally just to see if anything breaks. But assuming that's not the case, let's ship this. I'll let you know early next week.
Edit for future reference: 148470934&148470973
// Override the default return value of exception::what() for bad_function_call::what() | ||
// with a string that is specific to bad_function_call (see http://wg21.link/LWG2233). | ||
// This is an ABI break on platforms that sign and authenticate vtable function pointers | ||
// because it changes the mangling of the virtual function located in the vtable, which | ||
// changes how it gets signed. |
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.
As we discussed in Hagenberg with Oliver Hunt, this information is actually untrue and I think we (I?) put it there by mistaking an issue I had seen with another kind of issue, likely related to ABI tags.
I didn't see any issues with this, I think this is good to merge once my comments have been addressed. |
878ceec
to
40fc67f
Compare
…ility instead of ABI
40fc67f
to
8bec177
Compare
…ility instead of ABI (llvm#127697) We're currently adding `bad_function_call::what()` behind an ABI flag, even though adding it is not an ABI break and can be handled through availability.
…ility instead of ABI (llvm#127697) We're currently adding `bad_function_call::what()` behind an ABI flag, even though adding it is not an ABI break and can be handled through availability.
…ility instead of ABI (llvm#127697) We're currently adding `bad_function_call::what()` behind an ABI flag, even though adding it is not an ABI break and can be handled through availability.
…ility instead of ABI (llvm#127697) We're currently adding `bad_function_call::what()` behind an ABI flag, even though adding it is not an ABI break and can be handled through availability.
We're currently adding
bad_function_call::what()
behind an ABI flag, even though adding it is not an ABI break and can be handled through availability.