[PATCH] D14686: Protect against overloaded comma in random_shuffle and improve tests

Dmitri Gribenko via cfe-commits cfe-commits at lists.llvm.org
Fri Nov 20 14:35:45 PST 2015


gribozavr added inline comments.

================
Comment at: test/std/algorithms/alg.modifying.operations/alg.random.shuffle/random_shuffle.pass.cpp:35
@@ -30,1 +34,3 @@
 }
+
+template <class Iter>
----------------
gribozavr wrote:
> mclow.lists wrote:
> > This is not how I would rewrite this test.
> > I would write a routine that took two "iterators", and called `random_shuffle`, and then checked for the desired results.
> > 
> > Then call it with pointers. and RA iters, etc.
> > for example:
> > 
> >     template <Class Iter>
> >     void test(Iter first, Iter last, Iter resFirst, Iter resLast);
> > 
> >     test(nullptr, nullptr, nullptr, nullptr);
> >     int source[] = {1, 2, 3, 4};
> >     int res      [] = {1, 2, 3, 4};
> >     const unsigned size = sizeof(source)/sizeof(source[0]);
> >     
> >     test(source, source + size, res, res+size); 
> >     test(random_access_iterator<int*>(source) .... );
> > 
> > 
> I actually thought about this, and it is hard to rewrite it like that for two reasons.  First, `random_shuffle` mutates the elements, so one needs to restore the original sequence between calls to `test()` (otherwise it is not obvious that it was mutated).  Second, this overload of `random_shuffle` takes randomness from global state, so one can't just specify one expected result in the test.  That's why I first check for the initial shuffle exactly, and then only check that the output is a permutation of input.
> 
Ping?


Repository:
  rL LLVM

http://reviews.llvm.org/D14686





More information about the cfe-commits mailing list