Mailing-List: contact dev-help@stdcxx.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@stdcxx.apache.org
Received-SPF: pass (athena.apache.org: domain of msebor@gmail.com designates
 209.85.198.234 as permitted sender)
DomainKey-Signature: a=rsa-sha1; c=nofws;
        d=gmail.com; s=gamma;
        h=message-id:date:from:organization:user-agent:mime-version:to
         :subject:references:in-reply-to:content-type
         :content-transfer-encoding:sender;
        b=rvIE2DwMqVuA4a6Ybh8UpLY99rJusUeoipzWkmO+/GgkSWpD2FYb+3KHr8QO0G5Rt6
         xpHKkgXQKW9/CjIvMM2+inMXPd9/aA6U1eq0ZokCa1h2YGNrZW/P1K/fvtRCGlATvWvi
         CTyFQP+iOWAJytYL3MrLKvCCL6aE3lepa9Uf4=
Message-ID: <486B0D2D.4090500@roguewave.com>
Date: Tue, 01 Jul 2008 23:07:57 -0600
From: Martin Sebor <sebor@roguewave.com>
Organization: Rogue Wave Software, Inc.
User-Agent: Thunderbird 2.0.0.14 (X11/20080501)
MIME-Version: 1.0
To: dev@stdcxx.apache.org
Subject: Re: Tuple status
References: 
 <CFFDD219128FD94FB4F92B99F52D0A4901DB0607@exchmail01.Blue.Roguewave.Com>
 <CFFDD219128FD94FB4F92B99F52D0A4901DB060A@exchmail01.Blue.Roguewave.Com>
 <CFFDD219128FD94FB4F92B99F52D0A4901DB060F@exchmail01.Blue.Roguewave.Com>
In-Reply-To: 
 <CFFDD219128FD94FB4F92B99F52D0A4901DB060F@exchmail01.Blue.Roguewave.Com>
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Sender: Martin Sebor <msebor@gmail.com>

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:Eric.Lemings@roguewave.com] 
>> Sent: Monday, June 30, 2008 4:42 PM
>> To: dev@stdcxx.apache.org
>> 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:Eric.Lemings@roguewave.com] 
>>> Sent: Monday, June 30, 2008 10:46 AM
>>> To: dev@stdcxx.apache.org
>>> 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.
>>>