[PATCH] libstdc++: Fix flat_map::operator[] for const lvalue keys [PR120432]

[Tomasz Kaminski]

On Wed, May 28, 2025 at 3:00 PM Patrick Palka <ppalka@redhat.com> wrote:

> On Wed, 28 May 2025, Tomasz Kaminski wrote:
>
> >
> >
> > On Tue, May 27, 2025 at 7:08 PM Patrick Palka <ppalka@redhat.com> wrote:
> >       Tested on x86_64-pc-linux-gnu, does this look OK for trunk/15?
> >
> >       The 'volatile' issue from that PR Will be fixed in a separate
> patch as
> >       operator[] isn't the only operation that's affected.
>
LGTM to me, thanks.

> >
> >       -- >8 --
> >
> >       The const lvalue operator[] overload wasn't properly forwarding
> the key
> >       type to the generic overload.
> >
> >               PR libstdc++/120432
> >
> >       libstdc++-v3/ChangeLog:
> >
> >               * include/std/flat_map (_Flat_map_base::operator[]):
> Correct
> >               forwarding from the const lvalue key overload.
> >               * testsuite/23_containers/flat_map/1.cc (test08): New test.
> >               * testsuite/23_containers/flat_multimap/1.cc (test08): New
> test.
> >       ---
> >        libstdc++-v3/include/std/flat_map                      |  2 +-
> >        libstdc++-v3/testsuite/23_containers/flat_map/1.cc     | 10
> ++++++++++
> >        .../testsuite/23_containers/flat_multimap/1.cc         | 10
> ++++++++++
> >        3 files changed, 21 insertions(+), 1 deletion(-)
> >
> >       diff --git a/libstdc++-v3/include/std/flat_map
> b/libstdc++-v3/include/std/flat_map
> >       index 6593988d213c..4d9ced1e8191 100644
> >       --- a/libstdc++-v3/include/std/flat_map
> >       +++ b/libstdc++-v3/include/std/flat_map
> >       @@ -1142,7 +1142,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
> >              // element access
> >              mapped_type&
> >              operator[](const key_type& __x)
> >       -      { return operator[]<const key_type>(__x); }
> >       +      { return operator[]<const key_type&>(__x); }
> >
> > Given that the operator[] that we are forading to is implemented as:
> >         { return try_emplace(std::forward<_Key2>(__x)).first->second; }
> > I would just call try_emplace directly:
>
> Good point, the implementation is a simple one-liner either way, and it
> addresses the volatile key issue.  Like so?
>
> -- >8 --
>
> Subject: [PATCH] libstdc++: Fix flat_map::operator[] for const lvalue keys
>  [PR120432]
>
> The const lvalue operator[] overload wasn't properly forwarding the key
> type to the generic overload, causing a hard error for const keys.
>
> This patch fixes this by making the non-template overloads call
> try_emplace directly instead, which means we can remove the non-standard
> same_as constraint on the generic overload.
>
>         PR libstdc++/120432
>
> libstdc++-v3/ChangeLog:
>
>         * include/std/flat_map (flat_map::operator[]): Make the
>         non-template overloads call try_emplace directly.  Remove
>         non-standard same_as constraint on the template overload.
>         * testsuite/23_containers/flat_map/1.cc (test08): New test.
> ---
>  libstdc++-v3/include/std/flat_map                  |  6 +++---
>  libstdc++-v3/testsuite/23_containers/flat_map/1.cc | 10 ++++++++++
>  2 files changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/libstdc++-v3/include/std/flat_map
> b/libstdc++-v3/include/std/flat_map
> index 6593988d213c..5f9a2eda1939 100644
> --- a/libstdc++-v3/include/std/flat_map
> +++ b/libstdc++-v3/include/std/flat_map
> @@ -1142,14 +1142,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>        // element access
>        mapped_type&
>        operator[](const key_type& __x)
> -      { return operator[]<const key_type>(__x); }
> +      { return try_emplace(__x).first->second; }
>
>        mapped_type&
>        operator[](key_type&& __x)
> -      { return operator[]<key_type>(std::move(__x)); }
> +      { return try_emplace(std::move(__x)).first->second; }
>
>        template<typename _Key2>
> -       requires same_as<remove_cvref_t<_Key2>, _Key> ||
> __transparent_comparator<_Compare>
> +       requires __transparent_comparator<_Compare>
>         mapped_type&
>         operator[](_Key2&& __x)
>         { return try_emplace(std::forward<_Key2>(__x)).first->second; }
> diff --git a/libstdc++-v3/testsuite/23_containers/flat_map/1.cc
> b/libstdc++-v3/testsuite/23_containers/flat_map/1.cc
> index a9690208b09f..2af516410279 100644
> --- a/libstdc++-v3/testsuite/23_containers/flat_map/1.cc
> +++ b/libstdc++-v3/testsuite/23_containers/flat_map/1.cc
> @@ -253,6 +253,15 @@ test07()
>    VERIFY( std::ranges::equal(m, (std::pair<int,int>[]){{3,4}}) );
>  }
>
> +void
> +test08()
> +{
> +  // PR libstdc++/120432 - flat_map operator[] is broken for const lvalue
> keys
> +  std::flat_map<int, int> m;
> +  const int k = 42;
> +  m[k] = 0;
> +}
> +
>  int
>  main()
>  {
> @@ -266,4 +275,5 @@ main()
>    test05();
>    test06();
>    test07();
> +  test08();
>  }
> --
> 2.49.0.654.g845c48a16a
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://gcc.gnu.org/pipermail/libstdc++/attachments/20250528/3c5fc8e1/attachment.htm>