[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 10:10:23 PST 2021
zoecarver added a comment.
More comments later :)
Comment at: libcxx/include/tuple:459
+ // [tuple.cnstr]
The standard has about 100 ways of abbreviating the word "constructor".
Comment at: libcxx/include/tuple:462
+ // tuple() constructors (including allocator_arg_t variants)
+ template <class _Dep = true_type, _EnableIf<
I assume the `_Dep` is just a "dummy type" for the `_EnableIf`? If so, maybe we could keep calling it `_Dummy`, because we use that name in a lot of places in the codebase to indicate that the type doesn't matter, or is just used for a following `_EnableIf`. Right now, when I look at this, I'm wondering if it was changed to add a "dependency" to this constructor (which I don't think is the case).
Additionally, if you change this to say `class _T2 = _Tp` or `class _Dummy = _Tp` you can omit the `_And`, I think. (Note: here it doesn't really matter because you'd go back to using `__all` but below I think you can eliminate some extra template logic, which is good for both readability and compile time.)
Comment at: libcxx/include/tuple:466
+ ,int> = 0>
(Same comment on a few of these ctors.)
Comment at: libcxx/include/tuple:489
- tuple(_AllocArgT, _Alloc const& __a)
+ tuple(allocator_arg_t, _Alloc const& __a)
: __base_(allocator_arg_t(), __a,
This constructor could be `_NOEXCEPT_(_And<is_nothrow_default_constructible<_Tp>...>::value)`, right?
I think there are a couple of missing noexcepts here, but I think that's better done as a follow-up patch, to keep things separated. I am happy to work on this, after this patch lands. (Also, I won't point it out again, because it's not really relevant to this patch.)
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
More information about the libcxx-commits