Implementing N3584 "Addressing Tuples by type"

[Howard Hinnant]

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

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

This looks great to me Marshall!  Thanks much for your perseverance on this.  The only nit I have is that get<type>(tuple&&) should use move instead of forward (you've already got this correct in get<type>(pair&&)).

Please commit with this minor change.

Thanks,
Howard