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] > 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. > >> > >> > 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 >> > >> > > >