Re: [PATCH v2 2/6] gdb support: add gdb::ranges::replace algorithm

[Andrew Burgess <aburgess@redhat.com>]

Matthieu Longo <matthieu.longo@arm.com> writes:

> Provide a C++17-compatible replacement for the C++20 std::ranges::replace
> algorithm, allowing callers to use a consistent interface until GDB
> transitions to C++20. The helper should be removed once the C++ standard
> library implementation become available.
>
> https://en.cppreference.com/cpp/algorithm/ranges/replace
> ---
>  gdbsupport/array-view.h | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
>
> diff --git a/gdbsupport/array-view.h b/gdbsupport/array-view.h
> index 8431d7f5add..f9842ecff30 100644
> --- a/gdbsupport/array-view.h
> +++ b/gdbsupport/array-view.h
> @@ -225,6 +225,22 @@ void copy (gdb::array_view<U> src, gdb::array_view<T> dest)
>      std::copy_backward (src.begin (), src.end (), dest.end ());
>  }
>  
> +namespace ranges {
> +
> +/* Replace all occurrences of a value in the provided range.
> +
> +   Note: this helper is a reimplementation of std::ranges::replace, only
> +   available from C++20 onwards, and consequently, should be replaced by
> +   std::ranges::replace once GDB switches to C++20.  */
> +
> +template <class Range, typename T>
> +void replace (Range r, const T &old_value, const T &new_value)

Looking at the linked cppreference page, the C++20 functions take R as
'Range &&r'.  Doesn't your versions create a copy of the range?  This
will work fine for non-owning ranges, like gdb::array_view, but will
mean replace operates on a copy of the range for something like
std::vector.  Even if what you have above is intentional, I think this
difference should be highlighted and explained.

Also, as this isn't specifically tied to gdb::array_view, I wonder if
this would be better put into a new file gdbsupport/ranges.h ? I don't
think anything much is needed to create the new file other than just
adding the file, so that should be pretty easy to do.

Thanks,
Andrew