-
Notifications
You must be signed in to change notification settings - Fork 15.6k
[libc++] Inline fast path for exception_ptr copy constructor & destructor
#165909
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
base: main
Are you sure you want to change the base?
Changes from 16 commits
b97ad48
419f64d
b32d1be
ddaf78e
a026801
f787db6
d731778
a7316f9
2ef680d
455a2a6
68f72ea
12373fa
4c75c4e
2e0cbd1
209c1e3
96f1f68
9dbbc70
addd78f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,18 +17,13 @@ | |
| #include <__memory/construct_at.h> | ||
| #include <__type_traits/decay.h> | ||
| #include <__type_traits/is_pointer.h> | ||
| #include <__utility/move.h> | ||
| #include <__utility/swap.h> | ||
| #include <__verbose_abort> | ||
| #include <typeinfo> | ||
|
|
||
| #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER) | ||
| # pragma GCC system_header | ||
| #endif | ||
|
|
||
| _LIBCPP_PUSH_MACROS | ||
| #include <__undef_macros> | ||
|
|
||
| #ifndef _LIBCPP_ABI_MICROSOFT | ||
|
|
||
| # if _LIBCPP_AVAILABILITY_HAS_INIT_PRIMARY_EXCEPTION | ||
|
|
@@ -63,11 +58,12 @@ _LIBCPP_BEGIN_UNVERSIONED_NAMESPACE_STD | |
|
|
||
| #ifndef _LIBCPP_ABI_MICROSOFT | ||
|
|
||
| inline _LIBCPP_HIDE_FROM_ABI void swap(exception_ptr& __x, exception_ptr& __y) _NOEXCEPT; | ||
|
|
||
| class _LIBCPP_EXPORTED_FROM_ABI exception_ptr { | ||
| void* __ptr_; | ||
|
|
||
| static void __increment_refcount([[__gnu__::__nonnull__]] _LIBCPP_NOESCAPE void*) _NOEXCEPT; | ||
| static void __decrement_refcount([[__gnu__::__nonnull__]] _LIBCPP_NOESCAPE void*) _NOEXCEPT; | ||
|
|
||
| static exception_ptr __from_native_exception_pointer(void*) _NOEXCEPT; | ||
|
|
||
| template <class _Ep> | ||
|
|
@@ -82,17 +78,42 @@ class _LIBCPP_EXPORTED_FROM_ABI exception_ptr { | |
| _LIBCPP_HIDE_FROM_ABI exception_ptr() _NOEXCEPT : __ptr_() {} | ||
| _LIBCPP_HIDE_FROM_ABI exception_ptr(nullptr_t) _NOEXCEPT : __ptr_() {} | ||
|
|
||
| // These symbols are still exported from the library to prevent ABI breakage. | ||
| # ifdef _LIBCPP_BUILDING_LIBRARY | ||
| exception_ptr(const exception_ptr&) _NOEXCEPT; | ||
| exception_ptr& operator=(const exception_ptr&) _NOEXCEPT; | ||
| ~exception_ptr() _NOEXCEPT; | ||
| # else // _LIBCPP_BUILDING_LIBRARY | ||
| _LIBCPP_HIDE_FROM_ABI exception_ptr(const exception_ptr& __other) _NOEXCEPT : __ptr_(__other.__ptr_) { | ||
| if (__ptr_) | ||
| __increment_refcount(__ptr_); | ||
| } | ||
| _LIBCPP_HIDE_FROM_ABI exception_ptr& operator=(const exception_ptr& __other) _NOEXCEPT { | ||
| if (__ptr_ == __other.__ptr_) | ||
| return *this; | ||
| if (__other.__ptr_) | ||
| __increment_refcount(__other.__ptr_); | ||
| if (__ptr_) | ||
| __decrement_refcount(__ptr_); | ||
| __ptr_ = __other.__ptr_; | ||
| return *this; | ||
| } | ||
| _LIBCPP_HIDE_FROM_ABI ~exception_ptr() _NOEXCEPT { | ||
| if (__ptr_) | ||
| __decrement_refcount(__ptr_); | ||
| } | ||
| # endif // _LIBCPP_BUILDING_LIBRARY | ||
|
|
||
| _LIBCPP_HIDE_FROM_ABI exception_ptr(exception_ptr&& __other) _NOEXCEPT : __ptr_(__other.__ptr_) { | ||
| __other.__ptr_ = nullptr; | ||
| } | ||
| exception_ptr& operator=(const exception_ptr&) _NOEXCEPT; | ||
| _LIBCPP_HIDE_FROM_ABI exception_ptr& operator=(exception_ptr&& __other) _NOEXCEPT { | ||
| exception_ptr __tmp(std::move(__other)); | ||
| std::swap(__tmp, *this); | ||
| if (__ptr_) | ||
| __decrement_refcount(__ptr_); | ||
| __ptr_ = __other.__ptr_; | ||
| __other.__ptr_ = nullptr; | ||
|
Comment on lines
+111
to
+114
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this change required?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point. We have 3 choices:
To me, (3) actually feels most natural. My gut feeling tells me that "move" assignments are more frequently used/specialized than I updated the PR accordingly. Please let me know in case you prefer going into a different direction |
||
| return *this; | ||
| } | ||
| ~exception_ptr() _NOEXCEPT; | ||
|
|
||
| _LIBCPP_HIDE_FROM_ABI explicit operator bool() const _NOEXCEPT { return __ptr_ != nullptr; } | ||
|
|
||
|
|
@@ -104,16 +125,10 @@ class _LIBCPP_EXPORTED_FROM_ABI exception_ptr { | |
| return !(__x == __y); | ||
| } | ||
|
|
||
| friend _LIBCPP_HIDE_FROM_ABI void swap(exception_ptr& __x, exception_ptr& __y) _NOEXCEPT; | ||
|
|
||
| friend _LIBCPP_EXPORTED_FROM_ABI exception_ptr current_exception() _NOEXCEPT; | ||
| friend _LIBCPP_EXPORTED_FROM_ABI void rethrow_exception(exception_ptr); | ||
| }; | ||
|
|
||
| inline _LIBCPP_HIDE_FROM_ABI void swap(exception_ptr& __x, exception_ptr& __y) _NOEXCEPT { | ||
| std::swap(__x.__ptr_, __y.__ptr_); | ||
| } | ||
|
|
||
| # if _LIBCPP_HAS_EXCEPTIONS | ||
| # if _LIBCPP_AVAILABILITY_HAS_INIT_PRIMARY_EXCEPTION | ||
| template <class _Ep> | ||
|
|
@@ -223,6 +238,4 @@ _LIBCPP_HIDE_FROM_ABI exception_ptr make_exception_ptr(_Ep __e) _NOEXCEPT { | |
| #endif // _LIBCPP_ABI_MICROSOFT | ||
| _LIBCPP_END_UNVERSIONED_NAMESPACE_STD | ||
|
|
||
| _LIBCPP_POP_MACROS | ||
|
|
||
| #endif // _LIBCPP___EXCEPTION_EXCEPTION_PTR_H | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| // -*- C++ -*- | ||
| //===----------------------------------------------------------------------===// | ||
| // | ||
| // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. | ||
| // See https://llvm.org/LICENSE.txt for license information. | ||
| // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception | ||
| // | ||
| //===----------------------------------------------------------------------===// | ||
|
|
||
| // Provides the common functionality shared between cxxabi and glibcxx. | ||
|
|
||
| namespace std { | ||
|
|
||
| exception_ptr exception_ptr::__from_native_exception_pointer(void* __e) noexcept { | ||
| exception_ptr ptr; | ||
| ptr.__ptr_ = __e; | ||
| __increment_refcount(ptr.__ptr_); | ||
|
|
||
| return ptr; | ||
| } | ||
|
|
||
| exception_ptr::~exception_ptr() noexcept { __decrement_refcount(__ptr_); } | ||
|
|
||
| exception_ptr::exception_ptr(const exception_ptr& other) noexcept : __ptr_(other.__ptr_) { | ||
| __increment_refcount(__ptr_); | ||
| } | ||
|
|
||
| exception_ptr& exception_ptr::operator=(const exception_ptr& other) noexcept { | ||
| if (__ptr_ != other.__ptr_) { | ||
| __increment_refcount(other.__ptr_); | ||
| __decrement_refcount(__ptr_); | ||
| __ptr_ = other.__ptr_; | ||
| } | ||
| return *this; | ||
| } | ||
|
|
||
| } // namespace std |
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.
You'll probably need to add availability markups for Apple platforms.
Uh oh!
There was an error while loading. Please reload this page.
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.
I think in this particular case we don't need availability markup.
Afaict, availability markup is needed for newly introduced symbols.
However, I am not introducing a new symbol. I am turning an existing symbol into a no-longer used symbol.
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.
You're introducing
__increment_refcountand__decrement_refcount, so we need to provide some other implementation if they're not available. Using the old implementation seems like a rather obvious solution to me.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.
🤦 of course...