[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
Sat Jan 20 03:15:48 PST 2024
================
@@ -88,8 +88,353 @@ _LIBCPP_HIDE_FROM_ABI void __throw_bad_expected_access(_Arg&& __arg) {
# endif
}
+struct __conditional_no_unique_address_invoke_tag {};
+
+// This class implements an object with `[[no_unique_address]]` conditionally applied to it,
+// based on the value of `_NoUnique`.
+//
+// A member of this class must always have `[[no_unique_address]]` applied to
+// it. Otherwise, the `[[no_unique_address]]` in the "`_NoUnique == true`" case
+// would not have any effect. In the `false` case, the `__v` is not
+// `[[no_unique_address]]`, so nullifies the effects of the "outer"
+// `[[no_unique_address]]` regarding data layout.
+//
+// If we had a language feature, this class would basically be replaced by `[[no_unique_address(condition)]]`.
+template <bool _NoUnique, class _Tp>
+struct __conditional_no_unique_address;
+
+template <class _Tp>
+struct __conditional_no_unique_address<true, _Tp> {
+ template <class... _Args>
+ _LIBCPP_HIDE_FROM_ABI constexpr explicit __conditional_no_unique_address(in_place_t, _Args&&... __args)
+ : __v(std::forward<_Args>(__args)...) {}
+
+ template <class _Func, class... _Args>
+ _LIBCPP_HIDE_FROM_ABI constexpr explicit __conditional_no_unique_address(
+ __conditional_no_unique_address_invoke_tag, _Func&& __f, _Args&&... __args)
+ : __v(std::invoke(std::forward<_Func>(__f), std::forward<_Args>(__args)...)) {}
+
+ _LIBCPP_NO_UNIQUE_ADDRESS _Tp __v;
+};
+
+template <class _Tp>
+struct __conditional_no_unique_address<false, _Tp> {
+ template <class... _Args>
+ _LIBCPP_HIDE_FROM_ABI constexpr explicit __conditional_no_unique_address(in_place_t, _Args&&... __args)
+ : __v(std::forward<_Args>(__args)...) {}
+
+ template <class _Func, class... _Args>
+ _LIBCPP_HIDE_FROM_ABI constexpr explicit __conditional_no_unique_address(
+ __conditional_no_unique_address_invoke_tag, _Func&& __f, _Args&&... __args)
+ : __v(std::invoke(std::forward<_Func>(__f), std::forward<_Args>(__args)...)) {}
+
+ _Tp __v;
+};
+
+// This function returns whether the type `_Second` can be stuffed into the tail padding
+// of the `_First` type if both of them are given `[[no_unique_address]]`.
+template <class _First, class _Second>
+inline constexpr bool __fits_in_tail_padding = []() {
+ struct __x {
+ _LIBCPP_NO_UNIQUE_ADDRESS _First __first;
+ _LIBCPP_NO_UNIQUE_ADDRESS _Second __second;
+ };
+ return sizeof(__x) == sizeof(_First);
+}();
+
+// This class implements the storage used by `std::expected`. We have a few
+// goals for this storage:
+// 1. Whenever the underlying {_Tp | _Unex} combination has free bytes in its
+// tail padding, we should reuse it to store the bool discriminator of the
+// expected, so as to save space.
+// 2. Whenever the `expected<_Tp, _Unex>` as a whole has free bytes in its tail
+// padding, we should allow an object following the expected to be stored in
+// its tail padding.
+// 3. However, we never want a user object (say `X`) that would follow an
+// `expected<_Tp, _Unex>` to be stored in the padding bytes of the
+// underlying {_Tp | _Unex} union, if any. That is because we use
+// `construct_at` on that union, which would end up overwriting the `X`
+// member if it is stored in the tail padding of the union.
+//
+// To achieve this, `__expected_base`'s logic is implemented in an inner
+// `__repr` class. `__expected_base` holds one `__repr` member which is
+// conditionally `[[no_unique_address]]`. The `__repr` class holds the
+// underlying {_Tp | _Unex} union and a boolean "has value" flag.
+//
+// Which one of the `__repr_`/`__union_` members is `[[no_unique_address]]`
+// depends on whether the "has value" boolean fits into the tail padding of
+// the underlying {_Tp | _Unex} union:
+//
+// - In case the "has value" bool fits into the tail padding of the union, the
+// whole `__repr_` member is _not_ `[[no_unique_address]]` as it needs to be
+// transparently replaced on `emplace()`/`swap()` etc.
+// - In case the "has value" bool does not fit into the tail padding of the
+// union, only the union member must be transparently replaced (therefore is
+// _not_ `[[no_unique_address]]`) and the "has value" flag must be adjusted
+// manually.
+//
+// This way, the member that is transparently replaced on mutating operations
+// is never `[[no_unique_address]]`, satisfying the requirements from
+// "[basic.life]" in the standard.
+//
+// Stripped away of all superfluous elements, the layout of `__expected_base`
+// then looks like this:
+//
+// template <class Tp, class Err>
+// class expected_base {
+// union union_t {
+// [[no_unique_address]] Tp val;
+// [[no_unique_address]] Err unex;
+// };
+//
+// struct repr {
+// private:
+// // If "has value" fits into the tail, this should be
+// // `[[no_unique_address]]`, otherwise not.
+// [[no_unique_address]] conditional_no_unique_address<
+// fits_in_tail_padding<union_t, bool>,
+// union_t>::type union_;
+// [[no_unique_address]] bool has_val_;
+// };
+//
+// protected:
+// // If "has value" fits into the tail, this must _not_ be
+// // `[[no_unique_address]]` so that we fill out the
+// // complete `expected` object.
+// [[no_unique_address]] conditional_no_unique_address<
+// !fits_in_tail_padding<union_t, bool>,
+// repr>::type repr_;
+// };
+//
template <class _Tp, class _Err>
-class expected {
+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 __put_flag_in_tail = __fits_in_tail_padding<__union_t, bool>;
+
+ 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_(in_place, __tag, std::forward<_Args>(__args)...), __has_val_(true) {}
+
+ template <class... _Args>
+ _LIBCPP_HIDE_FROM_ABI constexpr explicit __repr(unexpect_t __tag, _Args&&... __args)
+ : __union_(in_place, __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_(in_place, __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_(in_place, __tag, std::forward<_Args>(__args)...), __has_val_(false) {}
+
+ template <class _OtherUnion>
+ _LIBCPP_HIDE_FROM_ABI constexpr explicit __repr(bool __has_val, _OtherUnion&& __other)
+ requires(!__put_flag_in_tail)
+ : __union_(__conditional_no_unique_address_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(!__put_flag_in_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(!__put_flag_in_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(!__put_flag_in_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(!__put_flag_in_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(!__put_flag_in_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 __conditional_no_unique_address<__put_flag_in_tail, __union_t> __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(__put_flag_in_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_(in_place, std::forward<_Args>(__args)...) {}
+
+ // In case we copy/move construct from another `expected` we need to create
+ // our `expected` so that it either has a value or not, depending on the "has
+ // value" flag of the other `expected`. To do this without falling back on
+ // `std::construct_at` we rely on guaranteed copy elision using two helper
+ // functions `__make_repr` and `__make_union`. There have to be two since
+ // there are two data layouts with different members being
+ // `[[no_unique_address]]`. GCC (as of version 13) does not do guaranteed
+ // copy elision when initializing `[[no_unique_address]]` members. The two
+ // cases are:
+ //
+ // - `__make_repr`: This is used when the "has value" flag lives in the tail
+ // of the union. In this case, the `__repr` member is _not_
+ // `[[no_unique_address]]`.
+ // - `__make_union`: When the "has value" flag does _not_ fit in the tail of
+ // the union, the `__repr` member is `[[no_unique_address]]` and the union
+ // is not.
+ //
+ // This constructor "catches" the first case and leaves the second case to
+ // `__union_t`, its constructors and `__make_union`.
+ template <class _OtherUnion>
+ _LIBCPP_HIDE_FROM_ABI constexpr explicit __expected_base(bool __has_val, _OtherUnion&& __other)
+ requires(__put_flag_in_tail)
+ : __repr_(__conditional_no_unique_address_invoke_tag{},
+ [&] { return __make_repr(__has_val, std::forward<_OtherUnion>(__other)); }) {}
+
+ _LIBCPP_HIDE_FROM_ABI constexpr void __destroy() {
+ if constexpr (__put_flag_in_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 (__put_flag_in_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 __conditional_no_unique_address<!__put_flag_in_tail, __repr> __repr_;
----------------
jiixyj wrote:
Done! I agree, this reads a bit nicer.
https://github.com/llvm/llvm-project/pull/69673
More information about the libcxx-commits
mailing list