incubator-stdcxx-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Martin Sebor <se...@roguewave.com>
Subject Re: test for lib.string.io
Date Thu, 29 Jun 2006 22:29:11 GMT
Anton Pevtsov wrote:
> The updated 21.string.io test with required changes to the test driver
> is here:
> http://people.apache.org/~antonp/stdcxx06292006/

I noticed _rw_sigcat() simply appends the string "istream" or
"ostream" rather than the fully expanded template name. I think
we should format them the same way as string, i.e., "istream" when
charT=char and Traits=char_traits<char>, "basic_istream<char>" when
Traits=char_traits<charT>, and "basic_istream<charT, Traits>"
otherwise, with charT and Traits expanded to the actual type, of
course).

Also, I think inserter and extractor (rather than op_in and op_out)
would better names for the command line options. And similarly for
the StringIds constants.

Assuming you agree with these suggestions please go ahead and commit
the changes to the driver (in a separate check-in).

I also have a few comments on the test itself. Feel free to commit
the test as it is for now (w/o making the changes I propose below).
We can make some or all of the changes in a subsequent commit.

The name STR_SPACES suggests that it stands for a string consisting
of 2 or more spaces when it actually is a string consisting of one
of each of the whitespace characters (in the "C" locale). I suggest
to rename it to WHITESPACE.

The macros _RWSTD_SIZE_T, _RWSTD_STREAMSIZE, etc. should only be used
in library and general test suite headers to avoid namespace pollution.
Tests should use the standard names wherever possible.

The macro _RWSTD_STATIC_CAST() should be used instead of the C++
keyword static_cast.

In general it's not safe to assume that std::ctype<T> is defined for
any T other than char and wchar_t so the Ctype facet should not derive
from it. Btw., it might make sense to define Ctype as a template and
specialize if for char, wchar_t, and UserChar, similarly to UserTraits.
That would let us test whether it is used irrespective of char_type
and allow us to exercise the behavior of the functions in non-C locales
(where whitespace might include other characters besides " \f\n\r\t\v").
This will be useful in other tests exercising iostreams.

When installing the Ctype facet in a locale it's more efficient to
create the facet on the stack instead of dynamically allocating it
on the heap. I.e., I suggest

     Ctype ctyp;
     Istream is (&inbuf);
     Ostream os (&outbuf);

     is.imbue (std::locale (is.getloc (), &ctyp));
     os.imbue (is.getloc ());

instead of

     // add facet std::ctype<UserChar>
     is.imbue (std::locale (is.getloc (), new Ctype));
     os.imbue (std::locale (os.getloc (), new Ctype));

Martin

Mime
View raw message