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

[Eric Fiselier via Phabricator via libcxx-commits]

EricWF added a comment.

Other than a couple breakages caused by this not working in C++14/C++11, I haven't seen anything blow up at Google, though I've yet to get a full test run in.
That is to say, I think this is good to land once the other inline comments are addressed.



================
Comment at: libcxx/include/tuple:754
             _Lazy<_And,
-                _Not<is_convertible<const tuple<_Up>&, _Tp> >...,
-                _Not<is_constructible<_Tp, const tuple<_Up>&> >...
+                  _Not<is_convertible<__maybe_const<_Const, tuple<_Up>>&, _Tp> >...,
+                  _Not<is_constructible<_Tp, __maybe_const<_Const, tuple<_Up>>&> >...
----------------
EricWF wrote:
> EricWF wrote:
> > What's `__maybe_const`? Should it be in this patch?
> Ignore this.
Actually, nevermind. `__maybe_const` is only available after C++17, but this compiles in older dialects. So we need to expose `__maybe_const` in older dialects.


================
Comment at: libcxx/include/type_traits:471
+struct __nat
+{
+#ifndef _LIBCPP_CXX03_LANG
----------------
I


================
Comment at: libcxx/include/type_traits:506-509
+  template <class...>
+  using _FirstImpl = __nat;
+  template <class...>
+  using _SecondImpl = __nat;
----------------
philnik wrote:
> EricWF wrote:
> > philnik wrote:
> > > Quuxplusone wrote:
> > > > Uh...? This completely changes the meaning of e.g. `_SecondType<int>` from "ill-formed" to "well-formed `__nat`." There's no way that's safe.
> > > `_FirstType` and `_SecondType` are exclusively used in `<tuple>` and only in `is_*` contexts. If I'm not mistaken `__nat` should be false in every single case.
> > I'm with Arthur, this should not succeed. This needs to be substitution failure.
> In which case does it have to be a substitution failure? IIRC there are cases where I need it to //not// be a substitution failure.
It doesn't need to be substitution failure, but substitution failure is 1000x better than generating a bogus type and continuing on.
I'm not OK with this change. 

In what cases are you trying to apply first/second type to a pack that doesn't have them?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116621