[libcxx-commits] [PATCH] D96523: [libc++] Rewrite the tuple constructors to be strictly Standards conforming
Zoe Carver via Phabricator via libcxx-commits
libcxx-commits at lists.llvm.org
Thu Feb 11 22:26:04 PST 2021
zoecarver added inline comments.
Comment at: libcxx/include/tuple:917
+ _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<
> _Lazy<_EnableMoveFromPair, _Up1, _Up2>,
> _Not<_Lazy<_And, // explicit check
> is_convertible<_Up1, _FirstType<_Tp..., void> >,
> is_convertible<_Up2, _SecondType<_Tp..., void> >
> > >
> , 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>`?)
First, why do we need the `void` types? We've already checked that there are at least two elements, and this should be a short-circuiting operator.
Second, I think type alias types should be counted as dependent types as long as they're substituted with template type arguments. So, what Arthur proposed should work. But I can play around with this patch a bit more locally tomorrow and try to get it working.
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
More information about the libcxx-commits