You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@stdcxx.apache.org by Travis Vitek <tv...@quovadx.com> on 2007/08/10 23:01:55 UTC
[PATCH] Update test 22.locale.num.put.mt.cpp to validate results [take 2]
Attached is a patch to enhance the num_put facet mt test. Threads
verify that the values they put compare equal to those put in the
primary thread.
2007-08-10 Travis Vitek <vi...@roguewave.com>
* 22.locale.num.put.mt.cpp(MyIos, MyStreambuf, MyNumData):
Added structures to simplify testing.
(run_test): Build a table of in/outputs for verification in thest
threads.
(thread_func): Assert the data written matches expected
(main): Add command line options for specifying number
of locales, locale names, and locale usage.
Re: [PATCH] Update test 22.locale.num.put.mt.cpp to validate results
[take 2]
Posted by Martin Sebor <se...@roguewave.com>.
Travis Vitek wrote:
>
> Martin Sebor wrote:
>>>
>>> +#define countof(x) (sizeof (x) / sizeof (*x))
>> Since you seem to like it so much ;-) we might as well move this
>> macro to some central test suite header (and rename it according
>> to the naming convention).
>>
>
> I'd love to, provided that we can define the naming convention to be
> used for macros in the test suite. The style guide I've seen has
> guidelines for names in the stdcxx implementation. I wouldn't think that
> we would want the same convention used for macros defined in the
> implementation as for macros defined within the test harness.
>
> The only macros that appear to be named consistently are the header
> guards. The names of other macros are pretty inconsistent. I'm guessing
> that you want it to be underscore seperated uppercase, which is fine by
> me. Some of the macros match this convention, of course many of them
> don't. Some start with RW, others with _RWSTD or _TEST, and quite a few
> of them don't have a prefix at all.
You're right, macros defined by the test driver don't follow
a consistent naming convention. The rest of the test driver
symbols use the "rw_" prefix for public (exported) names and
"_rw_" for internal symbols. I'd go with "RW_" for macros
(there's no need to uglify them by prepending an underscore
-- that's only necessary in the library).
Martin
RE: [PATCH] Update test 22.locale.num.put.mt.cpp to validate results [take 2]
Posted by Travis Vitek <tv...@quovadx.com>.
Martin Sebor wrote:
>
>>
>> +#define countof(x) (sizeof (x) / sizeof (*x))
>
>Since you seem to like it so much ;-) we might as well move this
>macro to some central test suite header (and rename it according
>to the naming convention).
>
I'd love to, provided that we can define the naming convention to be
used for macros in the test suite. The style guide I've seen has
guidelines for names in the stdcxx implementation. I wouldn't think that
we would want the same convention used for macros defined in the
implementation as for macros defined within the test harness.
The only macros that appear to be named consistently are the header
guards. The names of other macros are pretty inconsistent. I'm guessing
that you want it to be underscore seperated uppercase, which is fine by
me. Some of the macros match this convention, of course many of them
don't. Some start with RW, others with _RWSTD or _TEST, and quite a few
of them don't have a prefix at all.
How does adding _TEST_COUNT_OF(x) to the bottom of testdefs.h sound?
Travis
Re: [PATCH] Update test 22.locale.num.put.mt.cpp to validate results
[take 2]
Posted by Martin Sebor <se...@roguewave.com>.
Travis Vitek wrote:
>
[...]
>>> + TEST_N (data.bool_, bool, data.value_ != 0);
>>> + TEST_N (data.long_, long, data.value_);
>>> + TEST_N (data.ulong_, unsigned long, data.value_);
>> I note you've changed the test from invoking one num_put member
>> per iteration to invoking all members each iteration. I'm curious
>> about your rationale for the change? (I don't necessarily disagree
>> with the approach, just wondering what if anything led you to make
>> the change.) FWIW, the one advantage to sticking with the original
>> testing strategy is that it would let us eliminate the call to
>> pubsetp() and (possibly) do away with the macros.
>
> I'm missing something here. I don't see how we can eliminate the
> pubsetp() calls. If we did, then the next insertion operation will
> insert after the previous one. Instead of writing to the front of the
> buffer every time, we'd be appending to it. At some point an insertion
> will fail because the put pointer is at the end. Am I wrong?
You're right. I meant that if called put() only once each iteration
like we did before we would only need to call pubsetp() once as well.
Like so:
for (opt_nloops) {
buf.pubsetp (...);
switch (overload_to_call) {
case bool_overload: np.put ((bool)value); break;
case int_overload: np.put ((int)value); break;
case dbl_overload: np.put ((double)value); break;
case ptr_overload: np.put ((void*)value); break;
}
RW_ASSERT (postconditions);
}
Martin
RE: [PATCH] Update test 22.locale.num.put.mt.cpp to validate results [take 2]
Posted by Travis Vitek <tv...@quovadx.com>.
>Martin Sebor wrote:
>
>Travis Vitek wrote:
>> Attached is a patch to enhance the num_put facet mt test. Threads
>> verify that the values they put compare equal to those put in the
>> primary thread.
>
>There are few outstanding issues here that we need to resolve before
>committing this patch...
>
>[...]
>> + for (int i = 0; i != rw_opt_nloops; ++i) {
>>
>> - io.width (i % 16);
>> + // fill in the value and results for this locale
>> + const MyNumData& data = my_num_data [i % nlocales];
>>
>> - // exercise postive and negative values
>> - const int ival = i & 1 ? -i : i;
>> -
>> + // construct a named locale and imbue it in the ios object
>> + // so that the locale is used not only by the num_put facet
>> + const std::locale loc =
>> + rw_opt_shared_locale ? data.locale_
>> + : std::locale (data.locale_name_);
>> if (test_char) {
>> // exercise the narrow char specialization of the facet
>>
>> const std::num_put<char> &np =
>> std::use_facet<std::num_put<char> >(loc);
>>
>> - const std::ostreambuf_iterator<char> iter (&sb);
>> + nio.imbue (loc);
>
>Calling imbue() on the ios object before calling rdbuf() on it prevents
>the same locale from being imbued in the streambuf. Did you intend for
>that to happen?
>
No. I will double-check this is working as intended and update the
necessary tests if it is not.
>>
>[...]
>> +#define TEST(sb, buf, cmp, io, p, fill, valueT, val, charT) \
>> + sb.pubsetp (buf, countof (buf)); \
>> + io.rdbuf (&sb); \
>
>Is the call to rdbuf() necessary for every test or can it be done
>just once per thread? (Preferably before calling imbue() on the ios
>object.)
>
The rdbuf() isn't necessary every call. I'll update to call imbue() on
the ios object one time per iteration.
>> + *p.put (std::ostreambuf_iterator<charT>(&sb), \
>> + io, fill, (valueT)(val)) = charT (); \
>> + RW_ASSERT (!io.fail ()); \
>> + RW_ASSERT (!rw_strncmp (buf, cmp));
>>
>> - case put_long:
>> - np.put (iter, io, ' ', long (ival));
>> - break;
>> +#define TEST_N(o, t, v) \
>> + TEST(nsb, ncs, o.ncs_, nio, np, ' ', t, v, char)
>>
>> - case put_ulong:
>> - np.put (iter, io, ' ', (unsigned long)ival);
>> - break;
>> -
>> + TEST_N (data.bool_, bool, data.value_ != 0);
>> + TEST_N (data.long_, long, data.value_);
>> + TEST_N (data.ulong_, unsigned long, data.value_);
>
>I note you've changed the test from invoking one num_put member
>per iteration to invoking all members each iteration. I'm curious
>about your rationale for the change? (I don't necessarily disagree
>with the approach, just wondering what if anything led you to make
>the change.) FWIW, the one advantage to sticking with the original
>testing strategy is that it would let us eliminate the call to
>pubsetp() and (possibly) do away with the macros.
I'm missing something here. I don't see how we can eliminate the
pubsetp() calls. If we did, then the next insertion operation will
insert after the previous one. Instead of writing to the front of the
buffer every time, we'd be appending to it. At some point an insertion
will fail because the put pointer is at the end. Am I wrong?
The original test had dummy streambufs that did no actual insertion, so
this wasn't an issue.
>>
>[...]
>> +#define SETUP(sb, buf, io, p, fill, valueT, val, charT, loc) \
>> + sb.pubsetp (buf, countof (buf)); \
>> + io.rdbuf (&sb); \
>> + *p.put (std::ostreambuf_iterator<charT>(&sb), \
>> + io, fill, (valueT)(val)) = charT (); \
>> + rw_assert (!io.fail (), __FILE__, __LINE__, \
>
>I don't think rw_assert() is appropriate here. If the facet fails
>to format the "master" value into the buffer here the rest of the
>test is most likely hosed anyway and shouldn't even be attempted.
>Wouldn't rw_fatal() be a better choice?
Agreed
>
>Also, if you'd like to get rid of the macro (I would :), it should
>be possible to move the call to rdbuf() outside the loop and verify
>that the formatting was successful only once per iteration.
>
Done.
Re: [PATCH] Update test 22.locale.num.put.mt.cpp to validate results
[take 2]
Posted by Martin Sebor <se...@roguewave.com>.
Travis Vitek wrote:
> Attached is a patch to enhance the num_put facet mt test. Threads
> verify that the values they put compare equal to those put in the
> primary thread.
There are few outstanding issues here that we need to resolve before
committing this patch...
>
[...]
> @@ -58,150 +64,129 @@
[...]
>
> -#ifndef _RWSTD_NO_WCHAR_T
> + // the value that we will be formatting
> + int value_;
I suggest to make the type of the variable double to make it possible
to exercise the formatting of the fractional part of the value (for
floating point types).
>
> - struct WideBuf: std::wstreambuf {
> - int_type overflow (int_type c) { return c; }
> - } wb;
> + // holds the narrow/wide character representation of value_ and
> + // the number of used 'charT' in each buffer.
> + struct result {
> + char ncs_ [BufferSize];
> + std::char_traits<char>::off_type nlen_;
This member doesn't appear to be used in the test.
>
> -#endif // _RWSTD_NO_WCHAR_T
> + wchar_t wcs_ [BufferSize];
> + std::char_traits<wchar_t>::off_type wlen_;
Also unused.
> + };
>
> - struct Ios: std::ios {
> - Ios () { this->init (0); }
> - } io;
> + result bool_;
> + result long_;
> + result ulong_;
> + result llong_;
> + result ullong_;
> + result double_;
> + result ldouble_;
> + result pointer_;
FYI: the naming convention we've been trying to follow for names
referring to fundamental types is the one used by the C numeric
limits macros: i.e., "dbl" for double and "ldbl" for long double.
For pointers, we've been using "ptr." It comes in handy when using
the preprocessor when parametrizing functions on these types (and
when interacting with the test driver and command line options).
>
[...]
> +#define countof(x) (sizeof (x) / sizeof (*x))
Since you seem to like it so much ;-) we might as well move this
macro to some central test suite header (and rename it according
to the naming convention).
>
[...]
> + for (int i = 0; i != rw_opt_nloops; ++i) {
>
> - io.width (i % 16);
> + // fill in the value and results for this locale
> + const MyNumData& data = my_num_data [i % nlocales];
>
> - // exercise postive and negative values
> - const int ival = i & 1 ? -i : i;
> -
> + // construct a named locale and imbue it in the ios object
> + // so that the locale is used not only by the num_put facet
> + const std::locale loc =
> + rw_opt_shared_locale ? data.locale_
> + : std::locale (data.locale_name_);
> if (test_char) {
> // exercise the narrow char specialization of the facet
>
> const std::num_put<char> &np =
> std::use_facet<std::num_put<char> >(loc);
>
> - const std::ostreambuf_iterator<char> iter (&sb);
> + nio.imbue (loc);
Calling imbue() on the ios object before calling rdbuf() on it prevents
the same locale from being imbued in the streambuf. Did you intend for
that to happen?
>
[...]
> +#define TEST(sb, buf, cmp, io, p, fill, valueT, val, charT) \
> + sb.pubsetp (buf, countof (buf)); \
> + io.rdbuf (&sb); \
Is the call to rdbuf() necessary for every test or can it be done
just once per thread? (Preferably before calling imbue() on the ios
object.)
> + *p.put (std::ostreambuf_iterator<charT>(&sb), \
> + io, fill, (valueT)(val)) = charT (); \
> + RW_ASSERT (!io.fail ()); \
> + RW_ASSERT (!rw_strncmp (buf, cmp));
>
> - case put_long:
> - np.put (iter, io, ' ', long (ival));
> - break;
> +#define TEST_N(o, t, v) \
> + TEST(nsb, ncs, o.ncs_, nio, np, ' ', t, v, char)
>
> - case put_ulong:
> - np.put (iter, io, ' ', (unsigned long)ival);
> - break;
> -
> + TEST_N (data.bool_, bool, data.value_ != 0);
> + TEST_N (data.long_, long, data.value_);
> + TEST_N (data.ulong_, unsigned long, data.value_);
I note you've changed the test from invoking one num_put member
per iteration to invoking all members each iteration. I'm curious
about your rationale for the change? (I don't necessarily disagree
with the approach, just wondering what if anything led you to make
the change.) FWIW, the one advantage to sticking with the original
testing strategy is that it would let us eliminate the call to
pubsetp() and (possibly) do away with the macros.
[...]
> @@ -266,23 +229,126 @@
>
> /**************************************************************************/
>
> +
> static int
> run_test (int, char**)
> {
[...]
> + const char* const possible_locale_options[] = {
> + locale_list, "C\0", 0
> + };
Same applies as in this post:
http://www.mail-archive.com/stdcxx-dev@incubator.apache.org/msg04274.html
>
[...]
> +#define SETUP(sb, buf, io, p, fill, valueT, val, charT, loc) \
> + sb.pubsetp (buf, countof (buf)); \
> + io.rdbuf (&sb); \
> + *p.put (std::ostreambuf_iterator<charT>(&sb), \
> + io, fill, (valueT)(val)) = charT (); \
> + rw_assert (!io.fail (), __FILE__, __LINE__, \
I don't think rw_assert() is appropriate here. If the facet fails
to format the "master" value into the buffer here the rest of the
test is most likely hosed anyway and shouldn't even be attempted.
Wouldn't rw_fatal() be a better choice?
Also, if you'd like to get rid of the macro (I would :), it should
be possible to move the call to rdbuf() outside the loop and verify
that the formatting was successful only once per iteration.
[...]
> + // avoid divide by zero in thread if there are no locales to test
> + rw_fatal (nlocales != 0, 0, __LINE__,
> + "failed to create one or more usable locales!");
Assuming we change rw_locales() to always return at least "C" this
should never happen, right? (I.e., we'll be able to eliminate the
call to rw_fatal() after the change.)
Martin