On Fri, 25 Jul 2025, Tomasz Kaminski wrote:

> 
> 
> On Fri, Jul 25, 2025 at 4:57 PM Tomasz Kaminski <tkami...@redhat.com> wrote:
> 
> 
> On Fri, Jul 25, 2025 at 4:49 PM Patrick Palka <ppa...@redhat.com> wrote:
>       On Fri, 25 Jul 2025, Tomasz Kamiński wrote:
> 
>       >       PR libstdc++/121196
>       >
>       > libstdc++-v3/ChangeLog:
>       >
>       >       * include/std/inplace_vector (std::erase): Provide default 
> argument
>       >       for _Up parameter.
>       >       * testsuite/23_containers/inplace_vector/erasure.cc: Add test 
> for
>       >       using braces-init-list as arguments to erase_if and use function
>       >       to verify content of inplace_vector
>       > ---
>       > I believe the actual fix (adding = _Tp) is trivial and could be 
> commited
>       > without review. I have done more extensive changes to tests.
>       >
>       > Tested on x86_64-linux.
>       >
>       >  libstdc++-v3/include/std/inplace_vector       |  2 +-
>       >  .../23_containers/inplace_vector/erasure.cc   | 26 
> ++++++++++++++++---
>       >  2 files changed, 24 insertions(+), 4 deletions(-)
>       >
>       > diff --git a/libstdc++-v3/include/std/inplace_vector 
> b/libstdc++-v3/include/std/inplace_vector
>       > index 290cf6eb0e9..b5a81bed3c9 100644
>       > --- a/libstdc++-v3/include/std/inplace_vector
>       > +++ b/libstdc++-v3/include/std/inplace_vector
>       > @@ -1354,7 +1354,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>       >      }
>       > 
>       > 
>       > -  template<typename _Tp, size_t _Nm, typename _Up>
>       > +  template<typename _Tp, size_t _Nm, typename _Up = _Tp>
>       >      constexpr  size_t
>       >      erase(inplace_vector<_Tp, _Nm>& __cont, const _Up& __value)
>       >      {
>       > diff --git 
> a/libstdc++-v3/testsuite/23_containers/inplace_vector/erasure.cc 
> b/libstdc++-v3/testsuite/23_containers/inplace_vector/erasure.cc
>       > index c7fda097896..8fb56e90623 100644
>       > --- a/libstdc++-v3/testsuite/23_containers/inplace_vector/erasure.cc
>       > +++ b/libstdc++-v3/testsuite/23_containers/inplace_vector/erasure.cc
>       > @@ -2,18 +2,38 @@
>       > 
>       >  #include <inplace_vector>
>       >  #include <testsuite_hooks.h>
>       > +#include <span>
>       > +
>       > +template<typename T, size_t N>
>       > +constexpr bool
>       > +eq(const std::inplace_vector<T, N>& l, std::span<const T> r) {
>       > +  if (l.size() != r.size())
>       > +    return false;
>       > +  for (auto i = 0u; i < l.size(); ++i)
>       > +    if (l[i] != r[i])
>       > +      return false;
>       > +  return true;
>       > +};
> 
>       Is this helper function to make the equality checks more concise?
>       I like to just use:
> 
>         ranges::equal(c, (int[]){3, 6, 3});
> 
> I believe (int[]){...} is compound literal 
> (https://en.cppreference.com/w/c/language/compound_literal.html)
> that is not supported in standard C++. GCC gives me warning on that:
> <source>:2:29: warning: ISO C++ forbids compound-literals [-Wpedantic]

Whoops, I forget about that... and libstdc++ tests aren't compiled with
-pedantic by default unlike C++ front end test.

> In C++ we could use:
> > using T = int[];
> > ranges::equal(c, T{3, 6, 3});
> Or in C++26 this should work, because span can be constructed from 
> initializer_list:
> > ranges::equal(c, std::span{3, 6, 6});
> I will try to check that.
> 
> Because span constructor takes initializer_list<value_type> (i.e. 
> remove_cv_t<T>)
> The deduction does not work. Also there is no CTAD.
> 
> I just got this function copied from another test.
> 
>       which works generally for any range 'c' (of integers).  But either
>       approach works for me.  LGTM either way.
> 
> I ended up with something like follows:
> VERIFY( std::ranges::equal(c, std::span<int const>({3, 6, 6}));
> I think this obfuscates a the "meat" of the test with the sigils,
> so I prefer keeping eq calls that are less verbose.

Works for me.

>  
> 
>             > 
>             >  constexpr void
>             >  test_erase()
>             >  {
>             > -  std::inplace_vector<int, 15> c{1, 2, 3, 4, 5, 6, 5, 4, 3, 2, 
> 1, 4, 4, 9};
>             > +  std::inplace_vector<int, 15> c{1, 0, 3, 4, 5, 6, 5, 4, 3, 0, 
> 1, 4, 4, 9};
>             >    std::erase(c, 4);
>             >    VERIFY( c.size() == 10 );
>             >    std::erase(c, 1);
>             >    VERIFY( c.size() == 8 );
>             >    std::erase(c, 9);
>             >    VERIFY( c.size() == 7 );
>             > -  VERIFY( (c == std::inplace_vector<int, 15>{2, 3, 5, 6, 5, 3, 
> 2}) );
>             > +  VERIFY( eq(c, {0, 3, 5, 6, 5, 3, 0}) );
>             > + 
>             > +  std::erase(c, {});
>             > +  VERIFY( c.size() == 5 );
>             > +  VERIFY( eq(c, {3, 5, 6, 5, 3}) );
>             > +
>             > +  std::erase(c, {5});
>             > +  VERIFY( c.size() == 3 );
>             > +  VERIFY( eq(c, {3, 6, 3}) );
>             > 
>             >    std::inplace_vector<int, 0> e;
>             >    std::erase(e, 10);
>             > @@ -29,7 +49,7 @@ test_erase_if()
>             >    std::erase_if(c, [](int i) { return i == 4; });
>             >    VERIFY( c.size() == 8 );
>             >    std::erase_if(c, [](int i) { return i & 1; });
>             > -  VERIFY( (c == std::inplace_vector<int, 15>{2, 2}) );
>             > +  VERIFY( eq(c, {2, 2}) );
>             > 
>             >    std::inplace_vector<int, 0> e;
>             >    std::erase_if(e, [](int i) { return i > 5; });
>             > --
>             > 2.49.0
>             >
>             >
> 
> 
> 

Reply via email to