> -----Original Message-----
> From: Martin Sebor [mailto:[EMAIL PROTECTED]
> Sent: Monday, June 23, 2008 3:02 PM
> To: dev@stdcxx.apache.org
> 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
>
> [EMAIL PROTECTED] wrote:
> > Author: elemings
> > Date: Mon Jun 16 14:16:48 2008
> > New Revision: 668318
> >
> > URL: http://svn.apache.org/viewvc?rev=668318&view=rev
> > Log:
> > 2008-06-16 Eric Lemings <[EMAIL PROTECTED]>
> >
> > STDCXX-958
> > * include/tuple, include/rw/_tuple.h,
> include/rw/_tuple_traits.h:
> > Add initial version of headers defining tuple interface and
> > implementation. (Only tested on Linux/gcc-4.3
> platforms so far.)
> > * tests/utilities/20.tuple.cnstr.cpp: Rough outline of first
> > tuple test program.
> >
> [...]
> > +# 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,
> heterogenous types.
> > + * This class template is used to instantatiate tuple
> types. A tuple
> > + * type specifies zero or more element types for defining
> tuple values.
> > + * A tuple value can be constructed from a tuple type that
> 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 >
>
> 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.
>
> > +class tuple;
> > +
> > +/**
> > + * @internal
> > + * The template specialization for empty tuples.
> > + */
> > +_RWSTD_SPECIALIZED_CLASS
> > +class tuple< >
>
> Also please remove the space between the pointy braces here.
>
> > +{
> > + // 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
> element types.
> > + */
> > +template < class _HeadT, class... _TailT >
>
> 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.
>
> > +class tuple< _HeadT, _TailT... >
> > + : tuple< _TailT... >
> > +{
> > + typedef tuple< _TailT... > _Base;
> > +
> > +protected:
> > +
> > + _HeadT _C_head;
>
> Why protected and not private?
Good point. Given there's an internal accessor now, this could be
private I think.
>
> > +
> > +public:
> > +
> > + /**
> > + * Construct tuple with default values.
> > + */
> > + tuple ()
> > + : _C_head (), _Base () { /* empty */ }
>
> 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.
>
> > +
> > + /**
> > + * 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) { /*
> empty */ }
>
> Same as above.
>
> > +
> > + /**
> > + * Copy assign tuple from a different tuple value.
> > + * @param __tuple Another tuple value with same type.
> > + */
> > + tuple& operator= (const tuple& __tuple) {
> > + _C_head = __tuple._C_head;
> > + _Base::operator= (__tuple);
>
> Similarly to the ctor, the base assignment operator should
> probably be invoked first.
>
> > + 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 */ }
>
> The initialization order should follow the default ctor case
> (i.e., _Base first). Also, the existing convention doesn't
> drop the comma.
>
> > +
> > + tuple& operator= (tuple&& __tuple) {
> > + _C_head = std::move (__tuple._C_head);
> > + _Base::operator= (__tuple);
>
> Same as the other assignment operator.
>
> 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.
>
> > + return *this;
> > + }
> > +
> > +# endif // !defined _RWSTD_NO_RVALUE_REFERENCES
> > +
> > +# if !defined _RWSTD_NO_MEMBER_TEMPLATES
>
> 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
> argument. This
> > + * trait informs other library components that tuple can
> be constructed
> > + * with an allocator preï¬x argument.
> ^^^^^^^^
>
> Looks like an unreadable character has sneaked in here.
Noted.
>
> [...]
> > 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 = _RW::__rw_ignore ();
>
> 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.3.1.4, tuple helper classes:
> > +
> > +template <class>
>
> 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.
>
> > +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&
>
> 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.
>
> > +get (const tuple<_Types...>&);
> > +
> > +
> > +// 20.3.1.6, relational operators:
> > +
> > +template <class... _TypesT, class... _TypesU>
> > +bool operator== (const tuple<_TypesT...>& __x,
> > + const tuple<_TypesU...>& __y);
>
> 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.
>
> > +
> > +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 == __y)
> > + */
> > +template <class... _TypesT, class... _TypesU>
> > +bool operator!= (const tuple<_TypesT...>& __x,
> > + const tuple<_TypesU...>& __y)
> > +{
> > + return !(__x == __y);
> > +}
>
> 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.
