test for 21.string::replace

[Anton Pevtsov <an...@moscow.vdiweb.com>]

The attached file contains new version of the test for string::replace.
You might want to add tests cases to be sure that the test covers all
possible cases.

And there are two issues with this test:

1. It contains workaround for empty strings with UserChars formatting
output.
The cause is in the implementation of the %{#*S} directive: it tries to
cast basic_string<UserChar, ... > to string<char, ...> then gets
incorrect string size and finally fails. As a stub it is possible to
ignore "unknown" strings, but some callback for this case will be more
useful. What do you think about it?

2. There is no test case which results in the length_error exception
thrown by replace. The possible way to test this case requires the huge
string with length about SIZE_MAX / 2 and I think this not good idea. I
think we can try to use our own "string" class with modified size()
method to emulate this case, but will this be the appropriate test?


With best wishes,
Anton Pevtsov

Re: test for 21.string::replace

[Martin Sebor <se...@roguewave.com>]

Anton Pevtsov wrote:
> The attached file contains new version of the test for string::replace.

Excellent!

> You might want to add tests cases to be sure that the test covers all
> possible cases.

I'll look into it. I will definitely want to enhance the tests with
long strings, maybe even generate a few. This is the most important
function in all of basic_string.

> 
> And there are two issues with this test:
> 
> 1. It contains workaround for empty strings with UserChars formatting
> output.
> The cause is in the implementation of the %{#*S} directive: it tries to
> cast basic_string<UserChar, ... > to string<char, ...> then gets
> incorrect string size and finally fails. As a stub it is possible to
> ignore "unknown" strings, but some callback for this case will be more
> useful. What do you think about it?

I'm hoping to come up with a generic formatting directive that can be
used with all three types of strings: char*, wchar_t*, and UserChar*.
Something along the lines of the _rw_fmtxarrayv() function in
alg_test.cpp that does the formatting of arrays of X, but a little
bit more generic (i.e., with an additional argument telling the
function if it's dealing with char*, wchar_t*, or UserChar*).

> 
> 2. There is no test case which results in the length_error exception
> thrown by replace. The possible way to test this case requires the huge
> string with length about SIZE_MAX / 2 and I think this not good idea. I
> think we can try to use our own "string" class with modified size()
> method to emulate this case, but will this be the appropriate test?

I agree that trying to allocate huge amounts of memory is not a good
way to test the class. We can (and should), however, emulate low memory
conditions by setting a lower memory limit by calling setrlimit():
http://www.opengroup.org/onlinepubs/009695399/functions/getrlimit.html
That's something different than what you're talking about, though, as
that will induce a bad_alloc exception and not length_error.

It might be possible to explicitly specialize a member function of
basic_string (such as max_size) in the test to try to induce an
exception sooner. I believe some of our tests actually do that (I
have written them). Unfortunately, this technique is not safe since
the template is already instantiated in the library (both explicitly
as well as implicitly by virtue of being used in locale). Doing so
violates the One Definition Rule (one definition of the function is
in the library and another in the test) and may be detected by some
compilers (e.g., EDG eccp detects it in "export" mode).

A more reliable way to do almost the same thing is to exercise the
template on a user-defined traits or allocator type. The allocator
approach is better since basic_string::max_size() uses its
allocator_type::max_size(). The (theoretical) downside to this is
that if basic_string were explicitly specialized on char and wchar_t
in the library (as opposed to being explicitly instantiated as it is
now) the technique won't exercise the same same code in the char and
wchar_t case.

Please go ahead and commit it as it is and let's make improvements
(including those below) as the second pass.

One minor thing I'd like to adjust is the format of some of the
diagnostics. Instead of

# TEXT: line 354. std::basic_string<char, char_traits<char>, 
allocator<char>>("abc").replace (0, 2, "cc") == "ccdefgc", "ccdefgc"
expected

I would prefer to see

# TEXT: line 354. std::basic_string<char, char_traits<char>, 
allocator<char>>("abc").replace (0, 2, "cc") == "ccdefgc", got "ccdefgc"

I.e., the string on the right hand side of == should be the expected
result, with the actual result to follow after ", got"

I also think that it might be more space efficient (in terms of
a significantly reduced number of function calls) in this case to
create a data structure for each test case and build an array of
them to store them all and then simply iterate over the array one
element at a time. It should be easy to make this change and see
how much of a space difference it made.

Here's what I have in mind:

     static const struct {
         int line;
         const char *str;
         int         pos1;
         int         num1;
         const char* src;
         int         pos2;
         int         num2;
         int         cnt;
         int         ch;
         const char* res;
         int         it_res;
         int         bthrow;
     } test_cases[] = {
#define TEST(str, pos1, num1, src, pos2, num2, cnt, \
             ch, res, it_res, bthrow) \
      { __LINE__, str, pos1, num1, src, pos2, num2, cnt,  \
        ch, res, it_res, bthrow }

         TEST ("ab",  0, 1, "c", 0, 1, 2, 'c', "cb", "cb", 0);
         ...
     };

     static void test_replace () {
         //...
         const std::size_t ncases =
             sizeof test_cases / sizeof *test_cases;

         for (std::size_t i = 0; i != ncases; ++i)
             test_replace (pfid, tets_case [i].line, which, ...);
     }

Thanks
Martin