Eric Lemings wrote:
I got this problem fixed with my latest change.
The tuple test suite needs to be bolstered (suggestions for additional
test strategies greatly appreciated BTW) but the implementation as a
whole is stable enough for code review.
Excellent! It looks very clean -- I like it. Just a few quick
questions and comments on the implementation (I haven't looked
at the tests yet):
1. <tuple> should only #include the traits header(s) for the
the traits it needs, not all of <type_traits>, and use only
the __rw_xxx traits (not std::xxx)
2. would it be possible (and would it make sense) for <tuple>
to forward declare reference_wrapper instead of #including the
whole definition of the class to reduce the size of translation
units that don't use the class?
3. why is is the base class __rw_tuple rather than std::tuple?
(seems there's a lot of duplicate code between the two)
4. assuming the answer to (3) is yes, are the casts in tuple
copy ctor and copy assignment necessary or might there be
a cleaner way to accomplish the same thing? if not, please
consider adding a brief comment explaining why they are
important
5. assuming the answer to (3) is yes, would it be better to
define the [in]equality comparison for __rw_tuple as (possibly
member) functions called _C_equal and _C_less to avoid the
casts in the same operators in tuple and reduce the size of
the overload set?
6. we can and should remove the _RWSTD_NO_EXT_CXX_0X guards
from all implementation headers under include/rw/
7. the right place to #define _RWSTD_XXX macros is <rw/_defs.h>
8. why is std::ignore typedef'd to __rw_ignore when the latter
isn't used anywhere (AFAICS), and shouldn't it be defined in
a .cpp file to avoid multiply defined symbols?
9. shouldn't tuple_element take size_t as template argument
as per LWG issue 755
10. the non-trivial bits could use some comments :)
In __rw_tuple, are the accessors necessary? Would it be possible
(and cleaner) to declare the appropriate specialization(s) of
__rw_tuple a friend instead and have it (them) access the data
members directly?
A few notes on style :)
a) please re-read points (0), (1), and (5) here:
http://www.nabble.com/forum/ViewPost.jtp?post=18174685
b) please use the name _TypeT (as opposed to _Type) for template
parameters
c) in ctor initializer lists spanning multiple lines, please
avoid dropping the colon or the comma; i.e.,
struct A: Base {
int _C_i;
A (int i): Base (), _C_i (i) { /* empty */ }
or (for long initializer lists):
A (int i):
Base (very_long_sequence_of_arguments),
_C_i (i) {
// empty
}
d) I realize we haven't formally closed the VOTE on the variadic
template parameters but I think we're all in agreement so we
might as well start using the right naming convention (_TypeT
and _TypesT, etc.)
Martin
Brad.
-----Original Message-----
From: Eric Lemings [mailto:[EMAIL PROTECTED]
Sent: Monday, June 30, 2008 4:42 PM
To: [email protected]
Subject: RE: Tuple status
Committed.
Note, there is still a problem with using reference wrappers with
make_tuple() which I'm currently working on but I didn't want
to hold up
this rather large change any longer.
Brad.
-----Original Message-----
From: Eric Lemings [mailto:[EMAIL PROTECTED]
Sent: Monday, June 30, 2008 10:46 AM
To: [email protected]
Subject: Tuple status
Just a brief status on tuple progress.
I got the remaining portions of tuple (except the tie()
function) work
late Friday. I did a personal code review over the weekend and am
applying some cleanup and other finishing touches. Should
be checking
in a lot of changes later today. So just a heads up.
Brad.
