[libcxx-commits] [PATCH] D96523: [libc++] Rewrite the tuple constructors to be strictly Standards conforming

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone added inline comments.


================
Comment at: libcxx/include/tuple:917
+        _And<_Dep,
+            _Lazy<_EnableMoveFromPair, _Up1, _Up2>,
+            _Not<_Lazy<_And, // explicit check
----------------
ldionne wrote:
> zoecarver wrote:
> > If you're using `_Up1` here, do you need the `_Dep` template type param?
> Ok, so that's an excellent question, and I would have expected the answer to be "No". In fact, I initially wrote all those without these `_Lazy` calls whenever some arguments were dependent, but I was getting errors saying (basically): "you're expanding parameter packs of different lengths in the definition of the `_EnableMoveFromPair` alias".
> 
> I thought that sort of failure within the definition of an alias template would be considered part of the immediate context for SFINAE and it would result in the overload not being selected. But I was getting a hard error instead. I'm not sure whether I'm doing something subtly wrong, or my expectation about substitution failures in alias templates is wrong, or if it's a compiler bug. I decided not to investigate further because this worked, but I can if you request it.
I think Zoe is right that you could remove `_Dummy`. I have no opinion on the rest of the simplification you tried. But just this should work, right?:

    template <class _Alloc, class _Up1, class _Up2, _EnableIf<
        _And<
            _Lazy<_EnableMoveFromPair, _Up1, _Up2>,
            _Not<_Lazy<_And, // explicit check
                is_convertible<_Up1, _FirstType<_Tp..., void> >,
                is_convertible<_Up2, _SecondType<_Tp..., void> >
            > >
        >::value
    , int> = 0>

(replacing `_Dummy` with `void` in the places IIUC we don't care what it is)
(or maybe you need `SecondType<_Tp..., void, void>`?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96523