Re: svn commit: r675315 - /stdcxx/branches/4.2.x/tests/utilities/20.operators.cpp
[EMAIL PROTECTED] wrote: Author: elemings Date: Wed Jul 9 12:16:56 2008 New Revision: 675315 URL: http://svn.apache.org/viewvc?rev=675315&view=rev Log: 2008-07-09 Eric Lemings <[EMAIL PROTECTED]> STDCXX-550 * tests/utilities/20.operators.cpp (test_random_access_iterator): Oops. Should be `!defined' in #if directive. Modified: stdcxx/branches/4.2.x/tests/utilities/20.operators.cpp Modified: stdcxx/branches/4.2.x/tests/utilities/20.operators.cpp URL: http://svn.apache.org/viewvc/stdcxx/branches/4.2.x/tests/utilities/20.operators.cpp?rev=675315&r1=675314&r2=675315&view=diff == --- stdcxx/branches/4.2.x/tests/utilities/20.operators.cpp (original) +++ stdcxx/branches/4.2.x/tests/utilities/20.operators.cpp Wed Jul 9 12:16:56 2008 @@ -397,7 +397,7 @@ typedef RandomAccessIterator I; -#if defined _RWSTD_NO_DEBUG_ITER +#if !defined _RWSTD_NO_DEBUG_ITER RandomNumberGenerator rndgen; This won't compile when RandomAccessIterator is a plain pointer, which both string::iterator and vector::iterator happen to be when _RWSTD_NO_DEBUG_ITER is #defined (i.e., with optimization). Martin
Re: Conflicting identifiers
Eric Lemings wrote: Getting the following compile error due to conflicting __rw_is_same identifiers: Right. We need to either rename one of them, or get rid of and start using the new traits instead. Until the type traits implementation is ported to all platforms I've opted for the former approach in r675390. Martin 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.cnstr.cpp In file included from /work/stdcxx/branches/4.3.x/include/string:42, from /work/stdcxx/branches/4.3.x/tests/utilities/20.tuple.cnstr.cpp:417: /work/stdcxx/branches/4.3.x/include/rw/_select.h:103: error: redefinition of 'struct __rw::__rw_is_same<_TypeT, _TypeU>' /work/stdcxx/branches/4.3.x/include/rw/_meta_rel.h:52: error: previous definition of 'struct __rw::__rw_is_same<_TypeT, _TypeU>' /work/stdcxx/branches/4.3.x/include/rw/_select.h:110: error: redefinition of 'struct __rw::__rw_is_same<_TypeT, _TypeT>' /work/stdcxx/branches/4.3.x/include/rw/_meta_rel.h:57: error: previous definition of 'struct __rw::__rw_is_same<_TypeT, _TypeT>' make: *** [20.tuple.cnstr.o] Error 1 Brad.
Conflicting identifiers
Getting the following compile error due to conflicting __rw_is_same identifiers: 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.cnstr.cpp In file included from /work/stdcxx/branches/4.3.x/include/string:42, from /work/stdcxx/branches/4.3.x/tests/utilities/20.tuple.cnstr.cpp:417: /work/stdcxx/branches/4.3.x/include/rw/_select.h:103: error: redefinition of 'struct __rw::__rw_is_same<_TypeT, _TypeU>' /work/stdcxx/branches/4.3.x/include/rw/_meta_rel.h:52: error: previous definition of 'struct __rw::__rw_is_same<_TypeT, _TypeU>' /work/stdcxx/branches/4.3.x/include/rw/_select.h:110: error: redefinition of 'struct __rw::__rw_is_same<_TypeT, _TypeT>' /work/stdcxx/branches/4.3.x/include/rw/_meta_rel.h:57: error: previous definition of 'struct __rw::__rw_is_same<_TypeT, _TypeT>' make: *** [20.tuple.cnstr.o] Error 1 Brad.
Re: Static assertions in tests?
Travis Vitek wrote:
Martin Sebor wrote:
Eric Lemings wrote:
For compile-time tests, would it be preferable to use a
static assertion
or continue using good ol' rw_assert() even for compile-time
checks? In
the former case, the test will fail to build and, in the latter case,
the compile-time check will not be easily distinguisable from other
runtime assertions.
I would recommend against using one component of the library
(static_assert) to test another.
The approach taken by existing tests is to verify types and
signatures by using them in ways that would make the tests
ill-formed if they didn't match the requirements, causing
a compiler error. You can see examples of this approach in
the 23.vector.cons.cpp test that was just mentioned.
I happen to use this trick ..
typedef char assert_0 [(cond) ? 1 : -1];
I think we're talking about two different things. I assumed
Brad was asking about tests to check things like the expected
signatures of functions, such as the signature of std::get()
below:
std::tuple t;
std::get<0>(t);
My approach to writing a test for this would be along these
lines:
#include
#include
#include
template
void check_get_signature ()
{
ExpectSig* const sig = &std::get;
_RWSTD_UNUSED (sig);
}
void test_get_signatures ()
{
#define TEST(N, Tuple, RetT) \
check_get_signature&)>()
TEST (0, std::tuple, const int&);
TEST (1, std::tuple, char*&);
...
}
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
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
#include
int main ()
{
const char* s = "string";
{
std::tuple x (s);
const char* const y = std::get<0>(x);
assert (y == s);
}
{
std::tuple x (0, s);
const char* const y = std::get<1>(x);
assert (y == s);
}
}
RE: Static assertions in tests?
Martin Sebor wrote: > >Eric Lemings wrote: >> >> For compile-time tests, would it be preferable to use a >static assertion >> or continue using good ol' rw_assert() even for compile-time >checks? In >> the former case, the test will fail to build and, in the latter case, >> the compile-time check will not be easily distinguisable from other >> runtime assertions. > >I would recommend against using one component of the library >(static_assert) to test another. > >The approach taken by existing tests is to verify types and >signatures by using them in ways that would make the tests >ill-formed if they didn't match the requirements, causing >a compiler error. You can see examples of this approach in >the 23.vector.cons.cpp test that was just mentioned. > I happen to use this trick .. typedef char assert_0 [(cond) ? 1 : -1]; >Martin >
Re: Static assertions in tests?
Eric Lemings wrote: For compile-time tests, would it be preferable to use a static assertion or continue using good ol' rw_assert() even for compile-time checks? In the former case, the test will fail to build and, in the latter case, the compile-time check will not be easily distinguisable from other runtime assertions. I would recommend against using one component of the library (static_assert) to test another. The approach taken by existing tests is to verify types and signatures by using them in ways that would make the tests ill-formed if they didn't match the requirements, causing a compiler error. You can see examples of this approach in the 23.vector.cons.cpp test that was just mentioned. Martin
Re: Initialize by throwing an exception?
Eric Lemings wrote:
tests/containers/23.vector.cons.cpp:
...
685 #ifndef _RWSTD_NO_EXCEPTIONS
686
687 try {
688 // throw an exception to initialize the lib (allocates
689 // memory that's never deallocated; shows up as leaks)
690 _RW::__rw_throw (_RWSTD_ERROR_LOGIC_ERROR, "", "");
691 }
692 catch (...) {
693 }
694
695 #endif // _RWSTD_NO_EXCEPTIONS
...
This seems very odd. Initializing the library by throwing an exception.
Or is this meant only to initialize the exception handling portions of
the library?
The latter. I suspect this might be a white box test for the
old (pre-3.0) version of the library. The new library shouldn't
leak anything. Feel free to remove this bit after verifying my
hypothesis :)
Martin
Re: Tests for vector(size_t) constructor?
Eric Lemings wrote: I don't see any tests for the vector(size_t) constructor anywhere. Am I not looking in the right place? I don't know. Where are you looking? ;-) The test that should exercise it is 23.vector.cons.cpp. I don't see the the vector::vector(size_type) ctor being exercised in there so unless there's another test for it hiding somewhere it's not being tested at all :( There are whole classes that we don't have tests for, so it wouldn't surprise me if this one little overload wasn't tested either. Martin
Latest working draft
FYI. http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2008/n2691.pdf
Static assertions in tests?
For compile-time tests, would it be preferable to use a static assertion or continue using good ol' rw_assert() even for compile-time checks? In the former case, the test will fail to build and, in the latter case, the compile-time check will not be easily distinguisable from other runtime assertions. 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
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
#include
static const char* s = "hello world";
int main ()
{
const std::tuple t (s);
assert (std::get<0>(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
> -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
> -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
> -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.
Tests for vector(size_t) constructor?
I don't see any tests for the vector(size_t) constructor anywhere. Am I not looking in the right place? Thanks, 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
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
Initialize by throwing an exception?
tests/containers/23.vector.cons.cpp:
...
685 #ifndef _RWSTD_NO_EXCEPTIONS
686
687 try {
688 // throw an exception to initialize the lib (allocates
689 // memory that's never deallocated; shows up as leaks)
690 _RW::__rw_throw (_RWSTD_ERROR_LOGIC_ERROR, "", "");
691 }
692 catch (...) {
693 }
694
695 #endif // _RWSTD_NO_EXCEPTIONS
...
This seems very odd. Initializing the library by throwing an exception.
Or is this meant only to initialize the exception handling portions of
the library?
Was hoping someone could shed some light on this.
Brad.
Re: svn commit: r675050 - in /stdcxx/branches/4.3.x: include/rw/ tests/utilities/
[EMAIL PROTECTED] wrote: Author: vitek Date: Tue Jul 8 16:25:19 2008 New Revision: 675050 Travis, you might want to check your Insert TABs settings in your text editor (there seem to be a number of TABs in this change). 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
> -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
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
> -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_VALUE"string"
+#define STRING_VALUEstr_value
+static const char* str_value = "string";
+
static void
verify_tuple (const PairTuple& pt)
{
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);
+rw_assert (std::get<1> (pt) == STRING_VALUE, __FILE__,
__LINE__,
+ "std::get<1> (pt), got %p \"%s\", expected %p
\"%s\"",
+ std::get<1> (pt), std::get<1> (pt),
+ STRING_VALUE, STRING_VALUE);
}
I get the following assertions:
...
# ASSERTION (S7) (5 lines):
# TEXT: std::get<1> (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::get<1> (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::get<1> (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::get<1> (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
> -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? 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
> -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
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
Travis Vitek wrote:
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 <[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 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
+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
-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
> -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.
