CaseyCarter added inline comments.
================
Comment at: test/std/utilities/variant/variant.relops/relops.pass.cpp:60
+inline bool operator>(MakeEmptyT const&, MakeEmptyT const&) { assert(false); }
+inline bool operator>=(MakeEmptyT const&, MakeEmptyT const&) { assert(false); }
+
----------------
MSVC with /W4 is, uh, not found of these functions. Adding "return false;"
after the assert shuts it up.
================
Comment at: test/std/utilities/variant/variant.relops/relops.pass.cpp:189
+ V v2; makeEmpty(v2);
+ assert(test_less(v1, v2, true, false));
+ }
----------------
Bogus test: should be `assert(test_less(v1, v2, false, true));`. Maybe someone
forgot to implement part of P0032?
================
Comment at: test/std/utilities/variant/variant.relops/relops.pass.cpp:195
+ V v2;
+ assert(test_less(v1, v2, false, true));
+ }
----------------
Bogus test again: should be `assert(test_less(v1, v2, true, false));`.
================
Comment at:
test/std/utilities/variant/variant.variant/variant.assign/copy.pass.cpp:149
+ static_assert(!std::is_nothrow_copy_assignable<V>::value, "");
+ }
+}
----------------
All four of these are non-portable. Implementations are permitted to strengthen
noexcept, and these are assignments that can feasibly be implemented to not
throw.
================
Comment at:
test/std/utilities/variant/variant.variant/variant.mod/emplace_index_args.pass.cpp:92
+ int y = 42;
+ int z = 43;
+ V v(std::in_place_index<0>, -1);
----------------
y and z are unused here,
================
Comment at:
test/std/utilities/variant/variant.variant/variant.mod/emplace_type_args.pass.cpp:93
+ int y = 42;
+ int z = 43;
+ V v(std::in_place_type<int>, -1);
----------------
y and z are unused here.
================
Comment at:
test/std/utilities/variant/variant.variant/variant.swap/swap.pass.cpp:321
+ // the variant swap is called on.
+ assert(T::move_called == 1);
+ assert(T::move_assign_called == 0);
----------------
Non-portable. Replace with `LIBCPP_ASSERT(T::move_called == 1);
assert(T::move_called <= 2);`
================
Comment at:
test/std/utilities/variant/variant.variant/variant.swap/swap.pass.cpp:328
+ assert(T::swap_called == 0);
+ assert(T::move_called == 2);
+ assert(T::move_assign_called == 0);
----------------
Non-portable. Replace with `LIBCPP_ASSERT(T::move_called == 2);
assert(T::move_called <= 2);`
================
Comment at:
test/std/utilities/variant/variant.variant/variant.swap/swap.pass.cpp:349
+ assert(T1::move_called == 1); // throws
+ assert(T1::move_assign_called == 0);
+ assert(T2::move_called == 1);
----------------
Extra space before `==` here and on 370.
================
Comment at:
test/std/utilities/variant/variant.variant/variant.swap/swap.pass.cpp:350
+ assert(T1::move_assign_called == 0);
+ assert(T2::move_called == 1);
+ assert(std::get<0>(v1).value == 42);
----------------
Non-portable. Replace with `LIBCPP_ASSERT(T2::move_called == 1);
assert(T2::move_called <= 1);`
================
Comment at:
test/std/utilities/variant/variant.variant/variant.swap/swap.pass.cpp:352
+ assert(std::get<0>(v1).value == 42);
+ assert(v2.valueless_by_exception());
+ }
----------------
non-portable. s/b
```
if (T2::move_called != 0)
assert(v2.valueless_by_exception());
else
assert(std::get<1>(v2) == 100);
```
================
Comment at:
test/std/utilities/variant/variant.variant/variant.swap/swap.pass.cpp:367
+ }
+ assert(T2::move_called == 1);
+ assert(T2::swap_called == 0);
----------------
This line is a dup of 369. `T1::move_called` was probably intended, in which
case it is non-portable and should be `LIBCPP_ASSERT(T1::move_called == 1);
assert(T1::move_called <= 1);`
================
Comment at:
test/std/utilities/variant/variant.variant/variant.swap/swap.pass.cpp:371
+ assert(T2::move_assign_called == 0);
+ assert(std::get<0>(v1).value == 42);
+ assert(std::get<1>(v2).value == 100);
----------------
Non-portable. S/b:
```
if (T1::move_called != 0)
assert(v1.valueless_by_exception());
else
assert(std::get<0>(v1).value == 42);
```
================
Comment at:
test/std/utilities/variant/variant.variant/variant.swap/swap.pass.cpp:375
+ {
+ using T1 = ThrowsOnSecondMove;
+ using T2 = NonThrowingNonNoexceptType;
----------------
This test and the next are non-portable and I can't be bothered to fix them.
================
Comment at:
test/std/utilities/variant/variant.variant/variant.swap/swap.pass.cpp:403
+ {
+ // testing libc++ extension. If either variant stores a nothrow move
+ // constructible type v1.swap(v2) provides the strong exception safety
----------------
I shouldn't have to point out that a test with the comment "testing libc++
extension" is non-portable ;)
================
Comment at:
test/std/utilities/variant/variant.variant/variant.swap/swap.pass.cpp:405
+ // constructible type v1.swap(v2) provides the strong exception safety
+ // guarentee.
+ using T1 = ThrowingTypeWithNothrowSwap;
----------------
guarantee
================
Comment at:
test/std/utilities/variant/variant.variant/variant.swap/swap.pass.cpp:544
+ using V = std::variant<int, NotSwappable>;
+ static_assert(!has_swap_member<V>());
+ static_assert(std::is_swappable_v<V>, "");
----------------
This should be `LIBCPP_STATIC_ASSERT` (SFINAE isn't mandated for member swap
after LWG2749).
================
Comment at: test/std/utilities/variant/variant.visit/visit.pass.cpp:42
+ template <class ...Args>
+ bool operator()(Args&&... args) & {
+ set_call<Args&&...>(CT_NonConst | CT_LValue);
----------------
MSVC complains that all four of these operator() overloads name but do not use
`args`; removing the name shuts up the warning.
https://reviews.llvm.org/D26903
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
