[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<
+        _And<_Dep,
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
+        >::value
+    ,int> = 0>
Nit: space. 

(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



More information about the libcxx-commits mailing list