[libcxx-commits] [PATCH] D131495: [libc++] implement "pair" section of P2321R2 `zip`

[Konstantin Varlamov via Phabricator via libcxx-commits]

var-const accepted this revision as: var-const.
var-const added a comment.

Thanks a lot for working on this! LGTM, but I'll leave the final approval to @ldionne.



================
Comment at: libcxx/include/__utility/pair.h:211
+    explicit(!_CheckArgs::template __is_implicit<_U1&, _U2&>()) pair(pair<_U1, _U2>& __p)
+        noexcept((is_nothrow_constructible<first_type, _U1&>::value &&
+                  is_nothrow_constructible<second_type, _U2&>::value))
----------------
huixie90 wrote:
> var-const wrote:
> > The `noexcept` doesn't seem to be in the standard -- we're adding it for consistency with existing constructors, right?
> that is right. None of the overload in the standard has `noexcept` and we added `noexcept` to the other overloads. I am just trying to be consistent with what we did in the past. Do you think it is reasonable?
I think it's the right thing to do, but please double-check with @ldionne.


================
Comment at: libcxx/include/__utility/pair.h:514
+void swap(const pair<_T1, _T2>& __x, const pair<_T1, _T2>& __y)
+    _NOEXCEPT_(__is_nothrow_swappable<const _T1>::value &&
+               __is_nothrow_swappable<const _T2>::value)
----------------
huixie90 wrote:
> var-const wrote:
> > The `noexcept` specification is different from the standard -- looks like it's copied from the member version.
> The standard is `noexcept(noexcept(x.swap(y)));` which is equivalent to the one here. The reason I wrote this way is to be consistent with what we did in the past (see the overload above this one). do you think we should change it?
I'd rather use the same specification as the standard unless there's a reason to diverge (it saves the mental burden of thinking whether our specification is equivalent).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131495