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
Eric Lemings wrote: [...] 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? They are fully established (grep for "< class" vs " I can start a Wiki page for this unless you'd prefer to do it. Starting a page on coding style sounds like a great idea. [...] +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. That may be the case but... Otherwise, we should use just a plain "Type" name. ...we have an established convention that has the T at the end. I'm opposed to using different conventions for new vs existing code and I see no compelling reason to change the thousands of lines of existing code to conform to a different convention. What I'd like us to do is establish a convention to use for variadic templates. The convention that makes the most sense to me is one that builds on the existing convention for ordinary (non-variadic) templates. Of the three choices that I gave initially: template // in type traits template // in tuple template // in the spec the one that fits the bill is the first one: template We can tweak the name of the parameter pack but I don't see how we can change the name of the first parameter without deviating from the current convention. 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. For the non-variadic case this is quite elegantly handled by concepts. For example, the new variadic function template std::min() currently declared in the working paper like so: template const T& min (const T& a, const Args&... args); becomes: template requires SameType const T& min (const T& a , const Args&... args); [...] +template +_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. Yes, we discussed consistency at the last meeting. IIUC, the editor is going to make the appropriate changes to bring the new text into accord with the existing convention (i.e., change "T const&" to "const T&"). Martin
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
> -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
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! :)
+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
+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.
