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/utiliti

2008-07-09 Thread Eric Lemings 
 

 -Original Message-
 From: Travis Vitek [mailto:[EMAIL PROTECTED] 
 Sent: Tuesday, July 08, 2008 6:04 PM
 To: dev@stdcxx.apache.org
 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
 
  
...
 
 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).

I've been using _TYPENAME, _EXPLICIT, and _RWSTD_SPECIALIZED_FUNCTION
just to be consistent with existing code.  Is there a good reason not to
do this anymore?  (Actually I can think of one but I'll see if anyone
else can think of it and/or another.)

 
...
 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);

This might work for homogenous tuples where the element types can be
deduced from the values.  Not sure exactly how you would fit
user-defined (e.g. UserClass) values into it.  Also, you'd need an
expanded form for heterogenous tuples where the compatible/convertible
types would have to be explicitly specified.

For this latest update, I really wanted to just get a complete set of
tests in there however verbose they may be.

 
 -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?

Like the other value macros, to hide the actual value being used and to
provide a single point of definition where it can be modified.

 
 Modified: stdcxx/branches/4.3.x/tests/utilities/20.tuple.creation.cpp
...
 +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.

True.  But if that assumption became invalid for whatever reason, the
code above would still work.

Assumptions are bad.  Robustness is good.  :)

Brad.

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/utiliti

2008-07-09 Thread Martin Sebor 

Travis Vitek wrote:
 


Author: elemings
Date: Tue Jul  8 16:13:36 2008
New Revision: 675044

URL: http://svn.apache.org/viewvc?rev=675044view=rev
Log:
2008-07-08  Eric Lemings [EMAIL PROTECTED]

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 VV 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=675044r1=675043r2=675044view=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.


I agree. Without a name, it's obvious that the parameter
is unused. Saying it's unused in a comment is like adding
a /* return; */ comment to the end of void functions, or
adding an /* extern */ in front of the definition of non
member function definitions. IMO, all of these represent
unnecessary redundancies that are liable to make readers
wonder about their purpose rather than providing any
helpful insight. It would be much more helpful to document
the purpose of the unusual assignment operator than the
unused argument :)



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.


Good idea! Also, the inline specifier is redundant and should
be removed.




[...]

@@ -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?


I agree. We can start using typename and explicit in all new
code on 4.3.x and replace _TYPENAME and _EXPLICIT with the
real keywords. Ditto for _RWSTD_SPECIALIZED_FUNCTION and
_RWSTD_SPECIALIZED_CLASS.




[...]

@@ -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;


Right. We're not 100% consistent (and I can't say I'm crazy
about this style, either) but I find it more readable than
any of the alternatives I've seen.

Martin

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/utiliti

2008-07-09 Thread Travis Vitek 
 

Eric Lemings wrote:

 Travis Vitek wrote:

 Modified: 
stdcxx/branches/4.3.x/tests/utilities/20.tuple.creation.cpp
...
 +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.

True.  But if that assumption became invalid for whatever reason, the
code above would still work.

Assumptions are bad.  Robustness is good.  :)

As I see it, the tuple implementation is required to hold a copy of an
object of the specified type (const char* in this case). If you don't
verify the value held is indeed a copy, you are not actually verifying
the requirements. This is wrong, and wrong is much worse than bad. :)


Brad.

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/utiliti

2008-07-09 Thread Eric Lemings 
 

 -Original Message-
 From: Travis Vitek [mailto:[EMAIL PROTECTED] 
 Sent: Wednesday, July 09, 2008 12:28 PM
 To: dev@stdcxx.apache.org
 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
 
  
 
 Eric Lemings wrote:
 
  Travis Vitek wrote:
 
  Modified: 
 stdcxx/branches/4.3.x/tests/utilities/20.tuple.creation.cpp
 ...
  +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.
 
 True.  But if that assumption became invalid for whatever reason, the
 code above would still work.
 
 Assumptions are bad.  Robustness is good.  :)
 
 As I see it, the tuple implementation is required to hold a copy of an
 object of the specified type (const char* in this case). If you don't
 verify the value held is indeed a copy, you are not actually verifying
 the requirements. This is wrong, and wrong is much worse than bad. :)

Good point.

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/utiliti

2008-07-09 Thread Eric Lemings 
 

 -Original Message-
 From: Eric Lemings [mailto:[EMAIL PROTECTED] 
 Sent: Wednesday, July 09, 2008 12:40 PM
 To: dev@stdcxx.apache.org
 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
 
  
...
  
  As I see it, the tuple implementation is required to hold a 
 copy of an
  object of the specified type (const char* in this case). If 
 you don't
  verify the value held is indeed a copy, you are not 
 actually verifying
  the requirements. This is wrong, and wrong is much worse 
 than bad. :)
 
 Question:
 
 const char* s1 = string;
 const char* s2 = string;
 // s1 guaranteed to equal s2?

With the following change:

Index:
/work/stdcxx/branches/4.3.x/tests/utilities/20.tuple.cnstr.cpp

===
---
/work/stdcxx/branches/4.3.x/tests/utilities/20.tuple.cnstr.cpp
(revision 675050)
+++
/work/stdcxx/branches/4.3.x/tests/utilities/20.tuple.cnstr.cpp
(working copy)
@@ -74,18 +73,20 @@


 #define LONG_VALUE  INT_VALUE
-#define STRING_VALUEstring
+#define STRING_VALUEstr_value

+static const char* str_value = string;
+
 static void
 verify_tuple (const PairTuple pt)
 {
 rw_assert (std::get0 (pt) == LONG_VALUE, __FILE__,
__LINE__,
std::get0 (pt), got %d, expected %d,
std::get0 (pt), LONG_VALUE);
-rw_assert (0 == std::strcmp (std::get1 (pt),
STRING_VALUE),
-   __FILE__, __LINE__,
-   std::get1 (pt), got %s, expected %s,
-   std::get1 (pt), STRING_VALUE);
+rw_assert (std::get1 (pt) == STRING_VALUE, __FILE__,
__LINE__,
+   std::get1 (pt), got %p \%s\, expected %p
\%s\,
+   std::get1 (pt), std::get1 (pt),
+   STRING_VALUE, STRING_VALUE);
 }

I get the following assertions:

...
# ASSERTION (S7) (5 lines):
# TEXT: std::get1 (pt), got 0f18d8c0 string,
expected 0042796e string
# CLAUSE: [tuple.cnstr]
# FILE: 20.tuple.cnstr.cpp
# LINE: 86

# INFO (S1) (5 lines):
# TEXT: move constructor (heterogenous tuples)
# CLAUSE: [tuple.cnstr]
# FILE: 20.tuple.cnstr.cpp
# LINE: 458

# ASSERTION (S7) (5 lines):
# TEXT: std::get1 (pt), got 0f18d8c0 string,
expected 0042796e string
# CLAUSE: [tuple.cnstr]
# FILE: 20.tuple.cnstr.cpp
# LINE: 86

# INFO (S1) (5 lines):
# TEXT: copy assignment operator (heterogenous tuples)
# CLAUSE: [tuple.cnstr]
# FILE: 20.tuple.cnstr.cpp
# LINE: 480

# ASSERTION (S7) (5 lines):
# TEXT: std::get1 (pt), got 0f18d8c0 string,
expected 0042796e string
# CLAUSE: [tuple.cnstr]
# FILE: 20.tuple.cnstr.cpp
# LINE: 86

# INFO (S1) (5 lines):
# TEXT: move assignment operator (heterogenous tuples)
# CLAUSE: [tuple.cnstr]
# FILE: 20.tuple.cnstr.cpp
# LINE: 504

# ASSERTION (S7) (5 lines):
# TEXT: std::get1 (pt), got 0f18d8c0 string,
expected 0042796e string
# CLAUSE: [tuple.cnstr]
# FILE: 20.tuple.cnstr.cpp
# LINE: 86
...

It appears that pointer values are not guaranteed to be equal when
converting between pointer types.

Brad.

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/utiliti

2008-07-09 Thread Martin Sebor 

Eric Lemings wrote:
 


-Original Message-
From: Travis Vitek [mailto:[EMAIL PROTECTED] 
Sent: Wednesday, July 09, 2008 12:28 PM

To: dev@stdcxx.apache.org
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


 


Eric Lemings wrote:

Travis Vitek wrote:

Modified: 

stdcxx/branches/4.3.x/tests/utilities/20.tuple.creation.cpp
...
+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.

True.  But if that assumption became invalid for whatever reason, the
code above would still work.

Assumptions are bad.  Robustness is good.  :)

As I see it, the tuple implementation is required to hold a copy of an
object of the specified type (const char* in this case). If you don't
verify the value held is indeed a copy, you are not actually verifying
the requirements. This is wrong, and wrong is much worse than bad. :)


Question:

const char* s1 = string;
const char* s2 = string;
// s1 guaranteed to equal s2?


It's unspecified. The compiler is allowed to merge strings.
It's allowed to even go as far as to point s2 at (s1 + 1)
in the snippet below:

const char* s1 = Xstring;
const char* s2 = string;

Martin

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/utiliti

2008-07-09 Thread Eric Lemings 
 

 -Original Message-
 From: Martin Sebor [mailto:[EMAIL PROTECTED] 
 Sent: Wednesday, July 09, 2008 11:10 AM
 To: dev@stdcxx.apache.org
 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
 
...
  
  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.
 
 I agree. Without a name, it's obvious that the parameter
 is unused.

Examples in existing code:

The run_test() function in tests/containers/23.vector.cons.cpp.
Lines 56-64 in tests/containers/23.deque.modifiers.cpp.
The __rw_smanip member functions in include/iomanip.

Who did all that?  Not me.  :)  I'm sure there are plenty more examples.

Anyone care to search for all such cases and make it all consistent?

Brad.

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/utiliti

2008-07-09 Thread Martin Sebor 

Eric Lemings wrote:
 


-Original Message-
From: Martin Sebor [mailto:[EMAIL PROTECTED] 
Sent: Wednesday, July 09, 2008 11:10 AM

To: dev@stdcxx.apache.org
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



...
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.

I agree. Without a name, it's obvious that the parameter
is unused.


Examples in existing code:


As I said before, you can find examples of pretty much any
style, including two space indents. Are you purposely seeking
out these rare, obscure cases and adopting them in your code
just to make things interesting?



The run_test() function in tests/containers/23.vector.cons.cpp.


Not sure why the names are commented out. Maybe because
the author was intending to use them and didn't and they
got commented out to silence warnings.


Lines 56-64 in tests/containers/23.deque.modifiers.cpp.


They are there because normally, names local to each test
are declared static. In this test (and many others) they
can't be declared static because they are referenced from
template code and no all compilers find time (Sun C++ 5.3
has a bug that prevents it from finding static symbols
referenced from template code). So the /* extern */
comment is a reminder to prevent people from making them
static.


The __rw_smanip member functions in include/iomanip.


This is the { /* empty */ } comment that some style guides
suggest for non-trivial ctors with deliberately empty bodies
to indicate that the body wasn't left empty by accident when
the ctor was stubbed out early in the development of the
class. I don't feel strongly about using this style.



Who did all that?  Not me.  :)  I'm sure there are plenty more examples.


I'm sure there are.



Anyone care to search for all such cases and make it all consistent?


No. Please just adjust your code as suggested.

Thanks
Martin

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/utiliti

2008-07-09 Thread Eric Lemings 
 

 -Original Message-
 From: Martin Sebor [mailto:[EMAIL PROTECTED] 
 Sent: Wednesday, July 09, 2008 2:49 PM
 To: dev@stdcxx.apache.org
 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
 
 Eric Lemings wrote:
   
  
  -Original Message-
  From: Martin Sebor [mailto:[EMAIL PROTECTED] 
  Sent: Wednesday, July 09, 2008 11:10 AM
  To: dev@stdcxx.apache.org
  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
 
  ...
  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.
  I agree. Without a name, it's obvious that the parameter
  is unused.
  
  Examples in existing code:
 
 As I said before, you can find examples of pretty much any
 style, including two space indents. Are you purposely seeking
 out these rare, obscure cases and adopting them in your code
 just to make things interesting?

Actually no, if you believe that.  Was just providing examples since
Travis could find no such usage in existing code.

Brad.

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/utiliti

2008-07-09 Thread Eric Lemings 
 

 -Original Message-
 From: Martin Sebor [mailto:[EMAIL PROTECTED] 
 Sent: Wednesday, July 09, 2008 11:10 AM
 To: dev@stdcxx.apache.org
 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
 
...
  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.
 
 Good idea! Also, the inline specifier is redundant and should
 be removed.

A void return type causes an compile error:

gcc -c -I/work/stdcxx/branches/4.3.x/include/ansi -D_RWSTDDEBUG
-pthread -I/work/stdcxx/branches/4.3.x/include
-I/build/stdcxx-4.3.x-15D/include
-I/work/stdcxx/branches/4.3.x/tests/include  -pedantic -nostdinc++
-std=gnu++0x -D_RWSTD_EXT_CXX_0X -g   -W -Wall -Wcast-qual -Winline
-Wshadow -Wwrite-strings -Wno-long-long -Wcast-align
/work/stdcxx/branches/4.3.x/tests/utilities/20.tuple.creation.cpp
/work/stdcxx/branches/4.3.x/include/rw/_tuple.h: In member function
'void __rw::__rw_ignore::operator=(const _TypeT) const [with _TypeT =
double]':
/work/stdcxx/branches/4.3.x/include/rw/_tuple.h:131:   instantiated from
'__rw::__rw_tuple_HeadT, _TailT ... __rw::__rw_tuple_HeadT, _TailT
...::operator=(__rw::__rw_tuple_HeadU, _TailU ...) [with _HeadU =
double, _TailU = const char*, _HeadT = const __rw::__rw_ignore, _TailT
= const char*]'
/work/stdcxx/branches/4.3.x/include/rw/_tuple.h:130:   instantiated from
'__rw::__rw_tuple_HeadT, _TailT ... __rw::__rw_tuple_HeadT, _TailT
...::operator=(__rw::__rw_tuple_HeadU, _TailU ...) [with _HeadU =
int, _TailU = double, const char*, _HeadT = int, _TailT = const
__rw::__rw_ignore, const char*]'
/work/stdcxx/branches/4.3.x/include/tuple:123:   instantiated from
'std::tuple_TypesT std::tuple_TypesT::operator=(std::tuple_TypesU
...) [with _TypesU = int, double, const char*, _TypesT = int, const
__rw::__rw_ignore, const char*]'
/work/stdcxx/branches/4.3.x/tests/utilities/20.tuple.creation.cpp:75:
instantiated from here
/work/stdcxx/branches/4.3.x/include/rw/_tuple.h:181: error:
return-statement with a value, in function returning 'void'
make: *** [20.tuple.creation.o] Error 1

Brad.

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/utiliti

2008-07-09 Thread Eric Lemings 
 

 -Original Message-
 From: Eric Lemings 
 Sent: Wednesday, July 09, 2008 3:14 PM
 To: 'dev@stdcxx.apache.org'
 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
 
  
 
  -Original Message-
  From: Martin Sebor [mailto:[EMAIL PROTECTED] 
  Sent: Wednesday, July 09, 2008 11:10 AM
  To: dev@stdcxx.apache.org
  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
  
 ...
   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.
  
  Good idea! Also, the inline specifier is redundant and should
  be removed.
 
 A void return type causes an compile error:

Duh.  Disregard.

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/utiliti

2008-07-09 Thread Travis Vitek 
 

Martin Sebor wrote:

Eric Lemings wrote:
  
 
 Travis Vitek wrote:
  

 Eric Lemings wrote:
 Travis Vitek wrote:

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

 True.  But if that assumption became invalid for whatever
 reason, the code above would still work.

 Assumptions are bad.  Robustness is good.  :)

 As I see it, the tuple implementation is required to hold a 
 copy of an object of the specified type (const char* in this
 case). If you don't verify the value held is indeed a copy,
 you are not actually verifying the requirements. This is wrong,
 and wrong is much worse than bad. :)
 
 Question:
 
 const char* s1 = string;
 const char* s2 = string;
 // s1 guaranteed to equal s2?

It's unspecified. The compiler is allowed to merge strings.
It's allowed to even go as far as to point s2 at (s1 + 1)
in the snippet below:

 const char* s1 = Xstring;
 const char* s2 = string;


Just so we're clear, the case that should be happening with tuple is the
following...

  const char* s1 = string;
  const char* s2 (s1);

  assert (s1 == s2);

The following code works just fine with both the gnu tuple
implementation and ours. I'm not sure why the Brad is seeing the
assertion failure.

  $ cat t.cpp  g++ -std=gnu++0x t.cpp  ./a.out  echo good
  #include tuple
  #include assert.h

  static const char* s = hello world;

  int main ()
  {
const std::tupleconst char* t (s);
assert (std::get0(t) == s);

return 0;
  }
  $ good

  $ gmake t  ./t  echo good
  gcc -c -I/amd/devco/vitek/stdcxx/4.3.x/include/ansi -D_RWSTDDEBUG
-D_RWSTD_EXT_CXX_0X -I/amd/devco/vitek/stdcxx/4.3.x/include
-I/build/vitek/4.3.0/11S/include
-I/amd/devco/vitek/stdcxx/4.3.x/tests/include -pedantic -nostdinc++ -g
-std=gnu++0x -W -Wall -Wcast-qual -Winline -Wshadow -Wwrite-strings
-Wno-long-long -Wcast-align -Wno-empty-body -Wno-parentheses t.cpp
  gcc t.o -o t -L/build/vitek/4.3.0/11S/rwtest -lrwtest11S
-L/build/vitek/4.3.0/11S/lib -lstd11S -lsupc++ -lm
  good

Martin

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/utiliti

2008-07-09 Thread Martin Sebor 

Eric Lemings wrote:
 

[...]

With the following change:

Index:
/work/stdcxx/branches/4.3.x/tests/utilities/20.tuple.cnstr.cpp

===
---
/work/stdcxx/branches/4.3.x/tests/utilities/20.tuple.cnstr.cpp
(revision 675050)
+++
/work/stdcxx/branches/4.3.x/tests/utilities/20.tuple.cnstr.cpp
(working copy)
@@ -74,18 +73,20 @@

[...]

It appears that pointer values are not guaranteed to be equal when
converting between pointer types.


They must be. I see the assertions in the test but the small
program below works fine.

On an unrelated note, though: While working with the test
I found the heavy use of macros and globals quite confusing.
It makes it very difficult to find and track the tested
values, and virtually impossible to extend with new test
cases.

The approach I've found to work better is splitting up the
test into one or some other small number of worker functions
parametrized on all the arguments and expected values and
their types (if necessary) and other higher level functions,
for example one for each overload of the tested function,
with individual test cases and literal values of arguments
and expected results. You can find examples of such tests
in the tests/localization directory, such as all the
22.locale.{money,num,time}.{get,put}.cpp tests. I suggest
you follow this example in the tuple tests as well.

Martin

#include cassert
#include tuple

int main ()
{
const char* s = string;

{
std::tupleconst char* x (s);
const char* const y = std::get0(x);

assert (y == s);
}
{
std::tuplelong, const char* x (0, s);
const char* const y = std::get1(x);

assert (y == s);
}
}

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/utiliti

2008-07-08 Thread Travis Vitek 
 

Author: elemings
Date: Tue Jul  8 16:13:36 2008
New Revision: 675044

URL: http://svn.apache.org/viewvc?rev=675044view=rev
Log:
2008-07-08  Eric Lemings [EMAIL PROTECTED]

   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 VV 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=675044r1=675043r2=675044view=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=675044r1=675043r2=675044view=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*/)
 {