CaseyCarter added inline comments.
================ Comment at: test/std/utilities/variant/variant.hash/hash.pass.cpp:34 + +void test_hash_variant() +{ ---------------- This is non-portable. The fact that both your implementation and mine pass this test suggests they both have poor QoI: There's strong opinion in the community that a variant's index should contribute to its hash value, so that e.g. `variant<int, int>{in_place_index<0>, 42}` and `variant<int, int>{in_place_index<1>, 42}` have differing hash values. ================ Comment at: test/std/utilities/variant/variant.helpers/variant_size.pass.cpp:41 +{ + test<std::variant<>, 0>(); + test<std::variant<void*>, 1>(); ---------------- STL_MSFT wrote: > Hmm, is this cromulent now that empty variants are forbidden? It's not forbidden to *name* `variant<>`; it is forbidden to *instantiate* it. The pertinent specialization of `variant_size`: ``` template <class... Types> struct variant_size<variant<Types...>> : integral_constant<size_t, sizeof...(Types)> { }; ``` is implicitly specified to not instantiate `variant<Types...>`. https://reviews.llvm.org/D26903 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits