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.
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
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
