[2/2] c++/modules: ICE with modular fmtlib
Checks
Commit Message
Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look
OK for trunk?
I'll try to reduce and add testcases overnight for these separate bugs
before pushing.
-- >8 --
Building modular fmtlib triggered two small modules bugs in C++23 and
C++26 mode respectively (due to libstdc++ header differences).
The first is that a TEMPLATE_DECL having DECL_LANG_SPECIFIC doesn't
necessarily imply that its DECL_TEMPLATE_RESULT has DECL_LANG_SPECIFIC.
So we need to use STRIP_TEMPLATE consistently; this is a follow-up to
r12-7187-gdb84f382ae3dc2.
The second is that get_originating_module_decl was ICEing on class-scope
enumerators injected via using-enum.
gcc/cp/ChangeLog:
* module.cc (depset::hash::add_specializations): Use
STRIP_TEMPLATE consistently.
(get_originating_module_decl): Handle class-scope CONST_DECL.
---
gcc/cp/module.cc | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
Comments
On Fri, 9 Feb 2024, Patrick Palka wrote:
> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look
> OK for trunk?
>
> I'll try to reduce and add testcases overnight for these separate bugs
> before pushing.
>
> -- >8 --
>
> Building modular fmtlib triggered two small modules bugs in C++23 and
> C++26 mode respectively (due to libstdc++ header differences).
>
> The first is that a TEMPLATE_DECL having DECL_LANG_SPECIFIC doesn't
> necessarily imply that its DECL_TEMPLATE_RESULT has DECL_LANG_SPECIFIC.
> So we need to use STRIP_TEMPLATE consistently; this is a follow-up to
> r12-7187-gdb84f382ae3dc2.
>
> The second is that get_originating_module_decl was ICEing on class-scope
> enumerators injected via using-enum.
>
> gcc/cp/ChangeLog:
>
> * module.cc (depset::hash::add_specializations): Use
> STRIP_TEMPLATE consistently.
> (get_originating_module_decl): Handle class-scope CONST_DECL.
> ---
> gcc/cp/module.cc | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
> index 3c2fef0e3f4..659fa03dae1 100644
> --- a/gcc/cp/module.cc
> +++ b/gcc/cp/module.cc
> @@ -13248,7 +13248,7 @@ depset::hash::add_specializations (bool decl_p)
> if (use_tpl == 1)
> /* Implicit instantiations only walked if we reach them. */
> needs_reaching = true;
> - else if (!DECL_LANG_SPECIFIC (spec)
> + else if (!DECL_LANG_SPECIFIC (STRIP_TEMPLATE (spec))
> || !DECL_MODULE_PURVIEW_P (STRIP_TEMPLATE (spec)))
> /* Likewise, GMF explicit or partial specializations. */
> needs_reaching = true;
> @@ -18708,7 +18708,8 @@ get_originating_module_decl (tree decl)
> && (TREE_CODE (DECL_CONTEXT (decl)) == ENUMERAL_TYPE))
> decl = TYPE_NAME (DECL_CONTEXT (decl));
> else if (TREE_CODE (decl) == FIELD_DECL
> - || TREE_CODE (decl) == USING_DECL)
> + || TREE_CODE (decl) == USING_DECL
> + || TREE_CODE (decl) == CONST_DECL)
On second thought maybe we should test for CONST_DECL_USING_P (decl)
specifically. In other contexts a CONST_DECL could be a template
parameter, so using CONST_DECL_USING_P makes this code more readable
arguably.
> {
> decl = DECL_CONTEXT (decl);
> if (TREE_CODE (decl) != FUNCTION_DECL)
> --
> 2.43.0.561.g235986be82
>
>
On 2/9/24 17:11, Patrick Palka wrote:
> On Fri, 9 Feb 2024, Patrick Palka wrote:
>
>> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look
>> OK for trunk?
>>
>> I'll try to reduce and add testcases overnight for these separate bugs
>> before pushing.
>>
>> -- >8 --
>>
>> Building modular fmtlib triggered two small modules bugs in C++23 and
>> C++26 mode respectively (due to libstdc++ header differences).
>>
>> The first is that a TEMPLATE_DECL having DECL_LANG_SPECIFIC doesn't
>> necessarily imply that its DECL_TEMPLATE_RESULT has DECL_LANG_SPECIFIC.
>> So we need to use STRIP_TEMPLATE consistently; this is a follow-up to
>> r12-7187-gdb84f382ae3dc2.
>>
>> The second is that get_originating_module_decl was ICEing on class-scope
>> enumerators injected via using-enum.
>>
>> gcc/cp/ChangeLog:
>>
>> * module.cc (depset::hash::add_specializations): Use
>> STRIP_TEMPLATE consistently.
>> (get_originating_module_decl): Handle class-scope CONST_DECL.
>> ---
>> gcc/cp/module.cc | 5 +++--
>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
>> index 3c2fef0e3f4..659fa03dae1 100644
>> --- a/gcc/cp/module.cc
>> +++ b/gcc/cp/module.cc
>> @@ -13248,7 +13248,7 @@ depset::hash::add_specializations (bool decl_p)
>> if (use_tpl == 1)
>> /* Implicit instantiations only walked if we reach them. */
>> needs_reaching = true;
>> - else if (!DECL_LANG_SPECIFIC (spec)
>> + else if (!DECL_LANG_SPECIFIC (STRIP_TEMPLATE (spec))
>> || !DECL_MODULE_PURVIEW_P (STRIP_TEMPLATE (spec)))
>> /* Likewise, GMF explicit or partial specializations. */
>> needs_reaching = true;
>> @@ -18708,7 +18708,8 @@ get_originating_module_decl (tree decl)
>> && (TREE_CODE (DECL_CONTEXT (decl)) == ENUMERAL_TYPE))
>> decl = TYPE_NAME (DECL_CONTEXT (decl));
>> else if (TREE_CODE (decl) == FIELD_DECL
>> - || TREE_CODE (decl) == USING_DECL)
>> + || TREE_CODE (decl) == USING_DECL
>> + || TREE_CODE (decl) == CONST_DECL)
>
> On second thought maybe we should test for CONST_DECL_USING_P (decl)
> specifically. In other contexts a CONST_DECL could be a template
> parameter, so using CONST_DECL_USING_P makes this code more readable
> arguably.
That makes sense.
Jason
On Sat, 10 Feb 2024, Jason Merrill wrote:
> On 2/9/24 17:11, Patrick Palka wrote:
> > On Fri, 9 Feb 2024, Patrick Palka wrote:
> >
> > > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look
> > > OK for trunk?
> > >
> > > I'll try to reduce and add testcases overnight for these separate bugs
> > > before pushing.
> > >
> > > -- >8 --
> > >
> > > Building modular fmtlib triggered two small modules bugs in C++23 and
> > > C++26 mode respectively (due to libstdc++ header differences).
> > >
> > > The first is that a TEMPLATE_DECL having DECL_LANG_SPECIFIC doesn't
> > > necessarily imply that its DECL_TEMPLATE_RESULT has DECL_LANG_SPECIFIC.
> > > So we need to use STRIP_TEMPLATE consistently; this is a follow-up to
> > > r12-7187-gdb84f382ae3dc2.
> > >
> > > The second is that get_originating_module_decl was ICEing on class-scope
> > > enumerators injected via using-enum.
> > >
> > > gcc/cp/ChangeLog:
> > >
> > > * module.cc (depset::hash::add_specializations): Use
> > > STRIP_TEMPLATE consistently.
> > > (get_originating_module_decl): Handle class-scope CONST_DECL.
> > > ---
> > > gcc/cp/module.cc | 5 +++--
> > > 1 file changed, 3 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
> > > index 3c2fef0e3f4..659fa03dae1 100644
> > > --- a/gcc/cp/module.cc
> > > +++ b/gcc/cp/module.cc
> > > @@ -13248,7 +13248,7 @@ depset::hash::add_specializations (bool decl_p)
> > > if (use_tpl == 1)
> > > /* Implicit instantiations only walked if we reach them. */
> > > needs_reaching = true;
> > > - else if (!DECL_LANG_SPECIFIC (spec)
> > > + else if (!DECL_LANG_SPECIFIC (STRIP_TEMPLATE (spec))
> > > || !DECL_MODULE_PURVIEW_P (STRIP_TEMPLATE (spec)))
> > > /* Likewise, GMF explicit or partial specializations. */
> > > needs_reaching = true;
> > > @@ -18708,7 +18708,8 @@ get_originating_module_decl (tree decl)
> > > && (TREE_CODE (DECL_CONTEXT (decl)) == ENUMERAL_TYPE))
> > > decl = TYPE_NAME (DECL_CONTEXT (decl));
> > > else if (TREE_CODE (decl) == FIELD_DECL
> > > - || TREE_CODE (decl) == USING_DECL)
> > > + || TREE_CODE (decl) == USING_DECL
> > > + || TREE_CODE (decl) == CONST_DECL)
> >
> > On second thought maybe we should test for CONST_DECL_USING_P (decl)
> > specifically. In other contexts a CONST_DECL could be a template
> > parameter, so using CONST_DECL_USING_P makes this code more readable
> > arguably.
>
> That makes sense.
Like so? Now with reduced testcases:
-- >8 --
Subject: [PATCH] c++/modules: ICEs with modular fmtlib
gcc/cp/ChangeLog:
* module.cc (depset::hash::add_specializations): Use
STRIP_TEMPLATE consistently.
(get_originating_module_decl): Handle class-scope CONST_DECL.
gcc/testsuite/ChangeLog:
* gcc/testsuite/g++.dg/modules/friend-6_a.C: New test.
* gcc/testsuite/g++.dg/modules/using-enum-3_a.C: New test.
* gcc/testsuite/g++.dg/modules/using-enum-3_b.C: New test.
---
gcc/cp/module.cc | 5 +++--
gcc/testsuite/g++.dg/modules/friend-6_a.C | 11 +++++++++++
gcc/testsuite/g++.dg/modules/using-enum-3_a.C | 10 ++++++++++
gcc/testsuite/g++.dg/modules/using-enum-3_b.C | 5 +++++
4 files changed, 29 insertions(+), 2 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/modules/friend-6_a.C
create mode 100644 gcc/testsuite/g++.dg/modules/using-enum-3_a.C
create mode 100644 gcc/testsuite/g++.dg/modules/using-enum-3_b.C
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index 3c2fef0e3f4..86e43aee542 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -13248,7 +13248,7 @@ depset::hash::add_specializations (bool decl_p)
if (use_tpl == 1)
/* Implicit instantiations only walked if we reach them. */
needs_reaching = true;
- else if (!DECL_LANG_SPECIFIC (spec)
+ else if (!DECL_LANG_SPECIFIC (STRIP_TEMPLATE (spec))
|| !DECL_MODULE_PURVIEW_P (STRIP_TEMPLATE (spec)))
/* Likewise, GMF explicit or partial specializations. */
needs_reaching = true;
@@ -18708,7 +18708,8 @@ get_originating_module_decl (tree decl)
&& (TREE_CODE (DECL_CONTEXT (decl)) == ENUMERAL_TYPE))
decl = TYPE_NAME (DECL_CONTEXT (decl));
else if (TREE_CODE (decl) == FIELD_DECL
- || TREE_CODE (decl) == USING_DECL)
+ || TREE_CODE (decl) == USING_DECL
+ || CONST_DECL_USING_P (decl))
{
decl = DECL_CONTEXT (decl);
if (TREE_CODE (decl) != FUNCTION_DECL)
diff --git a/gcc/testsuite/g++.dg/modules/friend-6_a.C b/gcc/testsuite/g++.dg/modules/friend-6_a.C
new file mode 100644
index 00000000000..3b3d714b3f3
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/friend-6_a.C
@@ -0,0 +1,11 @@
+// { dg-additional-options "-fmodules-ts -Wno-pedantic" }
+// { dg-module-cmi friend_6 }
+
+module;
+# 1 "" 1
+template<class> struct Trans_NS___cxx11_basic_string {
+ template<class> friend class basic_stringbuf;
+};
+template struct Trans_NS___cxx11_basic_string<char>;
+# 6 "" 2
+export module friend_6;
diff --git a/gcc/testsuite/g++.dg/modules/using-enum-3_a.C b/gcc/testsuite/g++.dg/modules/using-enum-3_a.C
new file mode 100644
index 00000000000..e2651f36462
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/using-enum-3_a.C
@@ -0,0 +1,10 @@
+// { dg-do compile { target c++20 } }
+// { dg-additional-options "-fmodules-ts" }
+// { dg-module-cmi using_enum_3 }
+
+export module using_enum_3;
+export
+struct text_encoding {
+ enum class id { CP50220 };
+ using enum id;
+};
diff --git a/gcc/testsuite/g++.dg/modules/using-enum-3_b.C b/gcc/testsuite/g++.dg/modules/using-enum-3_b.C
new file mode 100644
index 00000000000..719dc0f2b84
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/using-enum-3_b.C
@@ -0,0 +1,5 @@
+// { dg-do compile { target c++20 } }
+// { dg-additional-options "-fmodules-ts" }
+import using_enum_3;
+
+static_assert(text_encoding::id::CP50220 == text_encoding::CP50220);
On 2/10/24 13:37, Patrick Palka wrote:
> On Sat, 10 Feb 2024, Jason Merrill wrote:
>
>> On 2/9/24 17:11, Patrick Palka wrote:
>>> On Fri, 9 Feb 2024, Patrick Palka wrote:
>>>
>>>> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look
>>>> OK for trunk?
>>>>
>>>> I'll try to reduce and add testcases overnight for these separate bugs
>>>> before pushing.
>>>>
>>>> -- >8 --
>>>>
>>>> Building modular fmtlib triggered two small modules bugs in C++23 and
>>>> C++26 mode respectively (due to libstdc++ header differences).
>>>>
>>>> The first is that a TEMPLATE_DECL having DECL_LANG_SPECIFIC doesn't
>>>> necessarily imply that its DECL_TEMPLATE_RESULT has DECL_LANG_SPECIFIC.
>>>> So we need to use STRIP_TEMPLATE consistently; this is a follow-up to
>>>> r12-7187-gdb84f382ae3dc2.
>>>>
>>>> The second is that get_originating_module_decl was ICEing on class-scope
>>>> enumerators injected via using-enum.
>>>>
>>>> gcc/cp/ChangeLog:
>>>>
>>>> * module.cc (depset::hash::add_specializations): Use
>>>> STRIP_TEMPLATE consistently.
>>>> (get_originating_module_decl): Handle class-scope CONST_DECL.
>>>> ---
>>>> gcc/cp/module.cc | 5 +++--
>>>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
>>>> index 3c2fef0e3f4..659fa03dae1 100644
>>>> --- a/gcc/cp/module.cc
>>>> +++ b/gcc/cp/module.cc
>>>> @@ -13248,7 +13248,7 @@ depset::hash::add_specializations (bool decl_p)
>>>> if (use_tpl == 1)
>>>> /* Implicit instantiations only walked if we reach them. */
>>>> needs_reaching = true;
>>>> - else if (!DECL_LANG_SPECIFIC (spec)
>>>> + else if (!DECL_LANG_SPECIFIC (STRIP_TEMPLATE (spec))
>>>> || !DECL_MODULE_PURVIEW_P (STRIP_TEMPLATE (spec)))
>>>> /* Likewise, GMF explicit or partial specializations. */
>>>> needs_reaching = true;
>>>> @@ -18708,7 +18708,8 @@ get_originating_module_decl (tree decl)
>>>> && (TREE_CODE (DECL_CONTEXT (decl)) == ENUMERAL_TYPE))
>>>> decl = TYPE_NAME (DECL_CONTEXT (decl));
>>>> else if (TREE_CODE (decl) == FIELD_DECL
>>>> - || TREE_CODE (decl) == USING_DECL)
>>>> + || TREE_CODE (decl) == USING_DECL
>>>> + || TREE_CODE (decl) == CONST_DECL)
>>>
>>> On second thought maybe we should test for CONST_DECL_USING_P (decl)
>>> specifically. In other contexts a CONST_DECL could be a template
>>> parameter, so using CONST_DECL_USING_P makes this code more readable
>>> arguably.
>>
>> That makes sense.
>
> Like so? Now with reduced testcases:
OK.
> -- >8 --
>
> Subject: [PATCH] c++/modules: ICEs with modular fmtlib
>
> gcc/cp/ChangeLog:
>
> * module.cc (depset::hash::add_specializations): Use
> STRIP_TEMPLATE consistently.
> (get_originating_module_decl): Handle class-scope CONST_DECL.
>
> gcc/testsuite/ChangeLog:
>
> * gcc/testsuite/g++.dg/modules/friend-6_a.C: New test.
> * gcc/testsuite/g++.dg/modules/using-enum-3_a.C: New test.
> * gcc/testsuite/g++.dg/modules/using-enum-3_b.C: New test.
> ---
> gcc/cp/module.cc | 5 +++--
> gcc/testsuite/g++.dg/modules/friend-6_a.C | 11 +++++++++++
> gcc/testsuite/g++.dg/modules/using-enum-3_a.C | 10 ++++++++++
> gcc/testsuite/g++.dg/modules/using-enum-3_b.C | 5 +++++
> 4 files changed, 29 insertions(+), 2 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/modules/friend-6_a.C
> create mode 100644 gcc/testsuite/g++.dg/modules/using-enum-3_a.C
> create mode 100644 gcc/testsuite/g++.dg/modules/using-enum-3_b.C
>
> diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
> index 3c2fef0e3f4..86e43aee542 100644
> --- a/gcc/cp/module.cc
> +++ b/gcc/cp/module.cc
> @@ -13248,7 +13248,7 @@ depset::hash::add_specializations (bool decl_p)
> if (use_tpl == 1)
> /* Implicit instantiations only walked if we reach them. */
> needs_reaching = true;
> - else if (!DECL_LANG_SPECIFIC (spec)
> + else if (!DECL_LANG_SPECIFIC (STRIP_TEMPLATE (spec))
> || !DECL_MODULE_PURVIEW_P (STRIP_TEMPLATE (spec)))
> /* Likewise, GMF explicit or partial specializations. */
> needs_reaching = true;
> @@ -18708,7 +18708,8 @@ get_originating_module_decl (tree decl)
> && (TREE_CODE (DECL_CONTEXT (decl)) == ENUMERAL_TYPE))
> decl = TYPE_NAME (DECL_CONTEXT (decl));
> else if (TREE_CODE (decl) == FIELD_DECL
> - || TREE_CODE (decl) == USING_DECL)
> + || TREE_CODE (decl) == USING_DECL
> + || CONST_DECL_USING_P (decl))
> {
> decl = DECL_CONTEXT (decl);
> if (TREE_CODE (decl) != FUNCTION_DECL)
> diff --git a/gcc/testsuite/g++.dg/modules/friend-6_a.C b/gcc/testsuite/g++.dg/modules/friend-6_a.C
> new file mode 100644
> index 00000000000..3b3d714b3f3
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/friend-6_a.C
> @@ -0,0 +1,11 @@
> +// { dg-additional-options "-fmodules-ts -Wno-pedantic" }
> +// { dg-module-cmi friend_6 }
> +
> +module;
> +# 1 "" 1
> +template<class> struct Trans_NS___cxx11_basic_string {
> + template<class> friend class basic_stringbuf;
> +};
> +template struct Trans_NS___cxx11_basic_string<char>;
> +# 6 "" 2
> +export module friend_6;
> diff --git a/gcc/testsuite/g++.dg/modules/using-enum-3_a.C b/gcc/testsuite/g++.dg/modules/using-enum-3_a.C
> new file mode 100644
> index 00000000000..e2651f36462
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/using-enum-3_a.C
> @@ -0,0 +1,10 @@
> +// { dg-do compile { target c++20 } }
> +// { dg-additional-options "-fmodules-ts" }
> +// { dg-module-cmi using_enum_3 }
> +
> +export module using_enum_3;
> +export
> +struct text_encoding {
> + enum class id { CP50220 };
> + using enum id;
> +};
> diff --git a/gcc/testsuite/g++.dg/modules/using-enum-3_b.C b/gcc/testsuite/g++.dg/modules/using-enum-3_b.C
> new file mode 100644
> index 00000000000..719dc0f2b84
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/using-enum-3_b.C
> @@ -0,0 +1,5 @@
> +// { dg-do compile { target c++20 } }
> +// { dg-additional-options "-fmodules-ts" }
> +import using_enum_3;
> +
> +static_assert(text_encoding::id::CP50220 == text_encoding::CP50220);
@@ -13248,7 +13248,7 @@ depset::hash::add_specializations (bool decl_p)
if (use_tpl == 1)
/* Implicit instantiations only walked if we reach them. */
needs_reaching = true;
- else if (!DECL_LANG_SPECIFIC (spec)
+ else if (!DECL_LANG_SPECIFIC (STRIP_TEMPLATE (spec))
|| !DECL_MODULE_PURVIEW_P (STRIP_TEMPLATE (spec)))
/* Likewise, GMF explicit or partial specializations. */
needs_reaching = true;
@@ -18708,7 +18708,8 @@ get_originating_module_decl (tree decl)
&& (TREE_CODE (DECL_CONTEXT (decl)) == ENUMERAL_TYPE))
decl = TYPE_NAME (DECL_CONTEXT (decl));
else if (TREE_CODE (decl) == FIELD_DECL
- || TREE_CODE (decl) == USING_DECL)
+ || TREE_CODE (decl) == USING_DECL
+ || TREE_CODE (decl) == CONST_DECL)
{
decl = DECL_CONTEXT (decl);
if (TREE_CODE (decl) != FUNCTION_DECL)