On Fri, 14 Oct 2022, Richard Biener wrote:
> On Thu, Oct 13, 2022 at 5:40 PM Patrick Palka via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
> >
> > Here during stream in we end up having created a type variant for the enum
> > before we read the enum's definition, and thus the variant inherited stale
> > TYPE_VALUES and TYPE_MIN/MAX_VALUES, which leads to an ICE (with -g). The
> > stale variant got created from set_underlying_type during earlier stream in
> > of the (redundant) typedef for the enum.
> >
> > This patch works around this by setting TYPE_VALUES and TYPE_MIN/MAX_VALUES
> > for all variants when reading in an enum definition. Does this look like
> > the right approach? Or perhaps we need to arrange that we read the enum
> > definition before reading in the typedef decl? Note that seems to be an
> > issue only when the typedef name and enum names are the same (thus the
> > typedef is redundant), otherwise we seem to read the enum definition first
> > as desired.
> >
> > PR c++/106848
> >
> > gcc/cp/ChangeLog:
> >
> > * module.cc (trees_in::read_enum_def): Set the TYPE_VALUES,
> > TYPE_MIN_VALUE and TYPE_MAX_VALUE of all type variants.
> >
> > gcc/testsuite/ChangeLog:
> >
> > * g++.dg/modules/enum-9_a.H: New test.
> > * g++.dg/modules/enum-9_b.C: New test.
> > ---
> > gcc/cp/module.cc | 9 ++++++---
> > gcc/testsuite/g++.dg/modules/enum-9_a.H | 5 +++++
> > gcc/testsuite/g++.dg/modules/enum-9_b.C | 6 ++++++
> > 3 files changed, 17 insertions(+), 3 deletions(-)
> > create mode 100644 gcc/testsuite/g++.dg/modules/enum-9_a.H
> > create mode 100644 gcc/testsuite/g++.dg/modules/enum-9_b.C
> >
> > diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
> > index 7ffeefa7c1f..97fb80bcd44 100644
> > --- a/gcc/cp/module.cc
> > +++ b/gcc/cp/module.cc
> > @@ -12303,9 +12303,12 @@ trees_in::read_enum_def (tree defn, tree
> > maybe_template)
> >
> > if (installing)
> > {
> > - TYPE_VALUES (type) = values;
> > - TYPE_MIN_VALUE (type) = min;
> > - TYPE_MAX_VALUE (type) = max;
> > + for (tree t = type; t; t = TYPE_NEXT_VARIANT (t))
> > + {
> > + TYPE_VALUES (t) = values;
> > + TYPE_MIN_VALUE (t) = min;
> > + TYPE_MAX_VALUE (t) = max;
> > + }
>
> it's definitely somewhat ugly but at least type_hash_canon doesn't hash
> these for ENUMERAL_TYPE (but it does compare them! which in principle
> means it could as well hash them ...)
>
> I think that if you read both from the same module that you should arrange
> to read what you refer to first? But maybe that's not the actual issue here.
*nod* reading in the enum before reading in the typedef seems like
the most direct solution, though not sure how to accomplish that :/
A somewhat orthogonal issue (that incidentally fixes this testcase) is
that we stream TYPE_MIN/MAX_VALUE only for enums with a definition, but
the frontend sets these fields even for opaque enums. If we make sure
to stream these fields for all ENUMERAL_TYPEs, then we won't have to
worry about these fields being stale for variants that may have been
created before reading in the enum definition (their TYPE_VALUES field
will still be stale I guess, but verify_type doesn't worry about that
it seems, so we avoid the ICE).
patch to that effect is at
https://gcc.gnu.org/pipermail/gcc-patches/2022-October/603831.html
>
> Richard.
>
> >
> > rest_of_type_compilation (type, DECL_NAMESPACE_SCOPE_P (defn));
> > }
> > diff --git a/gcc/testsuite/g++.dg/modules/enum-9_a.H
> > b/gcc/testsuite/g++.dg/modules/enum-9_a.H
> > new file mode 100644
> > index 00000000000..fb7d10ad3b6
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/modules/enum-9_a.H
> > @@ -0,0 +1,5 @@
> > +// PR c++/106848
> > +// { dg-additional-options -fmodule-header }
> > +// { dg-module-cmi {} }
> > +
> > +typedef enum memory_order { memory_order_seq_cst } memory_order;
> > diff --git a/gcc/testsuite/g++.dg/modules/enum-9_b.C
> > b/gcc/testsuite/g++.dg/modules/enum-9_b.C
> > new file mode 100644
> > index 00000000000..63e81675d0a
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/modules/enum-9_b.C
> > @@ -0,0 +1,6 @@
> > +// PR c++/106848
> > +// { dg-additional-options "-fmodules-ts -g" }
> > +
> > +import "enum-9_a.H";
> > +
> > +memory_order x = memory_order_seq_cst;
> > --
> > 2.38.0.68.ge85701b4af
> >
>
>
