[libcxx-commits] [libcxx] [libc++] Inline fast path for`exception_ptr` copy constructor & destructor (PR #165909)

[Adrian Vogelsgesang via libcxx-commits]

================
@@ -82,17 +85,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&) _NOEXCEPT : __ptr_(__other.__ptr_) {
+    if (__ptr_)
+      __increment_refcount(__ptr_);
+  }
+  _LIBCPP_HIDE_FROM_ABI exception_ptr& operator=(const exception_ptr&) _NOEXCEPT {
+    if (__ptr_ != __other.__ptr_) {
+      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;
----------------
vogelsgesang wrote:

Good point.

We have 3 choices:

1. implement both `swap` and the move operator, delegate move to swap (previous solution)
2. implement both `swap` and the move operator independently (currently in review)
3. implement only the move operator, and don't provide a specialization for `swap` 

To me, (3) actually feels most natural. My gut feeling tells me that "move" assignments are more frequently used/specialized than `swap`, so it feels more natural to implement the more common function (move) and delegate the less frequently used (swap) to the other one

I updated the PR accordingly. Please let me know in case you prefer going into a different direction

https://github.com/llvm/llvm-project/pull/165909