Re: [PATCH] Complete __gnu_debug::basic_string

[Jonathan Wakely <jwakely@redhat.com>]

On 29/03/21 22:25 +0200, François Dumont wrote:
>On 25/03/21 4:29 pm, Jonathan Wakely wrote:
>>Oh, it's the same generate(sz) bug as you already found. But I've
>>found other bugs, e.g. with GLIBCXX_SEED_TEST_RNG=1908970375).
>>
>>I think we should also add a check for non-empty containers to those
>>test functions, and ensure we don't try to erase from empty
>>containers (see attached).
>>
>>
>I had a look at this patch and on my side I was more thinking about 
>avoiding empty containers in the first place.
>
>What do you think ?

I did consider this and decided against it for the reasons below.

>François
>

>diff --git a/libstdc++-v3/testsuite/util/exception/safety.h b/libstdc++-v3/testsuite/util/exception/safety.h
>index 54449d2f7bb..6940f2f765d 100644
>--- a/libstdc++-v3/testsuite/util/exception/safety.h
>+++ b/libstdc++-v3/testsuite/util/exception/safety.h
>@@ -55,14 +55,14 @@ namespace __gnu_test
> 
>     // Return randomly generated integer on range [0, __max_size].
>     static size_type
>-    generate(size_type __max_size)
>+    generate(size_type __max_size, size_type __min_size = 0)
>     {
>       using param_type = typename distribution_type::param_type;
> 
>       // Make the engine and distribution static...
>       static engine_type engine = get_engine();
>       static distribution_type distribution;
>-      return distribution(engine, param_type{0, __max_size});
>+      return distribution(engine, param_type{__min_size, __max_size});
>     }
> 
>     // Given an instantiating type, return a unique value.
>@@ -198,7 +198,8 @@ namespace __gnu_test
> 
> 	  // Size test container.
> 	  const size_type max_elements = 100;
>-	  size_type n = generate(max_elements);
>+	  const size_type min_elements = 10;
>+	  size_type n = generate(max_elements, min_elements);

Why 10 though? Why not 1? Otherwise we never test the case where you
erase a single element from a container that only has a single
element.

And it would also mean we never test inserting into an empty
container, because it always starts with at least 10 elements.

So I decided that it was better to keep /most/ of the tests unchanged,
so they sometimes (depending on the RNG, which can now be made less
predictable by using a non-default seed) test with empty containers.
Only the operations that really can't work for empty containers need
to care about them. For other operations, we should keep testing the
empty case.