On Wed, 7 Feb 2024, 19:20 Patrick Palka, <ppalka@redhat.com> wrote:

> On Wed, 7 Feb 2024, Jonathan Wakely wrote:
>
> >
> >
> > 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?
>
> https://gcc.gnu.org/PR113814


Excellent, thanks!



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