[libcxx-commits] [PATCH] D130124: [libc++][ranges] fix `std::search_n` incorrect `static_assert`

Hui via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Tue Jul 19 14:45:26 PDT 2022


huixie90 added a comment.

In D130124#3663907 <https://reviews.llvm.org/D130124#3663907>, @ldionne wrote:

> Should we add a similar test in `std::search` and others that have a similar pattern?

`std::search` takes two ranges so it is slightly different. But yes it is worthwhile checking other algorithms which take a range and a value to see if they work properly



================
Comment at: libcxx/test/std/algorithms/alg.nonmodifying/alg.search/search_n_pred.pass.cpp:189
     test<random_access_iterator<const int*> >();
-
+    testStaticAssertBug();
 #if TEST_STD_VER > 17
----------------
ldionne wrote:
> jloser wrote:
> > huixie90 wrote:
> > > jloser wrote:
> > > > Nit: can we give a better name to this function?
> > > Any suggestions? I agree it is a terrible name. It is my failed attempt to give it a reasonable name
> > Maybe `test_binary_predicate_callable`? Note the camel case instead to match that of `test_constexpr` for example.
> Suggestion -- don't give it a name. Define it in `main` like this instead:
> 
> ```
> // test bug reported in https://reviews.llvm.org/D124079?#3661721
> {
>   A a[]       = {A(1, 2), A(2, 3), A(2, 4)};
>   int value   = 2;
>   auto result = std::search_n(a, a + 3, 1, value, Pred());
>   assert(result == a + 1);
> }
> ```
good suggestion. thanks


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130124/new/

https://reviews.llvm.org/D130124



More information about the libcxx-commits mailing list