stdcxx-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Travis Vitek" <Travis.Vi...@roguewave.com>
Subject RE: svn commit: r675044 - in /stdcxx/branches/4.3.x: include/rw/_tuple.h include/tuple tests/utilities/20.tuple.cnstr.cpp tests/utilities/20.tuple.creation.cpp tests/utilities/20.tuple.h tests/utilities/20.tuple.helpers.cpp
Date Wed, 09 Jul 2008 00:03:59 GMT
 

>Author: elemings
>Date: Tue Jul  8 16:13:36 2008
>New Revision: 675044
>
>URL: http://svn.apache.org/viewvc?rev=675044&view=rev
>Log:
>2008-07-08  Eric Lemings <eric.lemings@roguewave.com>
>
>	STDCXX-958
>	* include/tuple: Second parameter in value move ctor of pair
>	specialization missing rvalue reference.
>	(make_tuple, get, relational operators): Explicitly declare
>	as inline functions.
>	(tie): Implemented.
>	* include/rw/_tuple.h: Fix move semantics in heterogenous move
>	assignment operator.
>	(__rw_ignore): Add assignment operator to ignore all values.
>	* tests/utilities/20.tuple.cnstr.cpp: Added V&V for tuple
>	state and invariants.  Manually inspected proper construction
>	of all test tuples.  Updated/corrected/added tests as necessary.
>	* tests/utilities/20.tuple.creation.cpp: Added simple tie()
>	test.
>	* tests/utilities/20.tuple.h: Minor stylistic changes.
>	* tests/utilities/20.tuple.helpers.cpp: Same.
>
>
>Modified:
>    stdcxx/branches/4.3.x/include/rw/_tuple.h
>    stdcxx/branches/4.3.x/include/tuple
>    stdcxx/branches/4.3.x/tests/utilities/20.tuple.cnstr.cpp
>    stdcxx/branches/4.3.x/tests/utilities/20.tuple.creation.cpp
>    stdcxx/branches/4.3.x/tests/utilities/20.tuple.h
>    stdcxx/branches/4.3.x/tests/utilities/20.tuple.helpers.cpp
>
>Modified: stdcxx/branches/4.3.x/include/rw/_tuple.h
>URL: 
>http://svn.apache.org/viewvc/stdcxx/branches/4.3.x/include/rw/_
>tuple.h?rev=675044&r1=675043&r2=675044&view=diff
>===============================================================
>===============
>--- stdcxx/branches/4.3.x/include/rw/_tuple.h (original)
>+++ stdcxx/branches/4.3.x/include/rw/_tuple.h Tue Jul  8 16:13:36 2008
>@@ -174,7 +174,14 @@
> };
> 
> 
>-struct __rw_ignore { /* empty */ };
>+struct __rw_ignore
>+{
>+    template <class _TypeT>
>+    inline __rw_ignore const&
>+    operator= (const _TypeT& /*unused*/) const {
>+        return *this;
>+    }
>+};
>

I think the commented out parameter name should be removed. I don't see
this in existing code, and I personally find it a bit distracting.

This can probably be changed to use a void return type, which will
simplify the code further. You only really need the return type to chain
assignments or to call a function on the result, none of which we should
be doing.

>Modified: stdcxx/branches/4.3.x/include/tuple
>URL: 
>http://svn.apache.org/viewvc/stdcxx/branches/4.3.x/include/tupl
>e?rev=675044&r1=675043&r2=675044&view=diff
>===============================================================
>===============
>--- stdcxx/branches/4.3.x/include/tuple (original)
>+++ stdcxx/branches/4.3.x/include/tuple Tue Jul  8 16:13:36 2008
>@@ -211,7 +211,7 @@
> #    if !defined _RWSTD_NO_RVALUE_REFERENCES
> 
>     template <class _TypeU1, class _TypeU2>
>-    _EXPLICIT tuple (_TypeU1&& __x, _TypeU2 __y)
>+    _EXPLICIT tuple (_TypeU1&& __x, _TypeU2&& __y)
>         : _Base (_RWSTD_FORWARD (_TypeU1, __x),
>                  _RWSTD_FORWARD (_TypeU2, __y)) { /* empty */ }

Same here with the comment describing the empty function body.

>@@ -377,7 +381,7 @@
> // 20.3.1.5, element access:
> 
> template <_RWSTD_SIZE_T _Index, class _Head, class... _Tail>
>-_TYPENAME tuple_element<_Index, tuple<_Head, _Tail...> >::_Ref
>+inline _TYPENAME tuple_element<_Index, tuple<_Head, _Tail...> >::_Ref
> get (tuple<_Head, _Tail...>& __tuple)
> {
>     typedef tuple_element<_Index, tuple<_Head, _Tail...> > _Tuple;

In the recent past Martin recommended not using the _TYPENAME macro. It
isn't hurting anything, but it could probably be removed. I know that I
no longer use it in the traits code. I also noticed the _EXPLICIT macro
above. I think that one should be added to the list. Martin?

>@@ -385,7 +389,7 @@
> }
> 
> template <_RWSTD_SIZE_T _Index, class _Head, class... _Tail>
>-_TYPENAME tuple_element<_Index, tuple<_Head, _Tail...> >::_ConstRef
>+inline _TYPENAME tuple_element<_Index, tuple<_Head, _Tail...> 
>>::_ConstRef
> get (const tuple<_Head, _Tail...>& __tuple)
> {
>     typedef tuple_element<_Index, tuple<_Head, _Tail...> > _Tuple;
>@@ -396,59 +400,67 @@
> // 20.3.1.6, relational operators:
> 
> template <class... _TypesT, class... _TypesU>
>-bool operator== (const tuple<_TypesT...>& __x,
>-                 const tuple<_TypesU...>& __y)
>+inline bool
>+operator== (const tuple<_TypesT...>& __x,
>+            const tuple<_TypesU...>& __y)
> {
>     return _RWSTD_STATIC_CAST (const 
>_RW::__rw_tuple<_TypesT...>&, __x)
>            == _RWSTD_STATIC_CAST (const 
>_RW::__rw_tuple<_TypesU...>&, __y);

I think there is a formatting issue here. The prevailing style is for
the operator to start of the next line, but for the operands to be lined
up on their left. As an example

  return    __some_really_long_expression_1
         == __some_really_long_expression_2;

I don't really like it all that much, but I'm using it in the traits
code all over the place.

> }
> 
> _RWSTD_SPECIALIZED_FUNCTION
>-bool operator== (const tuple<>& /*__x*/,
>-                 const tuple<>& /*__y*/)
>+inline bool
>+operator== (const tuple<>& /*__x*/,
>+            const tuple<>& /*__y*/)
> {
>     return true;
> }

The commented unused argument names again. Also, I think the
_RWSTD_SPECIALIZED_FUNCTION macro can be eliminated. If I remember
correctly Martin asked that it be removed from the traits implementation
(which I've done).

> 
> template <class... _TypesT, class... _TypesU>
>-bool operator< (const tuple<_TypesT...>& __x,
>-                const tuple<_TypesU...>& __y)
>+inline bool
>+operator< (const tuple<_TypesT...>& __x,
>+           const tuple<_TypesU...>& __y)
> {
>     return _RWSTD_STATIC_CAST (const 
>_RW::__rw_tuple<_TypesT...>&, __x)
>            < _RWSTD_STATIC_CAST (const 
>_RW::__rw_tuple<_TypesU...>&, __y);
> }

Again with the formatting of the first line of the function body. :)

> 
> _RWSTD_SPECIALIZED_FUNCTION
>-bool operator< (const tuple<>& /*__x*/,
>-                const tuple<>& /*__y*/)
>+inline bool
>+operator< (const tuple<>& /*__x*/,
>+           const tuple<>& /*__y*/)
> {
>     return false;
> }

And again with the names... :P

>
>Modified: stdcxx/branches/4.3.x/tests/utilities/20.tuple.cnstr.cpp
>URL: 
>http://svn.apache.org/viewvc/stdcxx/branches/4.3.x/tests/utilit
>ies/20.tuple.cnstr.cpp?rev=675044&r1=675043&r2=675044&view=diff
>===============================================================
>===============
>--- stdcxx/branches/4.3.x/tests/utilities/20.tuple.cnstr.cpp (original)
>+++ stdcxx/branches/4.3.x/tests/utilities/20.tuple.cnstr.cpp 
>Tue Jul  8 16:13:36 2008
>@@ -32,8 +32,11 @@
> #if    !defined (_RWSTD_NO_EXT_CXX_0X) \
>     && !defined(_RWSTD_NO_RVALUE_REFERENCES)
> 
>+#include <cstring>              // for strcmp
> #include <tuple>
> 
>+#include <rw_valcmp.h>          // for rw_dblcmp
>+
> #include "20.tuple.h"
> 
> 
>/**************************************************************
>************/
>@@ -46,7 +49,7 @@
>     EmptyTuple et; _RWSTD_UNUSED (et);
>     IntTuple it; _RWSTD_UNUSED (it);
>     ConstIntTuple ct; _RWSTD_UNUSED (ct);
>-    //IntRefTuple rt; _RWSTD_UNUSED (rt); // ill-formed for references
>+    // ill-formed for tuples with element types containing references
>     PairTuple pt; _RWSTD_UNUSED (pt);
>     NestedTuple nt; _RWSTD_UNUSED (nt);
>     BigTuple bt; _RWSTD_UNUSED (bt);
>@@ -57,80 +60,174 @@
>     rw_assert (1 == UserClass::n_total_def_ctor_, __FILE__, __LINE__,
>                "tuple<UserClass>::tuple() called %d default ctors, "
>                "expected 1", UserClass::n_total_def_ctor_);
>-    rw_assert (0 == UserClass::n_total_copy_ctor_, __FILE__, __LINE__,
>-               "tuple<UserClass>::tuple() called %d copy ctors, "
>-               "expected 0", UserClass::n_total_copy_ctor_);
> }
> 
> 
>/**************************************************************
>************/
> 
>+#define INT_VALUE       int ('a')
>+
>+// assume that std::get() has been fully tested and works correctly
>+#define VERIFY_TUPLE(it) \
>+    rw_assert (std::get<0> (it) == INT_VALUE, __FILE__, __LINE__, \
>+               "std::get<0> (" #it "), got %d, expected %d", \
>+               std::get<0> (it), INT_VALUE);
>+
>+
>+#define LONG_VALUE      INT_VALUE
>+#define STRING_VALUE    "string"
>+
> static void
>-test_value_copy_ctor ()
>+verify_tuple (const PairTuple& pt)
> {
>-    rw_info (0, __FILE__, __LINE__, "value copy constructor");
>+    rw_assert (std::get<0> (pt) == LONG_VALUE, __FILE__, __LINE__,
>+               "std::get<0> (pt), got %d, expected %d",
>+               std::get<0> (pt), LONG_VALUE);
>+    rw_assert (0 == std::strcmp (std::get<1> (pt), STRING_VALUE),
>+               __FILE__, __LINE__,
>+               "std::get<1> (pt), got %s, expected %s",
>+               std::get<1> (pt), STRING_VALUE);
>+}

Is there any way that we could write a routine to generate a tuple and
then test it, so as to avoid always using the same few types and values
hidden behind the *_VALUE macros? The usage would be something like
this...

  TEST_TUPLE (1, 3.14f, 'a', "abc");

>-    int i = 1;
>-    IntTuple it1 (i); _RWSTD_UNUSED (it1);
>-    const IntTuple it2 (i); _RWSTD_UNUSED (it2);
>-    ConstIntTuple ct (i); _RWSTD_UNUSED (ct);
>-    IntRefTuple rt (i); _RWSTD_UNUSED (rt);
> 
>-    NestedTuple nt (it2); _RWSTD_UNUSED (nt);
>+#define USER_VALUE      user_val

I'm being a nit-picker, but this seems like an awful simple thing to be
wrapping a macro around. Is there a reason to do so?

>Modified: stdcxx/branches/4.3.x/tests/utilities/20.tuple.creation.cpp
>URL: 
>http://svn.apache.org/viewvc/stdcxx/branches/4.3.x/tests/utilit
>ies/20.tuple.creation.cpp?rev=675044&r1=675043&r2=675044&view=diff
>===============================================================
>===============
>--- 
>stdcxx/branches/4.3.x/tests/utilities/20.tuple.creation.cpp (original)
>+++ 
>stdcxx/branches/4.3.x/tests/utilities/20.tuple.creation.cpp 
>Tue Jul  8 16:13:36 2008
>@@ -63,11 +63,23 @@
> 
> 
>/**************************************************************
>************/
> 
>+#include <cstring>
>+
> static void
> test_tie ()
> {
>     rw_info (0, __FILE__, __LINE__, "tie");
> 
>+    int i = 0; double d = 0.0; const char* s = 0;
>+    std::tie (i, std::ignore, s)
>+        = std::make_tuple (256, 3.14159, "string");
>+
>+    rw_assert (i == 256, __FILE__, __LINE__,
>+               "i == 256, got false, expected true");
>+    rw_assert (d == 0.0, __FILE__, __LINE__,
>+               "d == 0.0, got false, expected true");
>+    rw_assert (0 == std::strcmp (s, "string"), __FILE__, __LINE__,
>+               "s == \"string\", got false, expected true");

The tuple is holding the original pointer (not a copy), so I think you
can check the actual pointer here.



>

Mime
View raw message