| Message ID | 20221013153921.3795800-1-ppalka@redhat.com |
|---|---|
| State | New |
| Headers |
From: ppalka@redhat.com (Patrick Palka) Date: Thu, 13 Oct 2022 11:39:21 -0400 Subject: [PATCH] c++ modules: verify_type failure with typedef enum [PR106848] Message-ID: <20221013153921.3795800-1-ppalka@redhat.com> |
| Series |
c++ modules: verify_type failure with typedef enum [PR106848]
|
|
Commit Message
Patrick Palka
Oct. 13, 2022, 3:39 p.m. UTC
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
Comments
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 > > > >
On Tue, Oct 18, 2022 at 8:26 PM Patrick Palka <ppalka@redhat.com> wrote: > > 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 :/ For LTO streaming we DFS walk tree edges from all entries into the tree graph we want to stream, collecting and streaming SCCs. Not sure if doing similar for module streaming would help this case though. > 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 > > > > > > > >
On Wed, 19 Oct 2022, Richard Biener wrote: > On Tue, Oct 18, 2022 at 8:26 PM Patrick Palka <ppalka@redhat.com> wrote: > > > > 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 :/ > > For LTO streaming we DFS walk tree edges from all entries into the tree > graph we want to stream, collecting and streaming SCCs. Not sure if > doing similar for module streaming would help this case though. FWIW I managed to obtain a more interesting reduction for this ICE, one that doesn't use a typedef bound to the same name as the enum: $ cat 106848_a.H template<typename _T1> struct pair { using type = void(*)(const _T1&); }; struct _ScannerBase { enum _TokenT { _S_token_anychar }; pair<_TokenT> _M_token_tbl; }; $ cat 106848_b.C import "106848_a.H"; using type = _ScannerBase; $ g++ -fmodules-ts -g 106848_a.H 106848_b.C 106848_b.C:3:14: error: type variant differs by TYPE_MAX_VALUE <enumeral_type 0x7f252c757f18 _TokenT ...> <enumeral_type 0x7f252c757f18 _TokenT ...> Like in the less interesting testcase, the problem is ultimately that we create a variant of the enum (as part of reading in pair<_TokenT>::type) before reading the enum's definition, thus the variant inherits stale TYPE_MIN/MAX_VALUE. Perhaps pair<_TokenT>::type should indirectly depend on the definition of _TokenT -- but IIUC we generally don't require a type to be defined in order to refer to it, so enforcing such a dependency would be a pessimization I think. So ISTM this isn't a dependency issue (pair<_TokenT>::type already implicitly depends on the ENUMERAL_TYPE, just not also the enum's defining TYPE_DECL), and the true issue is that we're streaming TYPE_MIN/MAX_VALUE only as part of an enum's definition, which the linked patch fixes. > > > 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 > > > > > > > > > > > > > >
On 10/19/22 09:55, Patrick Palka wrote: > On Wed, 19 Oct 2022, Richard Biener wrote: > >> On Tue, Oct 18, 2022 at 8:26 PM Patrick Palka <ppalka@redhat.com> wrote: >>> >>> 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 :/ >> >> For LTO streaming we DFS walk tree edges from all entries into the tree >> graph we want to stream, collecting and streaming SCCs. Not sure if >> doing similar for module streaming would help this case though. > > FWIW I managed to obtain a more interesting reduction for this ICE, one > that doesn't use a typedef bound to the same name as the enum: > > $ cat 106848_a.H > template<typename _T1> > struct pair { > using type = void(*)(const _T1&); > }; > struct _ScannerBase { > enum _TokenT { _S_token_anychar }; > pair<_TokenT> _M_token_tbl; > }; > > $ cat 106848_b.C > import "106848_a.H"; > > using type = _ScannerBase; > > $ g++ -fmodules-ts -g 106848_a.H 106848_b.C > 106848_b.C:3:14: error: type variant differs by TYPE_MAX_VALUE > <enumeral_type 0x7f252c757f18 _TokenT ...> > <enumeral_type 0x7f252c757f18 _TokenT ...> > > Like in the less interesting testcase, the problem is ultimately that we > create a variant of the enum (as part of reading in pair<_TokenT>::type) > before reading the enum's definition, thus the variant inherits stale > TYPE_MIN/MAX_VALUE. > > Perhaps pair<_TokenT>::type should indirectly depend on the definition > of _TokenT -- but IIUC we generally don't require a type to be defined > in order to refer to it, so enforcing such a dependency would be a > pessimization I think. > > So ISTM this isn't a dependency issue (pair<_TokenT>::type already > implicitly depends on the ENUMERAL_TYPE, just not also the enum's > defining TYPE_DECL), and the true issue is that we're streaming > TYPE_MIN/MAX_VALUE only as part of an enum's definition, which the > linked patch fixes. Thanks for the explanation, it's a situation I didn;t anticipate and your fix is good. Could you add a comment about why you need to propagate the values though? nathan > >> >>> 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 >>>>> >>>> >>>> >>> >> >> >
On Fri, 21 Oct 2022, Nathan Sidwell wrote: > On 10/19/22 09:55, Patrick Palka wrote: > > On Wed, 19 Oct 2022, Richard Biener wrote: > > > > > On Tue, Oct 18, 2022 at 8:26 PM Patrick Palka <ppalka@redhat.com> wrote: > > > > > > > > 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 :/ > > > > > > For LTO streaming we DFS walk tree edges from all entries into the tree > > > graph we want to stream, collecting and streaming SCCs. Not sure if > > > doing similar for module streaming would help this case though. > > > > FWIW I managed to obtain a more interesting reduction for this ICE, one > > that doesn't use a typedef bound to the same name as the enum: > > > > $ cat 106848_a.H > > template<typename _T1> > > struct pair { > > using type = void(*)(const _T1&); > > }; > > struct _ScannerBase { > > enum _TokenT { _S_token_anychar }; > > pair<_TokenT> _M_token_tbl; > > }; > > > > $ cat 106848_b.C > > import "106848_a.H"; > > > > using type = _ScannerBase; > > > > $ g++ -fmodules-ts -g 106848_a.H 106848_b.C > > 106848_b.C:3:14: error: type variant differs by TYPE_MAX_VALUE > > <enumeral_type 0x7f252c757f18 _TokenT ...> > > <enumeral_type 0x7f252c757f18 _TokenT ...> > > > > Like in the less interesting testcase, the problem is ultimately that we > > create a variant of the enum (as part of reading in pair<_TokenT>::type) > > before reading the enum's definition, thus the variant inherits stale > > TYPE_MIN/MAX_VALUE. > > > > Perhaps pair<_TokenT>::type should indirectly depend on the definition > > of _TokenT -- but IIUC we generally don't require a type to be defined > > in order to refer to it, so enforcing such a dependency would be a > > pessimization I think. > > > > So ISTM this isn't a dependency issue (pair<_TokenT>::type already > > implicitly depends on the ENUMERAL_TYPE, just not also the enum's > > defining TYPE_DECL), and the true issue is that we're streaming > > TYPE_MIN/MAX_VALUE only as part of an enum's definition, which the > > linked patch fixes. > > Thanks for the explanation, it's a situation I didn;t anticipate and your fix > is good. Could you add a comment about why you need to propagate the values > though? Thanks a lot, will do. Just to make sure since there are multiple solutions proposed, do you prefer to go with https://gcc.gnu.org/pipermail/gcc-patches/2022-October/603487.html or https://gcc.gnu.org/pipermail/gcc-patches/2022-October/603831.html ? Both solutions fix the PR106848 issue (empty TYPE_MIN/MAX_VALUE on an enum type variant), but the latter also fixes the related PR102600 (empty TYPE_MIN/MAX_VALUE on the main variant of an enum with no enumerators). (We could maybe even combine the two solutions: stream TYPE_MIN/MAX_VALUE as part of ENUMERAL_TYPE, and also update TYPE_VALUES of each variant during trees_in::read_enum_def) > > nathan > > > > > > > > > > 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 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > Nathan Sidwell > >
On 10/21/22 09:11, Patrick Palka wrote: > On Fri, 21 Oct 2022, Nathan Sidwell wrote: >> >> Thanks for the explanation, it's a situation I didn;t anticipate and your fix >> is good. Could you add a comment about why you need to propagate the values >> though? > > Thanks a lot, will do. Just to make sure since there are multiple > solutions proposed, do you prefer to go with > https://gcc.gnu.org/pipermail/gcc-patches/2022-October/603487.html > or > https://gcc.gnu.org/pipermail/gcc-patches/2022-October/603831.html ? > > Both solutions fix the PR106848 issue (empty TYPE_MIN/MAX_VALUE on an > enum type variant), but the latter also fixes the related PR102600 > (empty TYPE_MIN/MAX_VALUE on the main variant of an enum with no > enumerators). (We could maybe even combine the two solutions: stream > TYPE_MIN/MAX_VALUE as part of ENUMERAL_TYPE, and also update TYPE_VALUES > of each variant during trees_in::read_enum_def) Oh, let's go with the latter: * module.cc (trees_out::core_vals): Stream TYPE_MAX_VALUE and TYPE_MIN_VALUE of ENUMERAL_TYPE. (trees_in::core_vals): Likewise. (trees_out::write_enum_def): Don't stream them here. (trees_in::read_enum_def): Likewise. but, again, some comments -- at the new streaming point, and in the defn streamer were we no longer stream them. thanks. > >> >> nathan >> >>> >>>> >>>>> 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 >>>>>>> >>>>>> >>>>>> >>>>> >>>> >>>> >>> >> >> -- >> Nathan Sidwell >> >> >
On Mon, 24 Oct 2022, Nathan Sidwell wrote: > On 10/21/22 09:11, Patrick Palka wrote: > > On Fri, 21 Oct 2022, Nathan Sidwell wrote: > > > > > > > Thanks for the explanation, it's a situation I didn;t anticipate and your > > > fix > > > is good. Could you add a comment about why you need to propagate the > > > values > > > though? > > > > Thanks a lot, will do. Just to make sure since there are multiple > > solutions proposed, do you prefer to go with > > https://gcc.gnu.org/pipermail/gcc-patches/2022-October/603487.html > > or > > https://gcc.gnu.org/pipermail/gcc-patches/2022-October/603831.html ? > > > > Both solutions fix the PR106848 issue (empty TYPE_MIN/MAX_VALUE on an > > enum type variant), but the latter also fixes the related PR102600 > > (empty TYPE_MIN/MAX_VALUE on the main variant of an enum with no > > enumerators). (We could maybe even combine the two solutions: stream > > TYPE_MIN/MAX_VALUE as part of ENUMERAL_TYPE, and also update TYPE_VALUES > > of each variant during trees_in::read_enum_def) > > > Oh, let's go with the latter: > > * module.cc (trees_out::core_vals): Stream TYPE_MAX_VALUE and > TYPE_MIN_VALUE of ENUMERAL_TYPE. > (trees_in::core_vals): Likewise. > (trees_out::write_enum_def): Don't stream them here. > (trees_in::read_enum_def): Likewise. > > but, again, some comments -- at the new streaming point, and in the defn > streamer were we no longer stream them. > > thanks. Thanks a lot, here's what I ended up committing: -- >8 -- Subject: [PATCH] c++ modules: enum TYPE_MIN/MAX_VALUE streaming [PR106848] In the frontend, the TYPE_MIN/MAX_VALUE of ENUMERAL_TYPE is the same as that of the enum's underlying type (see start_enum). And the underlying type of an enum is always known, even for an opaque enum that lacks a definition. But currently, we stream TYPE_MIN/MAX_VALUE of an enum only as part of its definition. So if the enum is declared but never defined, the ENUMERAL_TYPE we stream in will have empty TYPE_MIN/MAX_VALUE fields despite these fields being non-empty on stream out. And even if the enum is defined, read_enum_def updates these fields only on the main variant of the enum type, so for other variants (that we may have streamed in earlier) these fields remain empty. That these fields are unexpectedly empty for some ENUMERAL_TYPEs is ultimately the cause of the below two PRs. This patch fixes this by making us stream TYPE_MIN/MAX_VALUE directly for each ENUMERAL_TYPE rather than as part of the enum's definition, so that we naturally also stream these fields for opaque enums (and each enum type variant). PR c++/106848 PR c++/102600 gcc/cp/ChangeLog: * module.cc (trees_out::core_vals): Stream TYPE_MAX_VALUE and TYPE_MIN_VALUE of ENUMERAL_TYPE. (trees_in::core_vals): Likewise. (trees_out::write_enum_def): Don't stream them here. (trees_in::read_enum_def): Likewise. gcc/testsuite/ChangeLog: * g++.dg/modules/enum-9_a.H: New test. * g++.dg/modules/enum-9_b.C: New test. * g++.dg/modules/enum-10_a.H: New test. * g++.dg/modules/enum-10_b.C: New test. * g++.dg/modules/enum-11_a.H: New test. * g++.dg/modules/enum-11_b.C: New test. --- gcc/cp/module.cc | 39 +++++++++++++++--------- gcc/testsuite/g++.dg/modules/enum-10_a.H | 5 +++ gcc/testsuite/g++.dg/modules/enum-10_b.C | 6 ++++ gcc/testsuite/g++.dg/modules/enum-11_a.H | 5 +++ gcc/testsuite/g++.dg/modules/enum-11_b.C | 8 +++++ gcc/testsuite/g++.dg/modules/enum-9_a.H | 13 ++++++++ gcc/testsuite/g++.dg/modules/enum-9_b.C | 6 ++++ 7 files changed, 67 insertions(+), 15 deletions(-) create mode 100644 gcc/testsuite/g++.dg/modules/enum-10_a.H create mode 100644 gcc/testsuite/g++.dg/modules/enum-10_b.C create mode 100644 gcc/testsuite/g++.dg/modules/enum-11_a.H create mode 100644 gcc/testsuite/g++.dg/modules/enum-11_b.C 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 73971e7ff47..9957df510e6 100644 --- a/gcc/cp/module.cc +++ b/gcc/cp/module.cc @@ -6017,9 +6017,17 @@ trees_out::core_vals (tree t) if (CODE_CONTAINS_STRUCT (code, TS_TYPE_NON_COMMON)) { + if (code == ENUMERAL_TYPE) + { + /* These fields get set even for opaque enums that lack a + definition, so we stream them directly for each ENUMERAL_TYPE. + We stream TYPE_VALUES as part of the definition. */ + WT (t->type_non_common.maxval); + WT (t->type_non_common.minval); + } /* Records and unions hold FIELDS, VFIELD & BINFO on these things. */ - if (!RECORD_OR_UNION_CODE_P (code) && code != ENUMERAL_TYPE) + else if (!RECORD_OR_UNION_CODE_P (code)) { // FIXME: These are from tpl_parm_value's 'type' writing. // Perhaps it should just be doing them directly? @@ -6530,9 +6538,17 @@ trees_in::core_vals (tree t) if (CODE_CONTAINS_STRUCT (code, TS_TYPE_NON_COMMON)) { + if (code == ENUMERAL_TYPE) + { + /* These fields get set even for opaque enums that lack a + definition, so we stream them directly for each ENUMERAL_TYPE. + We stream TYPE_VALUES as part of the definition. */ + RT (t->type_non_common.maxval); + RT (t->type_non_common.minval); + } /* Records and unions hold FIELDS, VFIELD & BINFO on these things. */ - if (!RECORD_OR_UNION_CODE_P (code) && code != ENUMERAL_TYPE) + else if (!RECORD_OR_UNION_CODE_P (code)) { /* This is not clobbering TYPE_CACHED_VALUES, because this is a type that doesn't have any. */ @@ -12217,8 +12233,8 @@ trees_out::write_enum_def (tree decl) tree type = TREE_TYPE (decl); tree_node (TYPE_VALUES (type)); - tree_node (TYPE_MIN_VALUE (type)); - tree_node (TYPE_MAX_VALUE (type)); + /* Note that we stream TYPE_MIN/MAX_VALUE directly as part of the + ENUMERAL_TYPE. */ } void @@ -12242,8 +12258,6 @@ trees_in::read_enum_def (tree defn, tree maybe_template) { tree type = TREE_TYPE (defn); tree values = tree_node (); - tree min = tree_node (); - tree max = tree_node (); if (get_overrun ()) return false; @@ -12254,8 +12268,8 @@ 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; + /* Note that we stream TYPE_MIN/MAX_VALUE directly as part of the + ENUMERAL_TYPE. */ rest_of_type_compilation (type, DECL_NAMESPACE_SCOPE_P (defn)); } @@ -12269,22 +12283,17 @@ trees_in::read_enum_def (tree defn, tree maybe_template) tree new_decl = TREE_VALUE (values); if (DECL_NAME (known_decl) != DECL_NAME (new_decl)) - goto bad; + break; new_decl = maybe_duplicate (new_decl); if (!cp_tree_equal (DECL_INITIAL (known_decl), DECL_INITIAL (new_decl))) - goto bad; + break; } if (known || values) - goto bad; - - if (!cp_tree_equal (TYPE_MIN_VALUE (type), min) - || !cp_tree_equal (TYPE_MAX_VALUE (type), max)) { - bad:; error_at (DECL_SOURCE_LOCATION (maybe_dup), "definition of %qD does not match", maybe_dup); inform (DECL_SOURCE_LOCATION (defn), diff --git a/gcc/testsuite/g++.dg/modules/enum-10_a.H b/gcc/testsuite/g++.dg/modules/enum-10_a.H new file mode 100644 index 00000000000..fb7d10ad3b6 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/enum-10_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-10_b.C b/gcc/testsuite/g++.dg/modules/enum-10_b.C new file mode 100644 index 00000000000..76dc3152963 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/enum-10_b.C @@ -0,0 +1,6 @@ +// PR c++/106848 +// { dg-additional-options "-fmodules-ts -g" } + +import "enum-10_a.H"; + +memory_order x = memory_order_seq_cst; diff --git a/gcc/testsuite/g++.dg/modules/enum-11_a.H b/gcc/testsuite/g++.dg/modules/enum-11_a.H new file mode 100644 index 00000000000..1aecabfd0bd --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/enum-11_a.H @@ -0,0 +1,5 @@ +// PR c++/102600 +// { dg-additional-options -fmodule-header } +// { dg-module-cmi {} } + +enum class byte : unsigned char { }; diff --git a/gcc/testsuite/g++.dg/modules/enum-11_b.C b/gcc/testsuite/g++.dg/modules/enum-11_b.C new file mode 100644 index 00000000000..4d77cab8953 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/enum-11_b.C @@ -0,0 +1,8 @@ +// PR c++/102600 +// { dg-additional-options -fmodules-ts } + +import "enum-11_a.H"; + +void push(byte) {} +void write(char v) { push(static_cast<byte>(v)); } +int main() { write(char{}); } 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..0dd4a0f2fb1 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/enum-9_a.H @@ -0,0 +1,13 @@ +// PR c++/106848 +// { dg-additional-options -fmodule-header } +// { dg-module-cmi {} } + +template<typename _T1> +struct pair { + using type = void(*)(const _T1&); +}; + +struct _ScannerBase { + enum _TokenT { _S_token_anychar }; + pair<_TokenT> _M_token_tbl; +}; 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..95e2812b81e --- /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"; + +_ScannerBase s;
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; + } 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;