EricWF marked 45 inline comments as done.
EricWF added a comment.
Address almost all inline comments.
================
Comment at: test/std/utilities/variant/variant.get/get_if_index.pass.cpp:92
+ V v(42l);
+ ASSERT_SAME_TYPE(decltype(std::get_if<1>(std::addressof(v))), long*);
+ assert(*std::get_if<1>(std::addressof(v)) == 42);
----------------
STL_MSFT wrote:
> Technically, addressof() lives in <memory>.
I'm just changing it to `&v`.
================
Comment at:
test/std/utilities/variant/variant.helpers/variant_alternative.pass.cpp:33
+void test() {
+ static_assert(std::is_same_v<
+ typename std::variant_alternative<I, V>::type, E>, "");
----------------
STL_MSFT wrote:
> You aren't directly including <type_traits> (not sure if
> "variant_test_helpers.hpp" is).
Fixing this. Does your `<variant>` somehow not drag in `<type_traits>`?
================
Comment at:
test/std/utilities/variant/variant.variant/variant.assign/copy.pass.cpp:28
+struct NoCopy {
+ NoCopy(NoCopy const&) = delete;
+ NoCopy& operator=(NoCopy const&) = default;
----------------
STL_MSFT wrote:
> NoCopy doesn't even have a default ctor, how are you supposed to make one?
> Appears to apply to the classes below.
Don't care about making them. They are only here for testing SFINAE.
================
Comment at:
test/std/utilities/variant/variant.variant/variant.assign/copy.pass.cpp:149
+ static_assert(!std::is_nothrow_copy_assignable<V>::value, "");
+ }
+}
----------------
CaseyCarter wrote:
> 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.
Ack. Ill rewrite these tests so they check situations the STL is not allowed to
strengthen.
================
Comment at:
test/std/utilities/variant/variant.variant/variant.ctor/T.pass.cpp:65
+ }
+#if defined(VARIANT_TEST_REFERENCES)
+ {
----------------
STL_MSFT wrote:
> You have like twenty different macros for enabling references.
I thought I regex replaced them all. I guess not :-(
================
Comment at:
test/std/utilities/variant/variant.variant/variant.ctor/in_place_index_init_list_Args.pass.cpp:1
+// -*- C++ -*-
+//===----------------------------------------------------------------------===//
----------------
STL_MSFT wrote:
> Should this test filename contain capital A?
Doesn't really matter. Making them all lower case for consistency.
================
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>, "");
----------------
CaseyCarter wrote:
> This should be `LIBCPP_STATIC_ASSERT` (SFINAE isn't mandated for member swap
> after LWG2749).
Ack.
https://reviews.llvm.org/D26903
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
