[libcxx-commits] [libcxx] [libc++] Ensure that `std::expected` has no tail padding (PR #69673)

Jan Kokemüller via libcxx-commits libcxx-commits at lists.llvm.org
Fri Oct 27 11:42:28 PDT 2023


================
@@ -88,8 +88,263 @@ _LIBCPP_HIDE_FROM_ABI void __throw_bad_expected_access(_Arg&& __arg) {
 #  endif
 }
 
+struct __expected_invoke_tag {};
+
+template <bool _NoUnique, class _Tp>
+class __expected_conditional_no_unique_address {
+  struct __unique {
+    template <class... _Args>
+    _LIBCPP_HIDE_FROM_ABI constexpr explicit __unique(_Args&&... __args)
+        : __v(std::forward<_Args>(__args)...) {}
+
+    template <class _Func, class... _Args>
+    _LIBCPP_HIDE_FROM_ABI constexpr explicit __unique(
+        __expected_invoke_tag, _Func&& __f, _Args&&... __args)
+        : __v(std::invoke(std::forward<_Func>(__f), std::forward<_Args>(__args)...)) {}
+
+    _Tp __v;
+  };
+
+  struct __no_unique {
+    template <class... _Args>
+    _LIBCPP_HIDE_FROM_ABI constexpr explicit __no_unique(_Args&&... __args)
+        : __v(std::forward<_Args>(__args)...) {}
+
+    template <class _Func, class... _Args>
+    _LIBCPP_HIDE_FROM_ABI constexpr explicit __no_unique(
+        __expected_invoke_tag, _Func&& __f, _Args&&... __args)
+        : __v(std::invoke(std::forward<_Func>(__f), std::forward<_Args>(__args)...)) {}
+
+    _LIBCPP_NO_UNIQUE_ADDRESS _Tp __v;
+  };
+
+public:
+  using __type = std::conditional<_NoUnique, __no_unique, __unique>::type;
+};
+
+template <class _Union>
+_LIBCPP_HIDE_FROM_ABI constexpr bool __expected_can_stuff_tail()
+{
+  struct __x {
+  private:
+    _LIBCPP_NO_UNIQUE_ADDRESS _Union __union_;
+    _LIBCPP_NO_UNIQUE_ADDRESS bool __has_val_;
+  };
+  return sizeof(__x) == sizeof(_Union);
+}
+
+template <class _Tp, class _Err>
+class __expected_base {
+  // use named union because [[no_unique_address]] cannot be applied to an unnamed union,
+  // also guaranteed elision into a potentially-overlapping subobject is unsettled (and
+  // it's not clear that it's implementable, given that the function is allowed to clobber
+  // the tail padding) - see https://github.com/itanium-cxx-abi/cxx-abi/issues/107.
+  union __union_t {
+    _LIBCPP_HIDE_FROM_ABI constexpr __union_t(const __union_t&) = delete;
+    _LIBCPP_HIDE_FROM_ABI constexpr __union_t(const __union_t&)
+      requires(is_copy_constructible_v<_Tp> && is_copy_constructible_v<_Err> &&
+               is_trivially_copy_constructible_v<_Tp> && is_trivially_copy_constructible_v<_Err>)
+    = default;
+    _LIBCPP_HIDE_FROM_ABI constexpr __union_t(__union_t&&) = delete;
+    _LIBCPP_HIDE_FROM_ABI constexpr __union_t(__union_t&&)
+      requires(is_move_constructible_v<_Tp> && is_move_constructible_v<_Err> &&
+               is_trivially_move_constructible_v<_Tp> && is_trivially_move_constructible_v<_Err>)
+    = default;
+    _LIBCPP_HIDE_FROM_ABI constexpr __union_t& operator=(const __union_t&) = delete;
+    _LIBCPP_HIDE_FROM_ABI constexpr __union_t& operator=(__union_t&&) = delete;
+
+    template <class... _Args>
+    _LIBCPP_HIDE_FROM_ABI constexpr explicit __union_t(in_place_t, _Args&&... __args)
+        : __val_(std::forward<_Args>(__args)...) {}
+
+    template <class... _Args>
+    _LIBCPP_HIDE_FROM_ABI constexpr explicit __union_t(unexpect_t, _Args&&... __args)
+        : __unex_(std::forward<_Args>(__args)...) {}
+
+    template <class _Func, class... _Args>
+    _LIBCPP_HIDE_FROM_ABI constexpr explicit __union_t(
+        std::__expected_construct_in_place_from_invoke_tag, _Func&& __f, _Args&&... __args)
+        : __val_(std::invoke(std::forward<_Func>(__f), std::forward<_Args>(__args)...)) {}
+
+    template <class _Func, class... _Args>
+    _LIBCPP_HIDE_FROM_ABI constexpr explicit __union_t(
+        std::__expected_construct_unexpected_from_invoke_tag, _Func&& __f, _Args&&... __args)
+        : __unex_(std::invoke(std::forward<_Func>(__f), std::forward<_Args>(__args)...)) {}
+
+    _LIBCPP_HIDE_FROM_ABI constexpr ~__union_t()
+      requires(is_trivially_destructible_v<_Tp> && is_trivially_destructible_v<_Err>)
+    = default;
+
+    // __repr's destructor handles this
+    _LIBCPP_HIDE_FROM_ABI constexpr ~__union_t() {}
+
+    _LIBCPP_NO_UNIQUE_ADDRESS _Tp __val_;
+    _LIBCPP_NO_UNIQUE_ADDRESS _Err __unex_;
+  };
+
+  static constexpr bool __can_stuff_tail = __expected_can_stuff_tail<__union_t>();
+
+  struct __repr {
+    _LIBCPP_HIDE_FROM_ABI constexpr explicit __repr() = delete;
+
+    template <class... _Args>
+    _LIBCPP_HIDE_FROM_ABI constexpr explicit __repr(in_place_t __tag, _Args&&... __args)
+        : __union_(__tag, std::forward<_Args>(__args)...), __has_val_(true) {}
+
+    template <class... _Args>
+    _LIBCPP_HIDE_FROM_ABI constexpr explicit __repr(unexpect_t __tag, _Args&&... __args)
+        : __union_(__tag, std::forward<_Args>(__args)...), __has_val_(false) {}
+
+    template <class... _Args>
+    _LIBCPP_HIDE_FROM_ABI constexpr explicit __repr(
+        std::__expected_construct_in_place_from_invoke_tag __tag, _Args&&... __args)
+        : __union_(__tag, std::forward<_Args>(__args)...), __has_val_(true) {}
+
+    template <class... _Args>
+    _LIBCPP_HIDE_FROM_ABI constexpr explicit __repr(
+        std::__expected_construct_unexpected_from_invoke_tag __tag, _Args&&... __args)
+        : __union_(__tag, std::forward<_Args>(__args)...), __has_val_(false) {}
+
+    template <class _OtherUnion>
+    _LIBCPP_HIDE_FROM_ABI constexpr explicit __repr(bool __has_val, _OtherUnion&& __other)
+      requires(!__can_stuff_tail)
+        : __union_(__expected_invoke_tag{},
+              [&] { return __make_union(__has_val, std::forward<_OtherUnion>(__other)); }),
+          __has_val_(__has_val) {}
+
+    _LIBCPP_HIDE_FROM_ABI constexpr __repr(const __repr&) = delete;
+    _LIBCPP_HIDE_FROM_ABI constexpr __repr(const __repr&)
+      requires(is_copy_constructible_v<_Tp> && is_copy_constructible_v<_Err> &&
+               is_trivially_copy_constructible_v<_Tp> && is_trivially_copy_constructible_v<_Err>)
+    = default;
+    _LIBCPP_HIDE_FROM_ABI constexpr __repr(__repr&&) = delete;
+    _LIBCPP_HIDE_FROM_ABI constexpr __repr(__repr&&)
+      requires(is_move_constructible_v<_Tp> && is_move_constructible_v<_Err> &&
+               is_trivially_move_constructible_v<_Tp> && is_trivially_move_constructible_v<_Err>)
+    = default;
+
+    _LIBCPP_HIDE_FROM_ABI constexpr __repr& operator=(const __repr&) = delete;
+    _LIBCPP_HIDE_FROM_ABI constexpr __repr& operator=(__repr&&) = delete;
+
+    _LIBCPP_HIDE_FROM_ABI constexpr ~__repr()
+      requires(is_trivially_destructible_v<_Tp> && is_trivially_destructible_v<_Err>)
+    = default;
+
+    _LIBCPP_HIDE_FROM_ABI constexpr ~__repr()
+      requires(!is_trivially_destructible_v<_Tp> || !is_trivially_destructible_v<_Err>)
+    {
+      __destroy_union_member();
+    }
+
+    _LIBCPP_HIDE_FROM_ABI constexpr void __destroy_union()
+      requires(!__can_stuff_tail && (is_trivially_destructible_v<_Tp> && is_trivially_destructible_v<_Err>))
+    {
+      std::destroy_at(&__union_.__v);
+    }
+
+    _LIBCPP_HIDE_FROM_ABI constexpr void __destroy_union()
+      requires(!__can_stuff_tail && (!is_trivially_destructible_v<_Tp> || !is_trivially_destructible_v<_Err>))
+    {
+      __destroy_union_member();
+      std::destroy_at(&__union_.__v);
+    }
+
+    template <class... _Args>
+    _LIBCPP_HIDE_FROM_ABI constexpr void __construct_union(in_place_t, _Args&&... __args)
+      requires(!__can_stuff_tail)
+    {
+      std::construct_at(&__union_.__v, in_place, std::forward<_Args>(__args)...);
+      __has_val_ = true;
+    }
+
+    template <class... _Args>
+    _LIBCPP_HIDE_FROM_ABI constexpr void __construct_union(unexpect_t, _Args&&... __args)
+      requires(!__can_stuff_tail)
+    {
+      std::construct_at(&__union_.__v, unexpect, std::forward<_Args>(__args)...);
+      __has_val_ = false;
+    }
+
+  private:
+    template <class, class>
+    friend class __expected_base;
+
+    _LIBCPP_HIDE_FROM_ABI constexpr void __destroy_union_member()
+      requires(!is_trivially_destructible_v<_Tp> || !is_trivially_destructible_v<_Err>)
+    {
+      if (__has_val_) {
+        std::destroy_at(std::addressof(__union_.__v.__val_));
+      } else {
+        std::destroy_at(std::addressof(__union_.__v.__unex_));
+      }
+    }
+
+    template <class _OtherUnion>
+    _LIBCPP_HIDE_FROM_ABI static constexpr __union_t __make_union(bool __has_val, _OtherUnion&& __other)
+      requires(!__can_stuff_tail)
+    {
+      if (__has_val)
+        return __union_t(in_place, std::forward<_OtherUnion>(__other).__val_);
+      else
+        return __union_t(unexpect, std::forward<_OtherUnion>(__other).__unex_);
+    }
+
+    _LIBCPP_NO_UNIQUE_ADDRESS __expected_conditional_no_unique_address<
+        __can_stuff_tail, __union_t>::__type __union_;
+    _LIBCPP_NO_UNIQUE_ADDRESS bool __has_val_;
+  };
+
+  template <class _OtherUnion>
+  _LIBCPP_HIDE_FROM_ABI static constexpr __repr __make_repr(bool __has_val, _OtherUnion&& __other)
+    requires(__can_stuff_tail)
+  {
+    if (__has_val)
+      return __repr(in_place, std::forward<_OtherUnion>(__other).__val_);
+    else
+      return __repr(unexpect, std::forward<_OtherUnion>(__other).__unex_);
+  }
+
+protected:
+  template <class... _Args>
+  _LIBCPP_HIDE_FROM_ABI constexpr explicit __expected_base(_Args&&... __args)
+      : __repr_(std::forward<_Args>(__args)...) {}
+
+  template <class _OtherUnion>
+  _LIBCPP_HIDE_FROM_ABI constexpr explicit __expected_base(bool __has_val, _OtherUnion&& __other)
+    requires(__can_stuff_tail)
+      : __repr_(__expected_invoke_tag{},
+            [&] { return __make_repr(__has_val, std::forward<_OtherUnion>(__other)); }) {}
+
+  _LIBCPP_HIDE_FROM_ABI constexpr void __destroy() {
+    if constexpr (__can_stuff_tail)
+      std::destroy_at(&__repr_.__v);
+    else
+      __repr_.__v.__destroy_union();
+  }
+
+  template <class _Tag, class... _Args>
+  _LIBCPP_HIDE_FROM_ABI constexpr void __construct(_Tag __tag, _Args&&... __args) {
+    if constexpr (__can_stuff_tail)
+      std::construct_at(&__repr_.__v, __tag, std::forward<_Args>(__args)...);
+    else
+      __repr_.__v.__construct_union(__tag, std::forward<_Args>(__args)...);
+  }
+
+  _LIBCPP_HIDE_FROM_ABI constexpr bool __has_val() const { return __repr_.__v.__has_val_; }
+  _LIBCPP_HIDE_FROM_ABI constexpr __union_t& __union() { return __repr_.__v.__union_.__v; }
+  _LIBCPP_HIDE_FROM_ABI constexpr const __union_t& __union() const { return __repr_.__v.__union_.__v; }
+  _LIBCPP_HIDE_FROM_ABI constexpr _Tp& __val() { return __repr_.__v.__union_.__v.__val_; }
+  _LIBCPP_HIDE_FROM_ABI constexpr const _Tp& __val() const { return __repr_.__v.__union_.__v.__val_; }
+  _LIBCPP_HIDE_FROM_ABI constexpr _Err& __unex() { return __repr_.__v.__union_.__v.__unex_; }
+  _LIBCPP_HIDE_FROM_ABI constexpr const _Err& __unex() const { return __repr_.__v.__union_.__v.__unex_; }
+
+private:
+  _LIBCPP_NO_UNIQUE_ADDRESS __expected_conditional_no_unique_address<
+      !__can_stuff_tail, __repr>::__type __repr_;
+};
+
 template <class _Tp, class _Err>
-class expected {
+class expected : private __expected_base<_Tp, _Err> {
----------------
jiixyj wrote:

I actually tried to do without `__expected_base`, but I found no elegant way to choose between `__make_repr` and `__make_union`. In case the whole `repr` needs to be constructed, there is a constructor in `__expected_base` that "catches" this case and calls `__make_repr`:

https://github.com/jiixyj/llvm-project/blob/237188e69467f956e9f75ffd4fb5edca1cf6359c/libcxx/include/__expected/expected.h#L307-L317

Otherwise, the arguments will be forwarded to `repr`'s constructor, where `__make_union` is called:

https://github.com/jiixyj/llvm-project/blob/237188e69467f956e9f75ffd4fb5edca1cf6359c/libcxx/include/__expected/expected.h#L208-L213

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


More information about the libcxx-commits mailing list