[libcxx-commits] [PATCH] D124516: [libc++] Implement `std::expected` P0323R12

[Hui via Phabricator via libcxx-commits]

huixie90 added inline comments.


================
Comment at: libcxx/include/__expected/expected.h:310
+
+#  if __cpp_concepts >= 202002
+  _LIBCPP_HIDE_FROM_ABI constexpr ~expected()
----------------
ldionne wrote:
> Which compiler(s) do not satisfy that?
clang 14 does not support multiple destructors


================
Comment at: libcxx/include/__expected/expected.h:612-616
+  bool __has_val_;
+  union {
+    _Tp __val_;
+    _Err __unex_;
+  };
----------------
ldionne wrote:
> I would like to suggest swapping the union and the `bool` here. I think that will result in a more space efficient layout in most cases. You could confirm and change, or infirm and keep.
I tried several examples and I could not find an example that the order of these two member affects the size of the class.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124516/new/

https://reviews.llvm.org/D124516