[libcxx-commits] [libcxx] [libcxx][string] Allow fancy pointers in basic_string (PR #199264)

[A. Jiang via libcxx-commits]

================
@@ -878,16 +880,97 @@ private:
 
   static_assert(sizeof(__short) == (sizeof(value_type) * (__min_cap + 1)), "__short has an unexpected size.");
 
-  union __rep {
+  template <class _LongT, class = void>
+  union __rep_t {
     __short __s;
-    __long __l;
+    _LongT __l;
 
-    __rep() = default;
-    _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 __rep(__short __r) : __s(__r) {}
-    _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 __rep(__long __r) : __l(__r) {}
-    _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 __rep(__uninitialized_tag) {}
+    _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 void __begin_long_lifetime() _NOEXCEPT {}
+
+    __rep_t() = default;
+    _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 __rep_t(__short __r) _NOEXCEPT : __s(__r) {}
+    _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 __rep_t(_LongT __r) _NOEXCEPT_(noexcept(_LongT(__r)))
+        : __l(__r) {}
+    _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 __rep_t(__uninitialized_tag) _NOEXCEPT {}
   };
 
+  template <class _LongT>
+  union __rep_t<
+      _LongT,
+      __enable_if_t<!is_trivially_default_constructible<_LongT>::value ||
+                    !is_trivially_copy_assignable<_LongT>::value || !is_trivially_destructible<_LongT>::value > > {
+    __short __s;
+    _LongT __l;
+
+    _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 bool __is_long() const _NOEXCEPT {
+      if (__libcpp_is_constant_evaluated() && __builtin_constant_p(__l.__is_long_)) {
+        return __l.__is_long_;
+      }
+      return __s.__is_long_;
+    }
+
+    // If _LongT is not trivially default constructible (this occurs if the pointer type inside _LongT is not trivially
+    // default constructible) then the lifetime of __l must be explicitly started with std::__construct_at before it can
+    // be assigned to. If _LongT is trivially default constructible, then the lifetime of __l can be implicitly started
+    // with a simple assignment and therefore does not need to be explicitly initialised here.
+    _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 void __begin_long_lifetime() _NOEXCEPT {
+      if _LIBCPP_CONSTEXPR (!is_trivially_default_constructible<_LongT>::value) {
+        std::__construct_at(std::addressof(__l));
+      }
+    }
+
+    _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 __rep_t() _NOEXCEPT : __s() {}
+    _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 __rep_t(__short __r) _NOEXCEPT : __s(__r) {}
+    _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 __rep_t(_LongT __r) _NOEXCEPT_(noexcept(_LongT(__r)))
+        : __l(__r) {}
+    _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 __rep_t(__uninitialized_tag) _NOEXCEPT : __s() {}
+    _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 __rep_t(const __rep_t& __other)
+        _NOEXCEPT_(noexcept(__l = __other.__l)) {
+      if (__other.__is_long()) {
+        __begin_long_lifetime();
+        __l = __other.__l;
+      } else
+        __s = __other.__s;
+    }
+    _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 __rep_t(__rep_t&& __other)
+        _NOEXCEPT_(noexcept(__l = std::move(__other.__l))) {
+      if (__other.__is_long()) {
+        __begin_long_lifetime();
+        __l = std::move(__other.__l);
+      } else
+        __s = __other.__s;
+    }
+    _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 __rep_t& operator=(const __rep_t& __other)
+        _NOEXCEPT_(noexcept(__l = __other.__l)) {
+      if (__is_long())
+        __l.~_LongT();
+      if (__other.__is_long()) {
+        __begin_long_lifetime();
+        __l = __other.__l;
+      } else
+        __s = __other.__s;
+      return *this;
+    }
+    _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 __rep_t& operator=(__rep_t&& __other)
+        _NOEXCEPT_(noexcept(__l = std::move(__other.__l))) {
+      if (__is_long())
+        __l.~_LongT();
+      if (__other.__is_long()) {
+        __begin_long_lifetime();
+        __l = std::move(__other.__l);
+      } else
+        __s = __other.__s;
+      return *this;
+    }
+
+    _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 ~__rep_t() _NOEXCEPT_(noexcept(__l.~_LongT())) {
----------------
frederick-vs-ja wrote:

I think we should make this destructor explicitly unconditionally `noexcept`.

```suggestion
    _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 ~__rep_t() noexcept {
```

A hostile fancy pointer type possibly have potentially throwing destructor, but when the destructor actually throws an exception, the behavior is undefined when using the fancy pointer with standard library facilities.

Also, the standard effectively requires the destructor of `basic_string` to always have `noexcept` destructor (per [[res.on.exception.handling]/3](https://eel.is/c++draft/res.on.exception.handling#3)). We should seal the potential-throwing-ness as early as possible, which makes other parts of the implementation possible to rely implicit `noexcept` specification on destructors.

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