On Wed, 7 Feb 2024, 19:20 Patrick Palka, <[email protected]> wrote:
> On Wed, 7 Feb 2024, Jonathan Wakely wrote:
>
> >
> >
> > On Wed, 7 Feb 2024 at 17:29, Patrick Palka <[email protected]> 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
> >
> >
> >
