Re: [PATCH] c++ modules: verify_type failure with typedef enum [PR106848]

[Nathan Sidwell <nathan@acm.org>]

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

-- 
Nathan Sidwell