On Jul 12, 2013, at 12:42 PM, Howard Hinnant <[email protected]> wrote:
> On Jul 11, 2013, at 12:29 PM, Marshall Clow <[email protected]> wrote: > >> Tuple and pair. > > This is really nice, thanks Marshall! I especially like the care you put > into the diagnostic messages. Thanks! > A few minor comments: > > 1. The new tests should go under tuple.tuple/tuple.elem, instead of under > tuple.general, because the former is where the standard defines them. Done. > > 2. This technically doesn't matter (else the tests wouldn't be passing), but > I would prefer that the new get definitions go /after/ the old get > definitions in <tuple> since the new one's call the old ones. The first time > I looked at this I had to ask myself if qualified name lookup would really > find the old get's from inside the new get's. I'd rather code readers not > need to ask this question. Done. > 3. In tuple.by.type.pass.cpp, in two places make_tuple is used with explicit > template arguments. I wold prefer instead that make_tuple not be used at all: > > std::tuple<int, std::string, cf> t1( 42, "Hi", { 1,2 } ); > ... > std::tuple<int, std::string, int, cf> t2( 42, "Hi", 23, { 1,2 } ); > > This is just less complicated. Done. > 4. Also in tuple.by.type.pass.cpp, I'd like to see this test added: > > { > std::tuple<std::unique_ptr<int>> t(std::unique_ptr<int>(new int(4))); > std::unique_ptr<int> p = std::get<std::unique_ptr<int>>(std::move(t)); > assert(*p == 4); > assert(std::get<0>(t) == nullptr); > } > > This test should reveal a minor bug in the implementation (you've already > fixed it in pair :-)). As a sanity check, it probably wouldn't hurt to make > a .fail.cpp out of this too: Done and fixed. > { > std::tuple<std::unique_ptr<int>> t(std::unique_ptr<int>(new int(4))); > std::unique_ptr<int> p = std::get<std::unique_ptr<int>>(t); > } Done. > 5. I don't see any tests for get<T>(pair). They didn't get included in the diff :-( They're there now. Thanks for the review. -- Marshall Marshall Clow Idio Software <mailto:[email protected]> A.D. 1517: Martin Luther nails his 95 Theses to the church door and is promptly moderated down to (-1, Flamebait). -- Yu Suzuki
n3584-2.patch
Description: Binary data
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
