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 21.string.swap
Date Sat, 25 Mar 2006 16:46:05 GMT
Anton Pevtsov wrote:
> The attached file contains the test for lib.string.swap.
> It seems to be useful to exercise that the swap takes constant time, but
> I am not sure that we have an apropriate mechanism for this check,
> especially for char and char_traits.

We do:

     std::string s1 ("foo");
     std::string s2 ("bar");

     const char* const p1 = s1.data ();
     const char* const p2 = s2.data ();

     s1.swap ();

     assert (s1.data () == p2);
     assert (s2.data () == p1);

With a user-defined allocator we should also check that the function
doesn't call allocator_type::allocate() (or deallocate). Even without
a user-defined allocator we can check the complexity of the function
in user-defined traits: by verifying that none of its linear members
(assign, copy, or move) is called (that is the true test; allocation
itself doesn't necessarily imply anything about the complexity of a
function, even though in practice it usually does).

> Also, this test become intresting than different allocators are used. If
> they are the same the std::swap results in the trivial pointers swap.

Yes, we should definitely exercise swap with unequal allocators.

> 
[...]
>     static charT wstr1 [LLEN];
>     static charT wstr2 [LLEN];
> 
>     // construct strings
>     rw_widen (wstr1, cs.str1, cs.str1_len);
>     rw_widen (wstr2, cs.str2, cs.str2_len);
> 
>     TestString str1 (wstr1, cs.str1_len);
>     TestString str2 (wstr2, cs.str2_len);

Instead of using the buffers here could we widen the narrow char
arrays directly into the strings:

     TestString str1;
     str1.resize (cs.str1_len);
     rw_widen (&str1 [0], cs.str1, cs.str1_len);

> 
>     str1.swap (str2);
> 
> #define CALLFMAT                                                             \
>     "line %d. std::basic_string<%s, %s<%2$s>, %s<%2$s>>(%{#*s})." 
          \
>     "swap (%{#*s})"
> 
> #define CALLARGS                                                             \
>     __LINE__, pfid->cname_, pfid->tname_, pfid->aname_, int (cs.str1_len), 
 \
>     cs.str1, int (cs.str2_len), rw_narrow (tmp1, wstr2, cs.str2_len)
> 
>     bool success = 
>         !TestString::traits_type::compare (wstr1, str2.c_str(), cs.str1_len);

Rather than relying on Traits I think it would be better to use
rw_match() to compare the contents of the string objects against
the original narrow char array. That's what the function is for:
to allow comparisons of heterogeneous arrays of characters. It
also returns the index of the first mismatched character which
can be helpful in diagnostics.

More important, though, we should be comparing the data() pointers
for equality (when the allocators compare equal).

> 
>     // strings used for output
>     static char tmp1[LLEN];
>     static char tmp2[LLEN];
>     static char tmp3[LLEN];
> 
>     rw_assert (success, 0, cs.line,
>                CALLFMAT " this == %{#*s}, got this = %{#*s}", CALLARGS, 
>                int (cs.str2_len), rw_narrow (tmp2, wstr2, cs.str2_len),
>                int (str1.size ()), 
>                rw_narrow (tmp3, str1.c_str (), str1.size ()));

I would prefer to keep the rw_assert() argument list as "simple" as
possible (they tend to be complicated enough with all the conditional
directives) and keep function calls separate.

Also, since these arrays are big (over 4K each), I think we should
keep their number to the necessary minimum and avoid them if at all
possible. Here, we should be able to use the new %{/Gs} directive
to print out the contents of the strings directly without calling
rw_narrow().

Martin

Mime
View raw message