Re: [PATCH] libstdc++: Work around modules issue causing hello-1 ICE [PR113710]

[Jonathan Wakely <jwakely@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2897 bytes --]

On Wed, 7 Feb 2024 at 17:29, Patrick Palka <ppalka@redhat.com> wrote:

> Tested on x86_64-pc-linux-gnu, does this look OK for trunk?
>

OK.

Do we have a reduced testcase to track the modules bug that will still
exist in the FE?



>
> -- >8 --
>
> The forward declarations of std::get in <bits/stl_pair.h> added in
> r14-8710-g65b4cba9d6a9ff are causing an ICE in the test modules/hello-1
> due to what seems to be a declaration merging issue in modules.
>
> What seems to be happening is that in hello-1_b.C we first include
> <string_view>, which indirectly includes <bits/stl_pair.h> which forms
> the dependent specialization tuple_element<__i, tuple<_Elements...>>
> (appearing in some of the std::get overloads) and adds it to the
> specializations table.
>
> We then import hello which indirectly includes <tuple> (in the GMF),
> within which we define a partial specialization of tuple_element with
> that same template-id.  So importing hello in turn streams in this
> partial specialization, but we don't notice it matches the previously
> created dependent specialization, and we end up with two equivalent
> types for this template-id with different TYPE_CANONICAL.
>
> This patch works around this issue by adding a forward declaration of
> the tuple_element partial specialization from <tuple> to <bits/stl_pair.h>
> so that it appears alongside the dependent specialization of the same
> template-id.  So when including <bits/stl_pair.h> we immediately register
> the template-id as a partial specialization, and if we later stream in the
> partial specialization the MK_partial case of trees_in::key_mergeable will
> match them up.  (So perhaps a proper modules fix for this might be to make
> key_mergeable try to match up a streamed in partial specialization with an
> existing specialization from the table via match_mergeable_specialization.)
>
>         PR c++/113710
>
> libstdc++-v3/ChangeLog:
>
>         * include/bits/stl_pair.h (tuple_element): Add forward
>         declaration of the partial specialization for tuple.
> ---
>  libstdc++-v3/include/bits/stl_pair.h | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/libstdc++-v3/include/bits/stl_pair.h
> b/libstdc++-v3/include/bits/stl_pair.h
> index 00ec53ebc33..8c71b1350e5 100644
> --- a/libstdc++-v3/include/bits/stl_pair.h
> +++ b/libstdc++-v3/include/bits/stl_pair.h
> @@ -1153,6 +1153,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>      struct tuple_element<1, pair<_Tp1, _Tp2>>
>      { typedef _Tp2 type; };
>
> +  // Forward declare the partial specialization for std::tuple
> +  // to work around modules bug PR c++/113710.
> +  template<size_t __i, typename... _Types>
> +    struct tuple_element<__i, tuple<_Types...>>;
> +
>  #if __cplusplus >= 201703L
>    template<typename _Tp1, typename _Tp2>
>      inline constexpr size_t tuple_size_v<pair<_Tp1, _Tp2>> = 2;
> --
> 2.43.0.561.g235986be82
>
>