On Jul 16, 2013, at 1:32 AM, Marshall Clow <[email protected]> wrote:
> On Jul 15, 2013, at 4:43 PM, Howard Hinnant <[email protected]> wrote: > >> >> On Jul 15, 2013, at 5:19 PM, Marshall Clow <[email protected]> wrote: >> >>> This is the second step towards fixing >>> http://llvm.org/bugs/show_bug.cgi?id=16599 >>> >>> Make std::pair's constructors and comparison operators (and make_pair) >>> constexpr. >> >> This is looking great! >> >> I've got just a few minor tweaks, and one big favor... ;-) >> >> While you're in the pair neighborhood, it would be great if you could do a >> drive-by fix on the pair copy and move constructors concerning making them = >> default when _LIBCPP_HAS_NO_DEFAULTED_FUNCTIONS is not defined, but keeping >> the current definitions when _LIBCPP_HAS_NO_DEFAULTED_FUNCTIONS is defined. > > Done. > >> Otherwise I can find nothing at all to complain about in <utility>. >> >> On the tests I think we need to test every constructor we're marking >> constexpr. The easiest way to ensure complete coverage is when you think >> you have it, comment out the constexpr on the constructors one by one and >> make sure you have a failing test. utility/pairs/pairs.pair has an >> attempted test/per constructor. We should put a constexpr test in each of >> those files targeting the relevant constructor. > > Done, except for one constructor: > template<class _Tuple, > class = typename enable_if<__tuple_convertible<_Tuple, > pair>::value>::type> > _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX11 > pair(_Tuple&& __p) > > We don't currently have any tests for this because I haven't done the > constexpr stuff for tuple yet. > >> The make_pair test should go in utility/pairs/pair.spec/make_pair.pass.cpp. >> And the comparison operator tests should go in >> utility/pairs/pair.spec/comparison.pass.cpp. This will add some duplication >> in the tests, but that way we can find all of the relevant tests when we >> have a bug report in this area a year from now. > > I think I've got these all in the right place now. > > Updated patch attached. This looks awesome Marshall, thanks! Please commit. I have one minor nit: In pairs/pairs.pair/assign_const_pair_U_V.pass.cpp, there is an unnecessary test. This is the same test as the one you have in const_pair_U_V.pass.cpp. The one in assign_const_pair_U_V.pass.cpp can be removed (svn revert the whole file). Thanks! Howard _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
