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: local policy)
Content-class: urn:content-classes:message
MIME-Version: 1.0
Content-Type: text/plain;
	charset="iso-8859-1"
Content-Transfer-Encoding: quoted-printable
Subject: RE: svn commit: r668318 - in /stdcxx/branches/4.3.x:
 include/rw/_tuple.h include/rw/_tuple_traits.h include/tuple
 tests/utilities/20.tuple.cnstr.cpp
Date: Mon, 23 Jun 2008 16:13:51 -0600
Message-ID: 
 <CFFDD219128FD94FB4F92B99F52D0A4901DB05E0@exchmail01.Blue.Roguewave.Com>
Thread-Topic: svn commit: r668318 - in /stdcxx/branches/4.3.x:
 include/rw/_tuple.h include/rw/_tuple_traits.h include/tuple
 tests/utilities/20.tuple.cnstr.cpp
Thread-Index: AcjVdMUw06SFFZ6ETyGZ3tgMmD2CpQABoTUw
References: <20080616211649.7F90A23889F3@eris.apache.org>
 <48600F33.8000908@roguewave.com>
From: "Eric Lemings" <Eric.Lemings@roguewave.com>
To: <dev@stdcxx.apache.org>

=20

> -----Original Message-----
> From: Martin Sebor [mailto:sebor@roguewave.com]=20
> Sent: Monday, June 23, 2008 3:02 PM
> To: dev@stdcxx.apache.org
> Subject: Re: svn commit: r668318 - in /stdcxx/branches/4.3.x:=20
> include/rw/_tuple.h include/rw/_tuple_traits.h include/tuple=20
> tests/utilities/20.tuple.cnstr.cpp
>=20
> elemings@apache.org wrote:
> > Author: elemings
> > Date: Mon Jun 16 14:16:48 2008
> > New Revision: 668318
> >=20
> > URL: http://svn.apache.org/viewvc?rev=3D668318&view=3Drev
> > Log:
> > 2008-06-16  Eric Lemings <eric.lemings@roguewave.com>
> >=20
> > 	STDCXX-958
> > 	* include/tuple, include/rw/_tuple.h,=20
> include/rw/_tuple_traits.h:
> > 	Add initial version of headers defining tuple interface and
> > 	implementation.  (Only tested on Linux/gcc-4.3=20
> platforms so far.)
> > 	* tests/utilities/20.tuple.cnstr.cpp: Rough outline of first
> > 	tuple test program.
> >=20
> [...]
> > +#    if !defined _RWSTD_NO_VARIADIC_TEMPLATES
> > +
> > +
> > +_RWSTD_NAMESPACE (std) {
> > +
> > +
> > +// 20.3.1, class template tuple:
> > +
> > +/**
> > + * @defgroup tuple Tuples [tuple]
> > + *
> > + * A fixed-size collection of values with variable,=20
> heterogenous types.
> > + * This class template is used to instantatiate tuple=20
> types.  A tuple
> > + * type specifies zero or more element types for defining=20
> tuple values.
> > + * A tuple value can be constructed from a tuple type that=20
> holds one
> > + * value for each element type in the tuple.
> > + *
> > + * @param Types A list of zero or more types.  No applicable type
> > + *              requirements or restrictions.
> > + */
> > +template < class... Types >
>=20
> Please follow the established convention and drop the spaces
> before class and before the closing pointy brace. Thanks! :)

It would be a lot easier if these conventions were actually ESTABLISHED.
Until they are established, we're going to be constantly running into
this problem.  Don't you agree?

I can start a Wiki page for this unless you'd prefer to do it.

>=20
> > +class tuple;
> > +
> > +/**
> > + * @internal
> > + * The template specialization for empty tuples.
> > + */
> > +_RWSTD_SPECIALIZED_CLASS
> > +class tuple< >
>=20
> Also please remove the space between the pointy braces here.
>=20
> > +{
> > +    // empty
> > +};
> > +
> > +/**
> > + * @internal
> > + * The basic template specialization for most tuples.  This class
> > + * template is used to instantiate all tuple types except for empty
> > + * tuples and tuples with exactly two element types.
> > + *
> > + * @param _HeadT First element type (required).
> > + * @param _TailT Template parameter pack for remaining=20
> element types.
> > + */
> > +template < class _HeadT, class... _TailT >
>=20
> In the interest of consistency we should adopt the same naming
> convention for the names of variadic template parameters. We
> have _TypeT and and Types in traits, _HeadT and _TailT here,
> and TTypes and UTypes in the standard. Which one should it
> be?

It is my opinion that the T and U prefixes/suffixes are only necessary
in binary contexts; i.e. where there are two different types or type
lists.  Otherwise, we should use just a plain "Type" name.

Furthermore, generic "Type" names should only be used in truly
generic contexts; i.e. contexts where just about any type can be
used.  Otherwise, the name should imply the restrictions/requirements
on the type; e.g. _ScalarType, _IntConst.  In the recursive tuple
case, using generic names would make the code much harder to follow
IMHO.


>=20
> > +class tuple< _HeadT, _TailT... >
> > +    : tuple< _TailT... >
> > +{
> > +    typedef tuple< _TailT... >  _Base;
> > +
> > +protected:
> > +
> > +    _HeadT  _C_head;
>=20
> Why protected and not private?

Good point.  Given there's an internal accessor now, this could be
private I think.

>=20
> > +
> > +public:
> > +
> > +    /**
> > +     * Construct tuple with default values.
> > +     */
> > +    tuple ()
> > +        : _C_head (), _Base () { /* empty */ }
>=20
> Unless tuple is somehow different from other (non-variadic
> classes) I think _Base() here should be listed first, because
> it's initialized first, no?

Yep.  I think these were all done in a subsequent change.

>=20
> > +
> > +    /**
> > +     * Copy construct tuple from a different tuple value.
> > +     * @param __tuple Another tuple value with same type.
> > +     */
> > +    tuple (const tuple& __tuple)
> > +        : _C_head (__tuple._C_head), _Base (__tuple) { /*=20
> empty */ }
>=20
> Same as above.
>=20
> > +
> > +    /**
> > +     * Copy assign tuple from a different tuple value.
> > +     * @param __tuple Another tuple value with same type.
> > +     */
> > +    tuple& operator=3D (const tuple& __tuple) {
> > +        _C_head =3D __tuple._C_head;
> > +        _Base::operator=3D (__tuple);
>=20
> Similarly to the ctor, the base assignment operator should
> probably be invoked first.
>=20
> > +        return *this;
> > +    }
> > +
> > +    _EXPLICIT
> > +    tuple (const _HeadT& __head, const _TailT&... __tail)
> > +        : _C_head (__head), _Base (__tail...) { /* empty */ }
> > +
> > +#      if !defined _RWSTD_NO_RVALUE_REFERENCES
> > +
> > +    tuple (tuple&& __tuple)
> > +        : _C_head (std::move (__tuple._C_head))
> > +        , _Base (std::forward<_Base> (__tuple)) { /* empty */ }
>=20
> The initialization order should follow the default ctor case
> (i.e., _Base first). Also, the existing convention doesn't
> drop the comma.
>=20
> > +
> > +    tuple& operator=3D (tuple&& __tuple) {
> > +        _C_head =3D std::move (__tuple._C_head);
> > +        _Base::operator=3D (__tuple);
>=20
> Same as the other assignment operator.
>=20
> Also, I think it would make the class easier to read if
> overloads were kept together wherever possible (i.e., all
> ctors, all assignment operators, etc.) Although I think
> I see why you didn't -- to avoid duplicating the rvalue
> references test. Hmm. Have we decided on the list of
> required compiler features yet? I saw your Compiler
> Requirements page on the wiki (thanks for starting it,
> btw.) and rvalue references isn't there. Do we think we
> can provide a useful implementation of C++ 0x without
> rvalue references?

No, probably not.  We could make _RWSTD_EXT_CXX_0X and
_RWSTD_NO_RVALUE_REFERENCES mutually dependent.

>=20
> > +        return *this;
> > +    }
> > +
> > +#      endif   // !defined _RWSTD_NO_RVALUE_REFERENCES
> > +
> > +#      if !defined _RWSTD_NO_MEMBER_TEMPLATES
>=20
> I think we can safely rely on member templates in C++ 0x.
> Do we all agree?

Yeah, I believe I removed this in a latter change.

> [...]
> > Added: stdcxx/branches/4.3.x/include/rw/_tuple_traits.h
> [...]
> > +template <class _Type>
> > +struct constructible_with_allocator_prefix;
> > +
> > +/**
> > + * Allows tuple construction with an allocator prefix=20
> argument.  This
> > + * trait informs other library components that tuple can=20
> be constructed
> > + * with an allocator pre=EF=AC=81x argument.
>                          ^^^^^^^^
>=20
> Looks like an unreadable character has sneaked in here.

Noted.

>=20
> [...]
> > Added: stdcxx/branches/4.3.x/include/tuple
> [...]
> > +
> > +#if defined _RWSTD_NO_EXT_CXX_0X
> > +#  error _RWSTD_NO_EXT_CXX_0X defined and C++0x header included
> > +#endif   // defined _RWSTD_NO_EXT_CXX_0X
> > +
> > +#ifndef _RWSTD_TUPLE_INCLUDED
> > +#  define _RWSTD_TUPLE_INCLUDED
> > +
> > +#  include <rw/_tuple.h>
> > +#  include <rw/_tuple_traits.h>
> > +
> > +
> > +_RWSTD_NAMESPACE (__rw) {
> > +
> > +struct __rw_ignore { /* empty */ };
> > +
> > +/// Transforms _Type into a suitable make_tuple() return type.
> > +template <class _Type>
> > +struct __rw_make_tuple {
> > +    /// @todo Deduce correct return type.
> > +    typedef _Type type;
> > +};
> > +
> > +}   // namespace __rw
> > +
> > +
> > +_RWSTD_NAMESPACE (std) {
> > +
> > +
> > +// 20.3.3, tuple creation functions:
> > +
> > +const _RW::__rw_ignore ignore =3D _RW::__rw_ignore ();
>=20
> This either needs to be static or preferably defined out
> of line (in a .cpp file). Ditto for std::allocator_arg
> in rw/_allocator.h.

Yep.  Will rectify.

>=20
> > +
> [...]
> > +// 20.3.1.4, tuple helper classes:
> > +
> > +template <class>
>=20
> Please don't omit the names of template parameters. I wish
> we could but I don't think aCC 3 supports it yet.

Ah.  Wasn't aware of that.

>=20
> > +class tuple_size; // undefined
> > +
> > +template <class... _Types>
> > +class tuple_size<tuple<_Types...> >;
> > +
> > +template <int, class>
> > +class tuple_element; // undefined
> > +
> > +template <int _Index, class... _Types>
> > +class tuple_element<_Index, tuple<_Types...> >;
> > +
> > +
> > +// 20.3.1.5, element access:
> > +
> > +template <int _Index, class... _Types>
> > +_TYPENAME tuple_element<_Index, tuple<_Types...> >::type&
> > +get (tuple<_Types...>&);
> > +
> > +template <int _Index, class ... _Types>
> > +_TYPENAME tuple_element<_Index, tuple<_Types...> >::type const&
>=20
> The const should come before tuple_element.

For style?  They are semantically equivalent though, aren't they?
BTW, this is how it is specified in the draft.

>=20
> > +get (const tuple<_Types...>&);
> > +
> > +
> > +// 20.3.1.6, relational operators:
> > +
> > +template <class... _TypesT, class... _TypesU>
> > +bool operator=3D=3D (const tuple<_TypesT...>& __x,
> > +                 const tuple<_TypesU...>& __y);
>=20
> I think aCC 6 gives a remark for function argument names
> in function prototypes.

That's a placeholder.  Once I implement it, the names will be
required.

>=20
> > +
> > +template <class... _TypesT, class... _TypesU>
> > +bool operator< (const tuple<_TypesT...>& __x,
> > +                const tuple<_TypesU...>& __y);
> > +
> > +/**
> > + * @param __x Tuple value on LHS of operator.
> > + * @param __y Tuple value on RHS of operator.
> > + * @return !(__x =3D=3D __y)
> > + */
> > +template <class... _TypesT, class... _TypesU>
> > +bool operator!=3D (const tuple<_TypesT...>& __x,
> > +                 const tuple<_TypesU...>& __y)
> > +{
> > +    return !(__x =3D=3D __y);
> > +}
>=20
> This and all other function templates defined here need to
> be declared inline.

Yep.  I was wondering about that today and found pretty uniform
`inline' usage.

Brad.