Implementing N3584 "Addressing Tuples by type"

Howard Hinnant hhinnant at apple.com
Fri Jul 12 12:42:26 PDT 2013


On Jul 11, 2013, at 12:29 PM, Marshall Clow <mclow.lists at gmail.com> wrote:

> Tuple and pair.


This is really nice, thanks Marshall!  I especially like the care you put into the diagnostic messages.

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.

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.

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.

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:

    {
    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);
    }

5.  I don't see any tests for get<T>(pair).

Thanks!
Howard




More information about the cfe-commits mailing list