EricWF marked 4 inline comments as done. EricWF added inline comments.
================ Comment at: include/variant:300-303 + std::tuple_element_t< + __choose_index_type(_NumElem), + std::tuple<unsigned char, unsigned short, unsigned int> + >; ---------------- mpark wrote: > Could we inline the whole thing and also avoid using the `tuple` utilities? > We should also add the `#include <limits>` > > ``` > conditional_t< > (_NumElem < numeric_limits<unsigned char>::max()), > unsigned char, > conditional_t<(_NumElem < numeric_limits<unsigned short>::max()), > unsigned short, > unsigned int>; > ``` Not sure that's any cleaner or clearer. Plus it makes it harder to test. Adding `#include <limits>` as suggested. ================ Comment at: test/libcxx/utilities/variant/variant.variant/variant_size.pass.cpp:52-53 + using Lim = std::numeric_limits<IndexType>; + static_assert(std::__choose_index_type(Lim::max() -1) != + std::__choose_index_type(Lim::max()), ""); + static_assert(std::is_same_v< ---------------- mpark wrote: > I guess I asked you to remove this above. Is this an essential part of the > test? It is a nice part of it. I pushed back on removing the function as noted above. Perhaps this can stay then? https://reviews.llvm.org/D40210 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
