[libcxx-commits] [PATCH] D116621: [libc++][P2321R2] Add const overloads to tuple swap, construction and assignment

[Eric Fiselier via Phabricator via libcxx-commits]

EricWF added inline comments.


================
Comment at: libcxx/include/tuple:1115
+    _LIBCPP_HIDE_FROM_ABI constexpr
+    const tuple& operator=(_If<_And<is_assignable<_Tp&&, _Tp&&>...>::value, tuple, __nat>&& __tuple) const {
+        std::__memberwise_forward_assign(*this,
----------------
I don't know that we need to do the `_If` trick here. Because it's not the true "copy assignment operator", we don't need to worry about the compiler generating one.

Try writing these like normal overloads with normal SFINAE.


================
Comment at: libcxx/include/type_traits:471
+struct __nat
+{
+#ifndef _LIBCPP_CXX03_LANG
----------------
EricWF wrote:
> I
I'm still against this. That's not how these meta-functions are meant to work. Also


================
Comment at: libcxx/include/type_traits:4098
 template<bool _Const, class _Tp>
-using __maybe_const = conditional_t<_Const, const _Tp, _Tp>;
-#endif // _LIBCPP_STD_VER > 17
+using __maybe_const = typename conditional<_Const, const _Tp, _Tp>::type;
 
----------------
Use `_If` here to avoid adding more template instantiations. 


================
Comment at: libcxx/test/std/utilities/tuple/tuple.tuple/tuple.assign/convert_cpp23.pass.cpp:16
+// UNSUPPORTED: c++03, c++11, c++14, c++17, c++20
+// XFAIL: gcc
+
----------------
ldionne wrote:
> Why does it fail on GCC?
Oh, good catch.

I suspect it's because GCC rejects mutating mutable members in constant expressions.

Since we want *some* test coverage on GCC, I suggest we find a way to make at least part of this test pass under it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116621