STL_MSFT added a comment.
Found some minor issues.
================
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);
----------------
Technically, addressof() lives in <memory>.
================
Comment at: test/std/utilities/variant/variant.get/get_if_type.pass.cpp:52
+ const V v(x);
+ ASSERT_SAME_TYPE(decltype(std::get_if<int&>(std::addressof(v))), int*);
+ assert(std::get_if<int&>(std::addressof(v)) == &x);
----------------
<memory> for addressof() (although I'll note that the other tests are saying
&v).
================
Comment at: test/std/utilities/variant/variant.get/get_index.pass.cpp:176
+ ASSERT_SAME_TYPE(decltype(std::get<0>(std::move(v))), const int&&);
+ assert(std::get<0>(std::move(v)) == 42);
+ }
----------------
Technically, <utility> for move().
================
Comment at: test/std/utilities/variant/variant.get/get_index.pass.cpp:220
+template <std::size_t I>
+using Idx = std::integral_constant<size_t, I>;
+
----------------
<type_traits> for integral_constant.
================
Comment at: test/std/utilities/variant/variant.get/get_type.pass.cpp:99
+ int x = 42;
+ V v(std::move(x));
+ ASSERT_SAME_TYPE(decltype(std::get<int&&>(v)), int&);
----------------
<utility> move()
================
Comment at: test/std/utilities/variant/variant.hash/hash.pass.cpp:15
+
+// template <class ..._Types> struct hash<variant<Types...>>;
+// template <> struct hash<monostate>;
----------------
"<class ..._Types>" has really terrible spacing. (Or as I like to say, "Space.
The final frontier.")
================
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>, "");
----------------
You aren't directly including <type_traits> (not sure if
"variant_test_helpers.hpp" is).
================
Comment at:
test/std/utilities/variant/variant.helpers/variant_alternative.pass.cpp:60
+ }
+#if !defined(TEST_VARIANT_REFERENCES)
+ {
----------------
The sense of this #if test seems to be flipped.
================
Comment at: test/std/utilities/variant/variant.helpers/variant_size.pass.cpp:20
+// template <class T> constexpr size_t variant_size_v
+// = variant_size<T>::value;
+
----------------
No indentation! :-<
================
Comment at: test/std/utilities/variant/variant.helpers/variant_size.pass.cpp:35
+ static_assert(std::variant_size_v<const volatile V> == E, "");
+ static_assert(std::is_base_of<std::integral_constant<std::size_t, E>,
+ std::variant_size<V>>::value, "");
----------------
Need <type_traits>.
================
Comment at: test/std/utilities/variant/variant.helpers/variant_size.pass.cpp:41
+{
+ test<std::variant<>, 0>();
+ test<std::variant<void*>, 1>();
----------------
Hmm, is this cromulent now that empty variants are forbidden?
================
Comment at: test/std/utilities/variant/variant.monostate/monostate.pass.cpp:20
+#include <type_traits>
+#include <cassert>
+
----------------
Don't actually need <cassert> since you have no plain asserts and this is C++17.
================
Comment at: test/std/utilities/variant/variant.relops/relops.pass.cpp:59
+inline bool operator<=(MakeEmptyT const&, MakeEmptyT const&) { assert(false); }
+inline bool operator>(MakeEmptyT const&, MakeEmptyT const&) { assert(false); }
+inline bool operator>=(MakeEmptyT const&, MakeEmptyT const&) { assert(false); }
----------------
No space after >, but space after < above. Madness!
================
Comment at: test/std/utilities/variant/variant.relops/relops.pass.cpp:66
+ try {
+ v = std::move(v2);
+ assert(false);
----------------
<utility> move()
================
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;
----------------
NoCopy doesn't even have a default ctor, how are you supposed to make one?
Appears to apply to the classes below.
================
Comment at:
test/std/utilities/variant/variant.variant/variant.assign/move.pass.cpp:114
+ {
+ // variant only provides move assignment when beth the move and move
+ // constructors are well formed
----------------
"beth". Also, "move and move constructors"?
================
Comment at:
test/std/utilities/variant/variant.variant/variant.assign/move.pass.cpp:212
+ V v2(42);
+ V& vref = (v1 = std::move(v2));
+ assert(&vref == &v1);
----------------
<utility> move()
================
Comment at:
test/std/utilities/variant/variant.variant/variant.ctor/T.pass.cpp:65
+ }
+#if defined(VARIANT_TEST_REFERENCES)
+ {
----------------
You have like twenty different macros for enabling references.
================
Comment at:
test/std/utilities/variant/variant.variant/variant.ctor/in_place_index_init_list_Args.pass.cpp:1
+// -*- C++ -*-
+//===----------------------------------------------------------------------===//
----------------
Should this test filename contain capital A?
================
Comment at:
test/std/utilities/variant/variant.variant/variant.ctor/in_place_type_Args.pass.cpp:23
+#include <type_traits>
+#include <string>
+#include <cassert>
----------------
This file doesn't actually use <string>.
================
Comment at:
test/std/utilities/variant/variant.variant/variant.ctor/move.pass.cpp:60
+ MakeEmptyT(MakeEmptyT &&) {
+ throw 42;
+ }
----------------
Inconsistent indentation.
================
Comment at:
test/std/utilities/variant/variant.variant/variant.mod/emplace_index_args.pass.cpp:88
+ {
+ using V = std::variant<int, long, const void*,
+ TestTypes::NoCtors, std::string>;
----------------
Why wrap here?
================
Comment at:
test/std/utilities/variant/variant.variant/variant.swap/swap.pass.cpp:114
+ noexcept(NT_Swap) {
+ lhs.swap_called++;
+ do_throw<!NT_Swap>();
----------------
Postincrement? You monster!
================
Comment at:
test/std/utilities/variant/variant.variant/variant.swap/swap.pass.cpp:286
+ assert(T::move_assign_called == 0);
+ assert(std::get<0>(v1).value == 42); // throw happend before v1 was
moved from
+ assert(std::get<0>(v2).value == 100);
----------------
happend
================
Comment at: test/std/utilities/variant/variant.visit/visit.pass.cpp:71
+ last_call_type = type;
+ last_call_args = std::addressof(makeArgumentID<Args...>());
+ }
----------------
Technically, <memory> for addressof().
================
Comment at: test/std/utilities/variant/variant.visit/visit.pass.cpp:105
+ assert(Fn::check_call<int&>(CT_Const | CT_LValue));
+ std::visit(std::move(obj), v);
+ assert(Fn::check_call<int&>(CT_NonConst | CT_RValue));
----------------
<utility> move()
================
Comment at: test/std/utilities/variant/variant.visit/visit.pass.cpp:202
+struct ReturnFirst {
+ template <class ...Args>
+ constexpr int operator()(int f, Args&&...) const {
----------------
Space. The final frontier. (Occurs below)
================
Comment at: test/support/variant_test_helpers.hpp:53
+ MakeEmptyT(MakeEmptyT &&) {
+ throw 42;
+ }
----------------
Indentation
================
Comment at: test/support/variant_test_helpers.hpp:63
+};
+static_assert(std::is_swappable_v<MakeEmptyT>, ""); // required for test
+
----------------
<type_traits>
https://reviews.llvm.org/D26903
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
