Implementing N3584 "Addressing Tuples by type"

[Marshall Clow]

On Jul 12, 2013, at 12:42 PM, Howard Hinnant <hhinnant at apple.com> wrote:

> 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.

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:mclow.lists at gmail.com>

A.D. 1517: Martin Luther nails his 95 Theses to the church door and is promptly moderated down to (-1, Flamebait).
        -- Yu Suzuki
-------------- next part --------------
A non-text attachment was scrubbed...
Name: n3584-2.patch
Type: application/octet-stream
Size: 15080 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130712/8ecb94f6/attachment.obj>