On Sat, 1 Nov 2025, 21:10 François Dumont, <[email protected]> wrote:

> Hi
>
> Here is the implementation of the std::__debug::inplace_vector.
>

It looks like this is going to repeat the assertions in e.g. operator[],
front, back etc because those already have assertions in the base class.


> I eventually had to introduce a dedicated std::erase_if/std::erase
> implementation for the __debug version. When directly using
> __gnu_debug::inplace_vector the std::erase_if was used downcasting the


If it's converting to a base, it's upcasting.


> inplace_vector into the std::inplace_vector base class so breaking
> encapsulation. So we really need dedicated implementations and I'll
> prepare patches for other containers.
>

Why is the implicit cast a problem? We know that erase_if doesn't iterate
past the end, and only dereferences valid iterators. That's one of the
benefits of using erase_if: it ensures the container is accessed safely.

Is encapsulation broken if a standard library function does that implicit
cast? Is it observable to users?



> I also added tests for std::swap. Overloading it in std:: namespace
> seems the best option to me. If it is defined in std::__debug:: as it
> should then calls to std::swap in user code are replaced by the generic
> call to swap in move.h instead of calling inplace_vector<>::swap.


Users should call swap not std::swap. That was the consensus when LWG
discussed https://cplusplus.github.io/LWG/issue4250

It
> still works to invalidate iterators but is a major performance issue. I
> noticed this running in gdb, is there a way to test for call to the
> correct swap ?
>
> I also wonder in
> testsuite/23_containers/inplace_vector/debug/invalidation/swap.cc test02
> how gcc does to find the proper swap method on the unqualified call ?
>

Argument Dependent Lookup ?


>      Add _GLIBCXX_DEBUG std::inplace_vector implementation.
>
>      Unlike other _GLIBCXX_DEBUG containers the
> std::__debug::inplace_vector is
>      inheriting privately from both __gnu_debug::_Safe_sequence<> and
> std::inplace_vector
>      classes to preserve encapsulation. Iterators won't be created or
> invalidated without
>      std::__debug::inplace_vector being aware. However
> std::__debug::inplace_vector<T, 0>
>      inherits publicly from std::inplace_vector<T, 0> to benefit from
> the same std::
>      functions overloads.
>
>      libstdc++-v3/ChangeLog:
>
>              * include/Makefile.am (debug_headers): Add inplace_vector.
>              * include/Makefile.in: Regenerate.
>              * include/debug/inplace_vector: New.
>              * include/debug/safe_base.h (~_Safe_sequence_base()): Add
> C++11 noexcept
>              qualification.
>              (_Safe_sequence_base::operator=(const
> _Safe_sequence_base&)): New.
> (_Safe_sequence_base::operator=(_Safe_sequence_base&&)): New.
>              (_Safe_sequence_base::_M_invalidate_all): Add C++20
> constexpr qualification.
>              * include/debug/safe_container.h
>              (_Safe_container::operator=(const _Safe_container&)):
> Implement using same
>              _Safe_sequence_base operator.
>              * include/std/inplace_vector
> [_GLIBCXX_DEBUG](std::inplace_vector<>): Move
>              implementation into __cxx1998 namespace.
>              (erase, erase_if): Limit to non-debug inplace_vector<>,
> cleanup code.
>              [_GLIBCXX_DEBUG]: Add include <debug/inplace_vector>.
>              * testsuite/23_containers/inplace_vector/cons/1.cc: Adapt,
> skip several
>              is_trivially_xxx checks when in _GLIBCXX_DEBUG mode.
>              * testsuite/23_containers/inplace_vector/copy.cc: Likewise.
>              * testsuite/23_containers/inplace_vector/move.cc: Likewise.
>              *
> testsuite/23_containers/inplace_vector/debug/assign1_neg.cc: New test case.
>              *
> testsuite/23_containers/inplace_vector/debug/assign2_neg.cc: New test case.
>              *
> testsuite/23_containers/inplace_vector/debug/assign3_neg.cc: New test case.
>              *
> testsuite/23_containers/inplace_vector/debug/assign4_backtrace_neg.cc:
> New test case.
>              *
> testsuite/23_containers/inplace_vector/debug/assign4_neg.cc: New test case.
>              *
> testsuite/23_containers/inplace_vector/debug/construct1_neg.cc: New test
> case.
>              *
> testsuite/23_containers/inplace_vector/debug/construct2_neg.cc: New test
> case.
>              *
> testsuite/23_containers/inplace_vector/debug/construct3_neg.cc: New test
> case.
>              *
> testsuite/23_containers/inplace_vector/debug/construct4_neg.cc: New test
> case.
>              *
> testsuite/23_containers/inplace_vector/debug/debug_functions.cc: New
> test case.
>              * testsuite/23_containers/inplace_vector/debug/erase.cc:
> New test case.
>              *
> testsuite/23_containers/inplace_vector/debug/insert1_neg.cc: New test case.
>              *
> testsuite/23_containers/inplace_vector/debug/insert2_neg.cc: New test case.
>              *
> testsuite/23_containers/inplace_vector/debug/insert3_neg.cc: New test case.
>              *
> testsuite/23_containers/inplace_vector/debug/insert4_neg.cc: New test case.
>              *
> testsuite/23_containers/inplace_vector/debug/insert5_neg.cc: New test case.
>              *
> testsuite/23_containers/inplace_vector/debug/insert7_neg.cc: New test case.
>              *
> testsuite/23_containers/inplace_vector/debug/invalidation/1.cc: New test
> case.
>              *
> testsuite/23_containers/inplace_vector/debug/invalidation/2.cc: New test
> case.
>              *
> testsuite/23_containers/inplace_vector/debug/invalidation/3.cc: New test
> case.
>              *
> testsuite/23_containers/inplace_vector/debug/invalidation/4.cc: New test
> case.
>              *
> testsuite/23_containers/inplace_vector/debug/invalidation/append_range.cc:
> New test case.
>              *
> testsuite/23_containers/inplace_vector/debug/invalidation/erase.cc: New
> test case.
>              *
> testsuite/23_containers/inplace_vector/debug/invalidation/pop_back.cc:
> New test case.
>              *
> testsuite/23_containers/inplace_vector/debug/invalidation/push_back.cc:
> New test case.
>              *
> testsuite/23_containers/inplace_vector/debug/invalidation/swap.cc: New
> test case.
>              *
> testsuite/23_containers/inplace_vector/debug/invalidation/try_append_range.cc:
>
> New test case.
>              *
> testsuite/23_containers/inplace_vector/debug/invalidation/try_emplace_back.cc:
>
> New test case.
>              *
> testsuite/23_containers/inplace_vector/debug/invalidation/try_push_back.cc:
>
> New test case.
>              *
> testsuite/23_containers/inplace_vector/debug/invalidation/unchecked_emplace_back.cc:
>
> New test case.
>              *
> testsuite/23_containers/inplace_vector/debug/multithreaded_swap.cc: New
> test case.
>              * testsuite/util/debug/checks.h: Avoid using _GLIBCXX_DEBUG
> containers in test
>              implementations.
>
> Tested under Linux x86_64.
>
> Ok to commit ?
>
> PR: https://forge.sourceware.org/gcc/gcc-TEST/pulls/119
>
> François
>
>

Reply via email to