[PATCH] D32515: [libcxx] [test] Changes to accommodate LWG 2904 "Make variant move-assignment more exception safe"
mpark added a comment. Thanks for the tests! I'll try this out with an implementation shortly. https://reviews.llvm.org/D32515 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D32671: [libcxx] [test] variant: test coverage for P0602 extension
mpark added a comment. @CaseyCarter: Does this not pass with the current version? Also, have you seen the tests in `test/libcxx/utilities/variant/variant.variant`? If yes, do those tests and the ones in this diff overlap at all? Curious as to how we should merge them. https://reviews.llvm.org/D32671 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D32671: [libcxx] [test] variant: test coverage for P0602 extension
mpark added a comment. Yes, you're right that fine-grained SMF triviality is implemented but LWG 2904 is not yet. I would love it if you can integrate the currently `libcxx` tests into `std` tests! Thank you :) https://reviews.llvm.org/D32671 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40210: [libc++] Shrink variant's index type when possible
mpark added inline comments.
Comment at: include/variant:295
+
+template
+using __variant_index_t =
`s/_NumElem/_NumAlts/`
Comment at: include/variant:300-303
+ std::tuple_element_t<
+ __choose_index_type(_NumElem),
+ std::tuple
+ >;
Could we inline the whole thing and also avoid using the `tuple` utilities?
We should also add the `#include `
```
conditional_t<
(_NumElem < numeric_limits::max()),
unsigned char,
conditional_t<(_NumElem < numeric_limits::max()),
unsigned short,
unsigned int>;
```
Comment at: include/variant:306
+
+template
+constexpr _IntType __variant_npos = static_cast<_IntType>(-1);
`s/_IntType/_IndexType/` maybe?
Comment at:
test/libcxx/utilities/variant/variant.variant/variant_size.pass.cpp:17
+
+#include
+#include
Doesn't seem like this is used?
Comment at:
test/libcxx/utilities/variant/variant.variant/variant_size.pass.cpp:26
+
+template
+struct make_variant_imp> {
Hm, I see 15 instances of `Indices` and 7 instances of `Indexes`.
Can we use `Indices` everywhere?
Comment at:
test/libcxx/utilities/variant/variant.variant/variant_size.pass.cpp:52-53
+ using Lim = std::numeric_limits;
+ static_assert(std::__choose_index_type(Lim::max() -1) !=
+std::__choose_index_type(Lim::max()), "");
+ static_assert(std::is_same_v<
I guess I asked you to remove this above. Is this an essential part of the test?
https://reviews.llvm.org/D40210
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28222: [libcxx] Re-implement LWG 2770 again: Fix tuple_size to work with structured bindings
mpark accepted this revision. mpark added a reviewer: mpark. mpark added a comment. That was fun =D https://reviews.llvm.org/D28222 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D27606: [libcxx] Fix tuple construction/assignment from types derived from tuple/pair/array.
mpark added inline comments.
Comment at: include/tuple:568
+using _QualType = typename _Deduced::_QualType;
+ static_assert(__tuple_like::value, "");
+return __tuple_constructible<_QualType, tuple>::value
indentation?
Comment at: include/tuple:871
template ,
+ class _TupBase = typename _Deduced::_QualType,
indentation?
Comment at: include/tuple:906
template ,
+ class _QualTupleBase = typename _Deducer::_QualType,
`s/_Deducer/_Deduced/`
Comment at: include/tuple:907
+ class _Deducer = __deduce_tuple_like<_Tuple>,
+ class _QualTupleBase = typename _Deducer::_QualType,
class = typename enable_if
`s/_QualTupleBase/_TupBase/`
Comment at: include/tuple:915
tuple&
-operator=(_Tuple&& __t) _NOEXCEPT_((is_nothrow_assignable::value))
+operator=(_Tuple&& __t) _NOEXCEPT_((is_nothrow_assignable::value))
{
A general comment about using the `base` in noexcept condition. I remember for
`variant` we wanted to express it directly rather than delegating it to the
"base". Does that also apply here?
Comment at: include/tuple:917
{
-base_.operator=(_VSTD::forward<_Tuple>(__t));
+base_.operator=(_VSTD::forward<_QualTupleBase>(__t));
return *this;
Here and elsewhere, why do we bother with `base_.operator=(...);` as opposed to
just `base_ = ...;`?
https://reviews.llvm.org/D27606
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
