incubator-stdcxx-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Martin Sebor <se...@roguewave.com>
Subject [VOTE] naming convention for variadic template arguments (was: 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 Fri, 27 Jun 2008 21:05:29 GMT
This thread kind of fizzled out so let me resurrect it and reiterate
the proposed naming convention to follow unless more specific names
are appropriate:

     template <class _TypeT, class... _Types>     # 1

This is in contrast to other styles, including:

     template <class _Type, class... _Types>      # 2
     template <class _HeadT, class... _TailT>     # 3
     template <class _TType, class... _UTypes>    # 4

The rationale for the proposed convention (#1) is that:

   A) unlike the alternatives, the first name (_TypeT) follows
      a well-established and entrenched convention for template
      parameters used throughout the library
   B) also unlike the alternatives, it is being used in the
      implementation of <type_traits>
   C) unlike (#2) (although not as well as #3) it more clearly
      distinguishes between the name of the first parameter and
      the parameter pack

Check the box below to vote:

   [ ] In favor
   [ ] Opposed (suggest an improvement and rationale)

Martin Sebor wrote:
> elemings@apache.org 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 <eric.lemings@roguewave.com>
>>
>>     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! :)
> 
>> +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?
> 
>> +class tuple< _HeadT, _TailT... >
>> +    : tuple< _TailT... >
>> +{
>> +    typedef tuple< _TailT... >  _Base;
>> +
>> +protected:
>> +
>> +    _HeadT  _C_head;
> 
> Why protected and not private?
> 
>> +
>> +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?
> 
>> +
>> +    /**
>> +     * 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?
> 
>> +        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?
> [...]
>> 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 prefix argument.
>                         ^^^^^^^^
> 
> Looks like an unreadable character has sneaked in here.
> 
> [...]
>> 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.
> 
>> +
> [...]
>> +// 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.
> 
>> +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.
> 
>> +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.
> 
>> +
>> +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.
> 
> Martin


Mime
View raw message