[PATCH] c++: canonicity of fn types w/ complex eh specs [PR115159]

[PATCH] c++: canonicity of fn types w/ complex eh specs [PR115159]

[Patrick Palka]

Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look
OK for trunk?

Alternatively, I considered fixing this by incrementing
comparing_specializations around the call to comp_except_specs in
cp_check_qualified_type, but generally for types whose identity
depends on whether comparing_specializations is set we need to
use structural equality anyway IIUC.

Or maybe it isn't right to fix this outside of modules, and we should
instead make modules cope with this cross-function function parameter
reference?  I briefly tried looking into this but didn't get very far.

-- >8 --

Here the member functions QList::g and QList::h are given the same
function type since their exception specifications are equivalent
according to cp_tree_equal.  In doing so however this means that the
type of QList::h refers to a function parameter from QList::g, which
ends up confusing modules streaming.

I'm not sure if modules can be fixed to handle this situation, but
regardless it seems weird in principle that a function parameter can
escape in such a way.  The analogous situation with a trailing return
type and decltype

  auto g(QList &other) -> decltype(f(other));
  auto h(QList &other) -> decltype(f(other));

behaves better because we don't canonicalize decltype, and so the
function types of g and h are non-canonical and therefore not shared.

In light of this, it seems natural to treat function types with complex
eh specs as non-canonical as well so that each such function declaration
is given a unique function/method type node.  The main benefit of type
canonicalization is to speed up repeated type comparisons, but it should
rare for us to repeatedly compare two otherwise compatible function
types with complex exception specifications, so foregoing canonicalization
should be harmless IIUC.  On the other hand this change simplifies the
code responsible for adjusting unparsed eh spec variants.

	PR c++/115159

gcc/cp/ChangeLog:

	* tree.cc (build_cp_fntype_variant): Don't reuse a variant with
	a complex exception specification.  Always use structural
	equality in that case.
	(fixup_deferred_exception_variants): Always use structural
	equality for adjusted variants.

gcc/testsuite/ChangeLog:

	* g++.dg/modules/noexcept-2_a.H: New test.
	* g++.dg/modules/noexcept-2_b.C: New test.
---
 gcc/cp/tree.cc                              | 70 +++++++--------------
 gcc/testsuite/g++.dg/modules/noexcept-2_a.H | 24 +++++++
 gcc/testsuite/g++.dg/modules/noexcept-2_b.C |  4 ++
 3 files changed, 51 insertions(+), 47 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/modules/noexcept-2_a.H
 create mode 100644 gcc/testsuite/g++.dg/modules/noexcept-2_b.C

diff --git a/gcc/cp/tree.cc b/gcc/cp/tree.cc
index 9d37d255d8d..7987c01520d 100644
--- a/gcc/cp/tree.cc
+++ b/gcc/cp/tree.cc
@@ -2761,16 +2761,27 @@ build_cp_fntype_variant (tree type, cp_ref_qualifier rqual,
 {
   cp_cv_quals type_quals = TYPE_QUALS (type);
 
-  if (cp_check_qualified_type (type, type, type_quals, rqual, raises, late))
-    return type;
+  /* Canonicalize the exception specification.  */
+  tree cr = flag_noexcept_type ? canonical_eh_spec (raises) : NULL_TREE;
+  /* For a complex exception specification, always create a distinct
+     non-canonical variant for simplicity.  This also prevents noexcept-specs
+     that are in terms of a function parameter from getting shared with an
+     another function.  */
+  bool complex_p = (cr && cr != noexcept_true_spec
+		    && !UNPARSED_NOEXCEPT_SPEC_P (cr));
+  if (!complex_p)
+    {
+      if (cp_check_qualified_type (type, type, type_quals, rqual, raises, late))
+	return type;
 
-  tree v = TYPE_MAIN_VARIANT (type);
-  for (; v; v = TYPE_NEXT_VARIANT (v))
-    if (cp_check_qualified_type (v, type, type_quals, rqual, raises, late))
-      return v;
+      tree v = TYPE_MAIN_VARIANT (type);
+      for (; v; v = TYPE_NEXT_VARIANT (v))
+	if (cp_check_qualified_type (v, type, type_quals, rqual, raises, late))
+	  return v;
+    }
 
   /* Need to build a new variant.  */
-  v = build_variant_type_copy (type);
+  tree v = build_variant_type_copy (type);
   if (!TYPE_DEPENDENT_P (v))
     /* We no longer know that it's not type-dependent.  */
     TYPE_DEPENDENT_P_VALID (v) = false;
@@ -2791,10 +2802,7 @@ build_cp_fntype_variant (tree type, cp_ref_qualifier rqual,
       break;
     }
 
-  /* Canonicalize the exception specification.  */
-  tree cr = flag_noexcept_type ? canonical_eh_spec (raises) : NULL_TREE;
-
-  if (TYPE_STRUCTURAL_EQUALITY_P (type))
+  if (TYPE_STRUCTURAL_EQUALITY_P (type) || complex_p)
     /* Propagate structural equality. */
     SET_TYPE_STRUCTURAL_EQUALITY (v);
   else if (TYPE_CANONICAL (type) != type || cr != raises || late)
@@ -2812,55 +2820,23 @@ build_cp_fntype_variant (tree type, cp_ref_qualifier rqual,
 /* TYPE is a function or method type with a deferred exception
    specification that has been parsed to RAISES.  Fixup all the type
    variants that are affected in place.  Via decltype &| noexcept
-   tricks, the unparsed spec could have escaped into the type system.
-   The general case is hard to fixup canonical types for.  */
+   tricks, the unparsed spec could have escaped into the type system.  */
 
 void
 fixup_deferred_exception_variants (tree type, tree raises)
 {
   tree original = TYPE_RAISES_EXCEPTIONS (type);
-  tree cr = flag_noexcept_type ? canonical_eh_spec (raises) : NULL_TREE;
 
   gcc_checking_assert (UNPARSED_NOEXCEPT_SPEC_P (original));
 
-  /* Though sucky, this walk will process the canonical variants
-     first.  */
-  tree prev = NULL_TREE;
   for (tree variant = TYPE_MAIN_VARIANT (type);
-       variant; prev = variant, variant = TYPE_NEXT_VARIANT (variant))
+       variant; variant = TYPE_NEXT_VARIANT (variant))
     if (TYPE_RAISES_EXCEPTIONS (variant) == original)
       {
 	gcc_checking_assert (variant != TYPE_MAIN_VARIANT (type));
 
-	if (!TYPE_STRUCTURAL_EQUALITY_P (variant))
-	  {
-	    cp_cv_quals var_quals = TYPE_QUALS (variant);
-	    cp_ref_qualifier rqual = type_memfn_rqual (variant);
-
-	    /* If VARIANT would become a dup (cp_check_qualified_type-wise)
-	       of an existing variant in the variant list of TYPE after its
-	       exception specification has been parsed, elide it.  Otherwise,
-	       build_cp_fntype_variant could use it, leading to "canonical
-	       types differ for identical types."  */
-	    tree v = TYPE_MAIN_VARIANT (type);
-	    for (; v; v = TYPE_NEXT_VARIANT (v))
-	      if (cp_check_qualified_type (v, variant, var_quals,
-					   rqual, cr, false))
-		{
-		  /* The main variant will not match V, so PREV will never
-		     be null.  */
-		  TYPE_NEXT_VARIANT (prev) = TYPE_NEXT_VARIANT (variant);
-		  break;
-		}
-	    TYPE_RAISES_EXCEPTIONS (variant) = raises;
-
-	    if (!v)
-	      v = build_cp_fntype_variant (TYPE_CANONICAL (variant),
-					   rqual, cr, false);
-	    TYPE_CANONICAL (variant) = TYPE_CANONICAL (v);
-	  }
-	else
-	  TYPE_RAISES_EXCEPTIONS (variant) = raises;
+	SET_TYPE_STRUCTURAL_EQUALITY (variant);
+	TYPE_RAISES_EXCEPTIONS (variant) = raises;
 
 	if (!TYPE_DEPENDENT_P (variant))
 	  /* We no longer know that it's not type-dependent.  */
diff --git a/gcc/testsuite/g++.dg/modules/noexcept-2_a.H b/gcc/testsuite/g++.dg/modules/noexcept-2_a.H
new file mode 100644
index 00000000000..b7144f42d7e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/noexcept-2_a.H
@@ -0,0 +1,24 @@
+// PR c++/115159
+// { dg-additional-options -fmodule-header }
+// { dg-module-cmi {} }
+
+struct QDebug;
+
+template<class T> void f(T);
+
+template<class T> struct QList {
+  QDebug g(QList &other) noexcept(noexcept(f(other)));
+  QDebug h(QList &other) noexcept(noexcept(f(other)));
+};
+
+struct QObjectData {
+  QList<int> children;
+};
+
+struct QIODevice {
+  QObjectData d_ptr;
+};
+
+struct QDebug {
+  QDebug(QIODevice);
+};
diff --git a/gcc/testsuite/g++.dg/modules/noexcept-2_b.C b/gcc/testsuite/g++.dg/modules/noexcept-2_b.C
new file mode 100644
index 00000000000..d34c63add10
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/noexcept-2_b.C
@@ -0,0 +1,4 @@
+// PR c++/115159
+// { dg-additional-options "-fmodules-ts -fno-module-lazy" }
+
+import "noexcept-2_a.H";
-- 
2.45.1.216.g4365c6fcf9

Re: [PATCH] c++: canonicity of fn types w/ complex eh specs [PR115159]

[Jason Merrill]

On 5/21/24 15:36, Patrick Palka wrote:
> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look
> OK for trunk?
> 
> Alternatively, I considered fixing this by incrementing
> comparing_specializations around the call to comp_except_specs in
> cp_check_qualified_type, but generally for types whose identity
> depends on whether comparing_specializations is set we need to
> use structural equality anyway IIUC.

Why not both?

> +  bool complex_p = (cr && cr != noexcept_true_spec
> +		    && !UNPARSED_NOEXCEPT_SPEC_P (cr));

Why treat unparsed specs differently from parsed ones?

Jason

Re: [PATCH] c++: canonicity of fn types w/ complex eh specs [PR115159]

[Patrick Palka]

On Tue, 21 May 2024, Jason Merrill wrote:

> On 5/21/24 15:36, Patrick Palka wrote:
> > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look
> > OK for trunk?
> > 
> > Alternatively, I considered fixing this by incrementing
> > comparing_specializations around the call to comp_except_specs in
> > cp_check_qualified_type, but generally for types whose identity
> > depends on whether comparing_specializations is set we need to
> > use structural equality anyway IIUC.
> 
> Why not both?

I figured the latter change isn't necessary/observable since
comparing_specializations would only make a difference for complex
exception specifications, and with this patch we won't even call
cp_check_qualified_type on a complex eh spec.

> 
> > +  bool complex_p = (cr && cr != noexcept_true_spec
> > +		    && !UNPARSED_NOEXCEPT_SPEC_P (cr));
> 
> Why treat unparsed specs differently from parsed ones?

Unparsed specs are unique according to cp_tree_equal, so in turn
function types with unparsed specs are unique, so it should be safe to
treat such types as canonical.  I'm not sure if this optimization
matters though; I'm happy to remove this case.

> 
> Jason
> 
> 

Re: [PATCH] c++: canonicity of fn types w/ complex eh specs [PR115159]

[Patrick Palka]

On Tue, 21 May 2024, Patrick Palka wrote:

> On Tue, 21 May 2024, Jason Merrill wrote:
> 
> > On 5/21/24 15:36, Patrick Palka wrote:
> > > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look
> > > OK for trunk?
> > > 
> > > Alternatively, I considered fixing this by incrementing
> > > comparing_specializations around the call to comp_except_specs in
> > > cp_check_qualified_type, but generally for types whose identity
> > > depends on whether comparing_specializations is set we need to
> > > use structural equality anyway IIUC.
> > 
> > Why not both?
> 
> I figured the latter change isn't necessary/observable since
> comparing_specializations would only make a difference for complex
> exception specifications, and with this patch we won't even call
> cp_check_qualified_type on a complex eh spec.
> 
> > 
> > > +  bool complex_p = (cr && cr != noexcept_true_spec
> > > +		    && !UNPARSED_NOEXCEPT_SPEC_P (cr));
> > 
> > Why treat unparsed specs differently from parsed ones?
> 
> Unparsed specs are unique according to cp_tree_equal, so in turn
> function types with unparsed specs are unique, so it should be safe to
> treat such types as canonical.  I'm not sure if this optimization
> matters though; I'm happy to remove this case.

FWIW if we do get rid of this case then I think in
fixup_deferred_exception_variants we can assert TYPE_STRUCTURAL_EQUALITY_P
is already set instead of having to set it.

> 
> > 
> > Jason
> > 
> > 
> 

Re: [PATCH] c++: canonicity of fn types w/ complex eh specs [PR115159]

[Jason Merrill]

On 5/21/24 17:27, Patrick Palka wrote:
> On Tue, 21 May 2024, Jason Merrill wrote:
> 
>> On 5/21/24 15:36, Patrick Palka wrote:
>>> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look
>>> OK for trunk?
>>>
>>> Alternatively, I considered fixing this by incrementing
>>> comparing_specializations around the call to comp_except_specs in
>>> cp_check_qualified_type, but generally for types whose identity
>>> depends on whether comparing_specializations is set we need to
>>> use structural equality anyway IIUC.
>>
>> Why not both?
> 
> I figured the latter change isn't necessary/observable since
> comparing_specializations would only make a difference for complex
> exception specifications, and with this patch we won't even call
> cp_check_qualified_type on a complex eh spec.

My concern is that if we're building a function type multiple times with 
the same noexcept-spec, this patch would mean creating multiple 
equivalent function types instead of reusing one already created for the 
same function.

>>> +  bool complex_p = (cr && cr != noexcept_true_spec
>>> +		    && !UNPARSED_NOEXCEPT_SPEC_P (cr));
>>
>> Why treat unparsed specs differently from parsed ones?
> 
> Unparsed specs are unique according to cp_tree_equal, so in turn
> function types with unparsed specs are unique, so it should be safe to
> treat such types as canonical.  I'm not sure if this optimization
> matters though; I'm happy to remove this case.

The idea that this optimization could make a difference raised the 
concern above.

Jason

Re: [PATCH] c++: canonicity of fn types w/ complex eh specs [PR115159]

[Patrick Palka]

On Tue, 21 May 2024, Jason Merrill wrote:

> On 5/21/24 17:27, Patrick Palka wrote:
> > On Tue, 21 May 2024, Jason Merrill wrote:
> > 
> > > On 5/21/24 15:36, Patrick Palka wrote:
> > > > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look
> > > > OK for trunk?
> > > > 
> > > > Alternatively, I considered fixing this by incrementing
> > > > comparing_specializations around the call to comp_except_specs in
> > > > cp_check_qualified_type, but generally for types whose identity
> > > > depends on whether comparing_specializations is set we need to
> > > > use structural equality anyway IIUC.
> > > 
> > > Why not both?
> > 
> > I figured the latter change isn't necessary/observable since
> > comparing_specializations would only make a difference for complex
> > exception specifications, and with this patch we won't even call
> > cp_check_qualified_type on a complex eh spec.
> 
> My concern is that if we're building a function type multiple times with the
> same noexcept-spec, this patch would mean creating multiple equivalent
> function types instead of reusing one already created for the same function.
> 
> > > > +  bool complex_p = (cr && cr != noexcept_true_spec
> > > > +		    && !UNPARSED_NOEXCEPT_SPEC_P (cr));
> > > 
> > > Why treat unparsed specs differently from parsed ones?
> > 
> > Unparsed specs are unique according to cp_tree_equal, so in turn
> > function types with unparsed specs are unique, so it should be safe to
> > treat such types as canonical.  I'm not sure if this optimization
> > matters though; I'm happy to remove this case.
> 
> The idea that this optimization could make a difference raised the concern
> above.

Aha, makes sense.  To that end it seems we could strengthen the ce_exact
in comp_except_specs to require == instead of cp_tree_equal equality
when comparing two noexcept-specs; the only ce_exact callers are
cp_check_qualified_type and cxx_type_hash_eq, which should be fine with
that strengthening.  This way, we at least do try to reuse a variant if
the (complex or unparsed) noexcept-spec is exactly the same.

Like so?

-- >8 --

Subject: [PATCH] c++: canonicity of fn types w/ complex eh specs [PR115159]

Here the member functions QList::g and QList::h are given the same
function type since their exception specifications are equivalent
according to cp_tree_equal.  In doing so however this means that the
type of QList::h refers to a function parameter from QList::g, which
ends up confusing modules streaming.

I'm not sure if modules can be fixed to handle this situation, but
regardless it seems weird in principle that a function parameter can
escape in such a way.  The analogous situation with a trailing return
type and decltype

  auto g(QList &other) -> decltype(f(other));
  auto h(QList &other) -> decltype(f(other));

behaves better because we don't canonicalize decltype, and so the
function types of g and h are non-canonical and therefore not shared.

In light of this, it seems natural to treat function types with complex
eh specs as non-canonical as well so that each such function declaration
is given a unique function/method type node.  The main benefit of type
canonicalization is to speed up repeated type comparisons, but it should
rare for us to repeatedly compare two otherwise compatible function
types with complex exception specifications, so foregoing canonicalization
should not cause any problems.

To that end, this patch strengthens the ce_exact case of comp_except_specs
to require identity instead of equivalence of the exception specification
so that build_cp_fntype_variant doesn't reuse a variant when it shouldn't.
And in build_cp_fntype_variant we need to use structural equality for types
with a complex eh spec.  In turn we could simplify the code responsible
for adjusting unparsed eh spec variants.

	PR c++/115159

gcc/cp/ChangeLog:

	* tree.cc (build_cp_fntype_variant): Always use structural
	equality for types with a complex exception specification.
	(fixup_deferred_exception_variants): Always use structural
	equality for adjusted variants.
	* typeck.cc (comp_except_specs): Require == instead of
	cp_tree_equal for noexcept-spec comparison in the ce_exact case.

gcc/testsuite/ChangeLog:

	* g++.dg/modules/noexcept-2_a.H: New test.
	* g++.dg/modules/noexcept-2_b.C: New test.
---
 gcc/cp/tree.cc                              | 47 +++++----------------
 gcc/cp/typeck.cc                            |  4 +-
 gcc/testsuite/g++.dg/modules/noexcept-2_a.H | 24 +++++++++++
 gcc/testsuite/g++.dg/modules/noexcept-2_b.C |  4 ++
 4 files changed, 41 insertions(+), 38 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/modules/noexcept-2_a.H
 create mode 100644 gcc/testsuite/g++.dg/modules/noexcept-2_b.C

diff --git a/gcc/cp/tree.cc b/gcc/cp/tree.cc
index 9d37d255d8d..93a64322418 100644
--- a/gcc/cp/tree.cc
+++ b/gcc/cp/tree.cc
@@ -2794,7 +2794,12 @@ build_cp_fntype_variant (tree type, cp_ref_qualifier rqual,
   /* Canonicalize the exception specification.  */
   tree cr = flag_noexcept_type ? canonical_eh_spec (raises) : NULL_TREE;
 
-  if (TYPE_STRUCTURAL_EQUALITY_P (type))
+  /* Always use structural equality for function types with a complex
+     exception specification since their identity may depend on e.g.
+     whether comparing_specializations is set.  */
+  bool complex_eh_spec_p = (cr && cr != noexcept_true_spec
+			    && !UNPARSED_NOEXCEPT_SPEC_P (cr));
+  if (TYPE_STRUCTURAL_EQUALITY_P (type) || complex_eh_spec_p)
     /* Propagate structural equality. */
     SET_TYPE_STRUCTURAL_EQUALITY (v);
   else if (TYPE_CANONICAL (type) != type || cr != raises || late)
@@ -2812,55 +2817,23 @@ build_cp_fntype_variant (tree type, cp_ref_qualifier rqual,
 /* TYPE is a function or method type with a deferred exception
    specification that has been parsed to RAISES.  Fixup all the type
    variants that are affected in place.  Via decltype &| noexcept
-   tricks, the unparsed spec could have escaped into the type system.
-   The general case is hard to fixup canonical types for.  */
+   tricks, the unparsed spec could have escaped into the type system.  */
 
 void
 fixup_deferred_exception_variants (tree type, tree raises)
 {
   tree original = TYPE_RAISES_EXCEPTIONS (type);
-  tree cr = flag_noexcept_type ? canonical_eh_spec (raises) : NULL_TREE;
 
   gcc_checking_assert (UNPARSED_NOEXCEPT_SPEC_P (original));
 
-  /* Though sucky, this walk will process the canonical variants
-     first.  */
-  tree prev = NULL_TREE;
   for (tree variant = TYPE_MAIN_VARIANT (type);
-       variant; prev = variant, variant = TYPE_NEXT_VARIANT (variant))
+       variant; variant = TYPE_NEXT_VARIANT (variant))
     if (TYPE_RAISES_EXCEPTIONS (variant) == original)
       {
 	gcc_checking_assert (variant != TYPE_MAIN_VARIANT (type));
 
-	if (!TYPE_STRUCTURAL_EQUALITY_P (variant))
-	  {
-	    cp_cv_quals var_quals = TYPE_QUALS (variant);
-	    cp_ref_qualifier rqual = type_memfn_rqual (variant);
-
-	    /* If VARIANT would become a dup (cp_check_qualified_type-wise)
-	       of an existing variant in the variant list of TYPE after its
-	       exception specification has been parsed, elide it.  Otherwise,
-	       build_cp_fntype_variant could use it, leading to "canonical
-	       types differ for identical types."  */
-	    tree v = TYPE_MAIN_VARIANT (type);
-	    for (; v; v = TYPE_NEXT_VARIANT (v))
-	      if (cp_check_qualified_type (v, variant, var_quals,
-					   rqual, cr, false))
-		{
-		  /* The main variant will not match V, so PREV will never
-		     be null.  */
-		  TYPE_NEXT_VARIANT (prev) = TYPE_NEXT_VARIANT (variant);
-		  break;
-		}
-	    TYPE_RAISES_EXCEPTIONS (variant) = raises;
-
-	    if (!v)
-	      v = build_cp_fntype_variant (TYPE_CANONICAL (variant),
-					   rqual, cr, false);
-	    TYPE_CANONICAL (variant) = TYPE_CANONICAL (v);
-	  }
-	else
-	  TYPE_RAISES_EXCEPTIONS (variant) = raises;
+	SET_TYPE_STRUCTURAL_EQUALITY (variant);
+	TYPE_RAISES_EXCEPTIONS (variant) = raises;
 
 	if (!TYPE_DEPENDENT_P (variant))
 	  /* We no longer know that it's not type-dependent.  */
diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
index 75b696e32e0..d535746fd43 100644
--- a/gcc/cp/typeck.cc
+++ b/gcc/cp/typeck.cc
@@ -1227,7 +1227,9 @@ comp_except_specs (const_tree t1, const_tree t2, int exact)
   if ((t1 && TREE_PURPOSE (t1))
       || (t2 && TREE_PURPOSE (t2)))
     return (t1 && t2
-	    && cp_tree_equal (TREE_PURPOSE (t1), TREE_PURPOSE (t2)));
+	    && (exact == ce_exact
+		? TREE_PURPOSE (t1) == TREE_PURPOSE (t2)
+		: cp_tree_equal (TREE_PURPOSE (t1), TREE_PURPOSE (t2))));
 
   if (t1 == NULL_TREE)			   /* T1 is ...  */
     return t2 == NULL_TREE || exact == ce_derived;
diff --git a/gcc/testsuite/g++.dg/modules/noexcept-2_a.H b/gcc/testsuite/g++.dg/modules/noexcept-2_a.H
new file mode 100644
index 00000000000..b7144f42d7e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/noexcept-2_a.H
@@ -0,0 +1,24 @@
+// PR c++/115159
+// { dg-additional-options -fmodule-header }
+// { dg-module-cmi {} }
+
+struct QDebug;
+
+template<class T> void f(T);
+
+template<class T> struct QList {
+  QDebug g(QList &other) noexcept(noexcept(f(other)));
+  QDebug h(QList &other) noexcept(noexcept(f(other)));
+};
+
+struct QObjectData {
+  QList<int> children;
+};
+
+struct QIODevice {
+  QObjectData d_ptr;
+};
+
+struct QDebug {
+  QDebug(QIODevice);
+};
diff --git a/gcc/testsuite/g++.dg/modules/noexcept-2_b.C b/gcc/testsuite/g++.dg/modules/noexcept-2_b.C
new file mode 100644
index 00000000000..d34c63add10
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/noexcept-2_b.C
@@ -0,0 +1,4 @@
+// PR c++/115159
+// { dg-additional-options "-fmodules-ts -fno-module-lazy" }
+
+import "noexcept-2_a.H";
-- 
2.45.1.216.g4365c6fcf9

Re: [PATCH] c++: canonicity of fn types w/ complex eh specs [PR115159]

[Jason Merrill]

On 5/21/24 21:55, Patrick Palka wrote:
> On Tue, 21 May 2024, Jason Merrill wrote:
> 
>> On 5/21/24 17:27, Patrick Palka wrote:
>>> On Tue, 21 May 2024, Jason Merrill wrote:
>>>
>>>> On 5/21/24 15:36, Patrick Palka wrote:
>>>>> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look
>>>>> OK for trunk?
>>>>>
>>>>> Alternatively, I considered fixing this by incrementing
>>>>> comparing_specializations around the call to comp_except_specs in
>>>>> cp_check_qualified_type, but generally for types whose identity
>>>>> depends on whether comparing_specializations is set we need to
>>>>> use structural equality anyway IIUC.
>>>>
>>>> Why not both?
>>>
>>> I figured the latter change isn't necessary/observable since
>>> comparing_specializations would only make a difference for complex
>>> exception specifications, and with this patch we won't even call
>>> cp_check_qualified_type on a complex eh spec.
>>
>> My concern is that if we're building a function type multiple times with the
>> same noexcept-spec, this patch would mean creating multiple equivalent
>> function types instead of reusing one already created for the same function.
>>
>>>>> +  bool complex_p = (cr && cr != noexcept_true_spec
>>>>> +		    && !UNPARSED_NOEXCEPT_SPEC_P (cr));
>>>>
>>>> Why treat unparsed specs differently from parsed ones?
>>>
>>> Unparsed specs are unique according to cp_tree_equal, so in turn
>>> function types with unparsed specs are unique, so it should be safe to
>>> treat such types as canonical.  I'm not sure if this optimization
>>> matters though; I'm happy to remove this case.
>>
>> The idea that this optimization could make a difference raised the concern
>> above.
> 
> Aha, makes sense.  To that end it seems we could strengthen the ce_exact
> in comp_except_specs to require == instead of cp_tree_equal equality
> when comparing two noexcept-specs; the only ce_exact callers are
> cp_check_qualified_type and cxx_type_hash_eq, which should be fine with
> that strengthening.  This way, we at least do try to reuse a variant if
> the (complex or unparsed) noexcept-spec is exactly the same.

Sounds good.

Given that, we probably still want to move the canonical_eh_spec up in 
build_cp_fntype_variant, and pass that to cp_check_qualified_type?

> Like so?
> 
> -- >8 --
> 
> Subject: [PATCH] c++: canonicity of fn types w/ complex eh specs [PR115159]
> 
> Here the member functions QList::g and QList::h are given the same
> function type since their exception specifications are equivalent
> according to cp_tree_equal.  In doing so however this means that the
> type of QList::h refers to a function parameter from QList::g, which
> ends up confusing modules streaming.
> 
> I'm not sure if modules can be fixed to handle this situation, but
> regardless it seems weird in principle that a function parameter can
> escape in such a way.  The analogous situation with a trailing return
> type and decltype
> 
>    auto g(QList &other) -> decltype(f(other));
>    auto h(QList &other) -> decltype(f(other));
> 
> behaves better because we don't canonicalize decltype, and so the
> function types of g and h are non-canonical and therefore not shared.
> 
> In light of this, it seems natural to treat function types with complex
> eh specs as non-canonical as well so that each such function declaration
> is given a unique function/method type node.  The main benefit of type
> canonicalization is to speed up repeated type comparisons, but it should
> rare for us to repeatedly compare two otherwise compatible function
> types with complex exception specifications, so foregoing canonicalization
> should not cause any problems.
> 
> To that end, this patch strengthens the ce_exact case of comp_except_specs
> to require identity instead of equivalence of the exception specification
> so that build_cp_fntype_variant doesn't reuse a variant when it shouldn't.
> And in build_cp_fntype_variant we need to use structural equality for types
> with a complex eh spec.  In turn we could simplify the code responsible
> for adjusting unparsed eh spec variants.
> 
> 	PR c++/115159
> 
> gcc/cp/ChangeLog:
> 
> 	* tree.cc (build_cp_fntype_variant): Always use structural
> 	equality for types with a complex exception specification.
> 	(fixup_deferred_exception_variants): Always use structural
> 	equality for adjusted variants.
> 	* typeck.cc (comp_except_specs): Require == instead of
> 	cp_tree_equal for noexcept-spec comparison in the ce_exact case.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/modules/noexcept-2_a.H: New test.
> 	* g++.dg/modules/noexcept-2_b.C: New test.
> ---
>   gcc/cp/tree.cc                              | 47 +++++----------------
>   gcc/cp/typeck.cc                            |  4 +-
>   gcc/testsuite/g++.dg/modules/noexcept-2_a.H | 24 +++++++++++
>   gcc/testsuite/g++.dg/modules/noexcept-2_b.C |  4 ++
>   4 files changed, 41 insertions(+), 38 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/modules/noexcept-2_a.H
>   create mode 100644 gcc/testsuite/g++.dg/modules/noexcept-2_b.C
> 
> diff --git a/gcc/cp/tree.cc b/gcc/cp/tree.cc
> index 9d37d255d8d..93a64322418 100644
> --- a/gcc/cp/tree.cc
> +++ b/gcc/cp/tree.cc
> @@ -2794,7 +2794,12 @@ build_cp_fntype_variant (tree type, cp_ref_qualifier rqual,
>     /* Canonicalize the exception specification.  */
>     tree cr = flag_noexcept_type ? canonical_eh_spec (raises) : NULL_TREE;
>   
> -  if (TYPE_STRUCTURAL_EQUALITY_P (type))
> +  /* Always use structural equality for function types with a complex
> +     exception specification since their identity may depend on e.g.
> +     whether comparing_specializations is set.  */
> +  bool complex_eh_spec_p = (cr && cr != noexcept_true_spec
> +			    && !UNPARSED_NOEXCEPT_SPEC_P (cr));
> +  if (TYPE_STRUCTURAL_EQUALITY_P (type) || complex_eh_spec_p)
>       /* Propagate structural equality. */
>       SET_TYPE_STRUCTURAL_EQUALITY (v);
>     else if (TYPE_CANONICAL (type) != type || cr != raises || late)
> @@ -2812,55 +2817,23 @@ build_cp_fntype_variant (tree type, cp_ref_qualifier rqual,
>   /* TYPE is a function or method type with a deferred exception
>      specification that has been parsed to RAISES.  Fixup all the type
>      variants that are affected in place.  Via decltype &| noexcept
> -   tricks, the unparsed spec could have escaped into the type system.
> -   The general case is hard to fixup canonical types for.  */
> +   tricks, the unparsed spec could have escaped into the type system.  */
>   
>   void
>   fixup_deferred_exception_variants (tree type, tree raises)
>   {
>     tree original = TYPE_RAISES_EXCEPTIONS (type);
> -  tree cr = flag_noexcept_type ? canonical_eh_spec (raises) : NULL_TREE;
>   
>     gcc_checking_assert (UNPARSED_NOEXCEPT_SPEC_P (original));
>   
> -  /* Though sucky, this walk will process the canonical variants
> -     first.  */
> -  tree prev = NULL_TREE;
>     for (tree variant = TYPE_MAIN_VARIANT (type);
> -       variant; prev = variant, variant = TYPE_NEXT_VARIANT (variant))
> +       variant; variant = TYPE_NEXT_VARIANT (variant))
>       if (TYPE_RAISES_EXCEPTIONS (variant) == original)
>         {
>   	gcc_checking_assert (variant != TYPE_MAIN_VARIANT (type));
>   
> -	if (!TYPE_STRUCTURAL_EQUALITY_P (variant))
> -	  {
> -	    cp_cv_quals var_quals = TYPE_QUALS (variant);
> -	    cp_ref_qualifier rqual = type_memfn_rqual (variant);
> -
> -	    /* If VARIANT would become a dup (cp_check_qualified_type-wise)
> -	       of an existing variant in the variant list of TYPE after its
> -	       exception specification has been parsed, elide it.  Otherwise,
> -	       build_cp_fntype_variant could use it, leading to "canonical
> -	       types differ for identical types."  */
> -	    tree v = TYPE_MAIN_VARIANT (type);
> -	    for (; v; v = TYPE_NEXT_VARIANT (v))
> -	      if (cp_check_qualified_type (v, variant, var_quals,
> -					   rqual, cr, false))
> -		{
> -		  /* The main variant will not match V, so PREV will never
> -		     be null.  */
> -		  TYPE_NEXT_VARIANT (prev) = TYPE_NEXT_VARIANT (variant);
> -		  break;
> -		}
> -	    TYPE_RAISES_EXCEPTIONS (variant) = raises;
> -
> -	    if (!v)
> -	      v = build_cp_fntype_variant (TYPE_CANONICAL (variant),
> -					   rqual, cr, false);
> -	    TYPE_CANONICAL (variant) = TYPE_CANONICAL (v);
> -	  }
> -	else
> -	  TYPE_RAISES_EXCEPTIONS (variant) = raises;
> +	SET_TYPE_STRUCTURAL_EQUALITY (variant);
> +	TYPE_RAISES_EXCEPTIONS (variant) = raises;
>   
>   	if (!TYPE_DEPENDENT_P (variant))
>   	  /* We no longer know that it's not type-dependent.  */
> diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
> index 75b696e32e0..d535746fd43 100644
> --- a/gcc/cp/typeck.cc
> +++ b/gcc/cp/typeck.cc
> @@ -1227,7 +1227,9 @@ comp_except_specs (const_tree t1, const_tree t2, int exact)
>     if ((t1 && TREE_PURPOSE (t1))
>         || (t2 && TREE_PURPOSE (t2)))
>       return (t1 && t2
> -	    && cp_tree_equal (TREE_PURPOSE (t1), TREE_PURPOSE (t2)));
> +	    && (exact == ce_exact
> +		? TREE_PURPOSE (t1) == TREE_PURPOSE (t2)
> +		: cp_tree_equal (TREE_PURPOSE (t1), TREE_PURPOSE (t2))));
>   
>     if (t1 == NULL_TREE)			   /* T1 is ...  */
>       return t2 == NULL_TREE || exact == ce_derived;
> diff --git a/gcc/testsuite/g++.dg/modules/noexcept-2_a.H b/gcc/testsuite/g++.dg/modules/noexcept-2_a.H
> new file mode 100644
> index 00000000000..b7144f42d7e
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/noexcept-2_a.H
> @@ -0,0 +1,24 @@
> +// PR c++/115159
> +// { dg-additional-options -fmodule-header }
> +// { dg-module-cmi {} }
> +
> +struct QDebug;
> +
> +template<class T> void f(T);
> +
> +template<class T> struct QList {
> +  QDebug g(QList &other) noexcept(noexcept(f(other)));
> +  QDebug h(QList &other) noexcept(noexcept(f(other)));
> +};
> +
> +struct QObjectData {
> +  QList<int> children;
> +};
> +
> +struct QIODevice {
> +  QObjectData d_ptr;
> +};
> +
> +struct QDebug {
> +  QDebug(QIODevice);
> +};
> diff --git a/gcc/testsuite/g++.dg/modules/noexcept-2_b.C b/gcc/testsuite/g++.dg/modules/noexcept-2_b.C
> new file mode 100644
> index 00000000000..d34c63add10
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/noexcept-2_b.C
> @@ -0,0 +1,4 @@
> +// PR c++/115159
> +// { dg-additional-options "-fmodules-ts -fno-module-lazy" }
> +
> +import "noexcept-2_a.H";

Re: [PATCH] c++: canonicity of fn types w/ complex eh specs [PR115159]

[Patrick Palka]

On Tue, 21 May 2024, Jason Merrill wrote:

> On 5/21/24 21:55, Patrick Palka wrote:
> > On Tue, 21 May 2024, Jason Merrill wrote:
> > 
> > > On 5/21/24 17:27, Patrick Palka wrote:
> > > > On Tue, 21 May 2024, Jason Merrill wrote:
> > > > 
> > > > > On 5/21/24 15:36, Patrick Palka wrote:
> > > > > > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look
> > > > > > OK for trunk?
> > > > > > 
> > > > > > Alternatively, I considered fixing this by incrementing
> > > > > > comparing_specializations around the call to comp_except_specs in
> > > > > > cp_check_qualified_type, but generally for types whose identity
> > > > > > depends on whether comparing_specializations is set we need to
> > > > > > use structural equality anyway IIUC.
> > > > > 
> > > > > Why not both?
> > > > 
> > > > I figured the latter change isn't necessary/observable since
> > > > comparing_specializations would only make a difference for complex
> > > > exception specifications, and with this patch we won't even call
> > > > cp_check_qualified_type on a complex eh spec.
> > > 
> > > My concern is that if we're building a function type multiple times with
> > > the
> > > same noexcept-spec, this patch would mean creating multiple equivalent
> > > function types instead of reusing one already created for the same
> > > function.
> > > 
> > > > > > +  bool complex_p = (cr && cr != noexcept_true_spec
> > > > > > +		    && !UNPARSED_NOEXCEPT_SPEC_P (cr));
> > > > > 
> > > > > Why treat unparsed specs differently from parsed ones?
> > > > 
> > > > Unparsed specs are unique according to cp_tree_equal, so in turn
> > > > function types with unparsed specs are unique, so it should be safe to
> > > > treat such types as canonical.  I'm not sure if this optimization
> > > > matters though; I'm happy to remove this case.
> > > 
> > > The idea that this optimization could make a difference raised the concern
> > > above.
> > 
> > Aha, makes sense.  To that end it seems we could strengthen the ce_exact
> > in comp_except_specs to require == instead of cp_tree_equal equality
> > when comparing two noexcept-specs; the only ce_exact callers are
> > cp_check_qualified_type and cxx_type_hash_eq, which should be fine with
> > that strengthening.  This way, we at least do try to reuse a variant if
> > the (complex or unparsed) noexcept-spec is exactly the same.
> 
> Sounds good.
> 
> Given that, we probably still want to move the canonical_eh_spec up in
> build_cp_fntype_variant, and pass that to cp_check_qualified_type?

And compare the canonical spec directly from cp_check_qualified_type
instead of using comp_except_specs?  Then IIUC for

  void f() throw(int);
  void g() throw(char);

we'd give g the same function type as f, which seems wrong?


> 
> > Like so?
> > 
> > -- >8 --
> > 
> > Subject: [PATCH] c++: canonicity of fn types w/ complex eh specs [PR115159]
> > 
> > Here the member functions QList::g and QList::h are given the same
> > function type since their exception specifications are equivalent
> > according to cp_tree_equal.  In doing so however this means that the
> > type of QList::h refers to a function parameter from QList::g, which
> > ends up confusing modules streaming.
> > 
> > I'm not sure if modules can be fixed to handle this situation, but
> > regardless it seems weird in principle that a function parameter can
> > escape in such a way.  The analogous situation with a trailing return
> > type and decltype
> > 
> >    auto g(QList &other) -> decltype(f(other));
> >    auto h(QList &other) -> decltype(f(other));
> > 
> > behaves better because we don't canonicalize decltype, and so the
> > function types of g and h are non-canonical and therefore not shared.
> > 
> > In light of this, it seems natural to treat function types with complex
> > eh specs as non-canonical as well so that each such function declaration
> > is given a unique function/method type node.  The main benefit of type
> > canonicalization is to speed up repeated type comparisons, but it should
> > rare for us to repeatedly compare two otherwise compatible function
> > types with complex exception specifications, so foregoing canonicalization
> > should not cause any problems.
> > 
> > To that end, this patch strengthens the ce_exact case of comp_except_specs
> > to require identity instead of equivalence of the exception specification
> > so that build_cp_fntype_variant doesn't reuse a variant when it shouldn't.
> > And in build_cp_fntype_variant we need to use structural equality for types
> > with a complex eh spec.  In turn we could simplify the code responsible
> > for adjusting unparsed eh spec variants.
> > 
> > 	PR c++/115159
> > 
> > gcc/cp/ChangeLog:
> > 
> > 	* tree.cc (build_cp_fntype_variant): Always use structural
> > 	equality for types with a complex exception specification.
> > 	(fixup_deferred_exception_variants): Always use structural
> > 	equality for adjusted variants.
> > 	* typeck.cc (comp_except_specs): Require == instead of
> > 	cp_tree_equal for noexcept-spec comparison in the ce_exact case.
> > 
> > gcc/testsuite/ChangeLog:
> > 
> > 	* g++.dg/modules/noexcept-2_a.H: New test.
> > 	* g++.dg/modules/noexcept-2_b.C: New test.
> > ---
> >   gcc/cp/tree.cc                              | 47 +++++----------------
> >   gcc/cp/typeck.cc                            |  4 +-
> >   gcc/testsuite/g++.dg/modules/noexcept-2_a.H | 24 +++++++++++
> >   gcc/testsuite/g++.dg/modules/noexcept-2_b.C |  4 ++
> >   4 files changed, 41 insertions(+), 38 deletions(-)
> >   create mode 100644 gcc/testsuite/g++.dg/modules/noexcept-2_a.H
> >   create mode 100644 gcc/testsuite/g++.dg/modules/noexcept-2_b.C
> > 
> > diff --git a/gcc/cp/tree.cc b/gcc/cp/tree.cc
> > index 9d37d255d8d..93a64322418 100644
> > --- a/gcc/cp/tree.cc
> > +++ b/gcc/cp/tree.cc
> > @@ -2794,7 +2794,12 @@ build_cp_fntype_variant (tree type, cp_ref_qualifier
> > rqual,
> >     /* Canonicalize the exception specification.  */
> >     tree cr = flag_noexcept_type ? canonical_eh_spec (raises) : NULL_TREE;
> >   -  if (TYPE_STRUCTURAL_EQUALITY_P (type))
> > +  /* Always use structural equality for function types with a complex
> > +     exception specification since their identity may depend on e.g.
> > +     whether comparing_specializations is set.  */
> > +  bool complex_eh_spec_p = (cr && cr != noexcept_true_spec
> > +			    && !UNPARSED_NOEXCEPT_SPEC_P (cr));
> > +  if (TYPE_STRUCTURAL_EQUALITY_P (type) || complex_eh_spec_p)
> >       /* Propagate structural equality. */
> >       SET_TYPE_STRUCTURAL_EQUALITY (v);
> >     else if (TYPE_CANONICAL (type) != type || cr != raises || late)
> > @@ -2812,55 +2817,23 @@ build_cp_fntype_variant (tree type, cp_ref_qualifier
> > rqual,
> >   /* TYPE is a function or method type with a deferred exception
> >      specification that has been parsed to RAISES.  Fixup all the type
> >      variants that are affected in place.  Via decltype &| noexcept
> > -   tricks, the unparsed spec could have escaped into the type system.
> > -   The general case is hard to fixup canonical types for.  */
> > +   tricks, the unparsed spec could have escaped into the type system.  */
> >     void
> >   fixup_deferred_exception_variants (tree type, tree raises)
> >   {
> >     tree original = TYPE_RAISES_EXCEPTIONS (type);
> > -  tree cr = flag_noexcept_type ? canonical_eh_spec (raises) : NULL_TREE;
> >       gcc_checking_assert (UNPARSED_NOEXCEPT_SPEC_P (original));
> >   -  /* Though sucky, this walk will process the canonical variants
> > -     first.  */
> > -  tree prev = NULL_TREE;
> >     for (tree variant = TYPE_MAIN_VARIANT (type);
> > -       variant; prev = variant, variant = TYPE_NEXT_VARIANT (variant))
> > +       variant; variant = TYPE_NEXT_VARIANT (variant))
> >       if (TYPE_RAISES_EXCEPTIONS (variant) == original)
> >         {
> >   	gcc_checking_assert (variant != TYPE_MAIN_VARIANT (type));
> >   -	if (!TYPE_STRUCTURAL_EQUALITY_P (variant))
> > -	  {
> > -	    cp_cv_quals var_quals = TYPE_QUALS (variant);
> > -	    cp_ref_qualifier rqual = type_memfn_rqual (variant);
> > -
> > -	    /* If VARIANT would become a dup (cp_check_qualified_type-wise)
> > -	       of an existing variant in the variant list of TYPE after its
> > -	       exception specification has been parsed, elide it.  Otherwise,
> > -	       build_cp_fntype_variant could use it, leading to "canonical
> > -	       types differ for identical types."  */
> > -	    tree v = TYPE_MAIN_VARIANT (type);
> > -	    for (; v; v = TYPE_NEXT_VARIANT (v))
> > -	      if (cp_check_qualified_type (v, variant, var_quals,
> > -					   rqual, cr, false))
> > -		{
> > -		  /* The main variant will not match V, so PREV will never
> > -		     be null.  */
> > -		  TYPE_NEXT_VARIANT (prev) = TYPE_NEXT_VARIANT (variant);
> > -		  break;
> > -		}
> > -	    TYPE_RAISES_EXCEPTIONS (variant) = raises;
> > -
> > -	    if (!v)
> > -	      v = build_cp_fntype_variant (TYPE_CANONICAL (variant),
> > -					   rqual, cr, false);
> > -	    TYPE_CANONICAL (variant) = TYPE_CANONICAL (v);
> > -	  }
> > -	else
> > -	  TYPE_RAISES_EXCEPTIONS (variant) = raises;
> > +	SET_TYPE_STRUCTURAL_EQUALITY (variant);
> > +	TYPE_RAISES_EXCEPTIONS (variant) = raises;
> >     	if (!TYPE_DEPENDENT_P (variant))
> >   	  /* We no longer know that it's not type-dependent.  */
> > diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
> > index 75b696e32e0..d535746fd43 100644
> > --- a/gcc/cp/typeck.cc
> > +++ b/gcc/cp/typeck.cc
> > @@ -1227,7 +1227,9 @@ comp_except_specs (const_tree t1, const_tree t2, int
> > exact)
> >     if ((t1 && TREE_PURPOSE (t1))
> >         || (t2 && TREE_PURPOSE (t2)))
> >       return (t1 && t2
> > -	    && cp_tree_equal (TREE_PURPOSE (t1), TREE_PURPOSE (t2)));
> > +	    && (exact == ce_exact
> > +		? TREE_PURPOSE (t1) == TREE_PURPOSE (t2)
> > +		: cp_tree_equal (TREE_PURPOSE (t1), TREE_PURPOSE (t2))));
> >       if (t1 == NULL_TREE)			   /* T1 is ...  */
> >       return t2 == NULL_TREE || exact == ce_derived;
> > diff --git a/gcc/testsuite/g++.dg/modules/noexcept-2_a.H
> > b/gcc/testsuite/g++.dg/modules/noexcept-2_a.H
> > new file mode 100644
> > index 00000000000..b7144f42d7e
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/modules/noexcept-2_a.H
> > @@ -0,0 +1,24 @@
> > +// PR c++/115159
> > +// { dg-additional-options -fmodule-header }
> > +// { dg-module-cmi {} }
> > +
> > +struct QDebug;
> > +
> > +template<class T> void f(T);
> > +
> > +template<class T> struct QList {
> > +  QDebug g(QList &other) noexcept(noexcept(f(other)));
> > +  QDebug h(QList &other) noexcept(noexcept(f(other)));
> > +};
> > +
> > +struct QObjectData {
> > +  QList<int> children;
> > +};
> > +
> > +struct QIODevice {
> > +  QObjectData d_ptr;
> > +};
> > +
> > +struct QDebug {
> > +  QDebug(QIODevice);
> > +};
> > diff --git a/gcc/testsuite/g++.dg/modules/noexcept-2_b.C
> > b/gcc/testsuite/g++.dg/modules/noexcept-2_b.C
> > new file mode 100644
> > index 00000000000..d34c63add10
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/modules/noexcept-2_b.C
> > @@ -0,0 +1,4 @@
> > +// PR c++/115159
> > +// { dg-additional-options "-fmodules-ts -fno-module-lazy" }
> > +
> > +import "noexcept-2_a.H";
> 
> 

Re: [PATCH] c++: canonicity of fn types w/ complex eh specs [PR115159]

[Jason Merrill]

On 5/22/24 09:01, Patrick Palka wrote:
> On Tue, 21 May 2024, Jason Merrill wrote:
> 
>> On 5/21/24 21:55, Patrick Palka wrote:
>>> On Tue, 21 May 2024, Jason Merrill wrote:
>>>
>>>> On 5/21/24 17:27, Patrick Palka wrote:
>>>>> On Tue, 21 May 2024, Jason Merrill wrote:
>>>>>
>>>>>> On 5/21/24 15:36, Patrick Palka wrote:
>>>>>>> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look
>>>>>>> OK for trunk?
>>>>>>>
>>>>>>> Alternatively, I considered fixing this by incrementing
>>>>>>> comparing_specializations around the call to comp_except_specs in
>>>>>>> cp_check_qualified_type, but generally for types whose identity
>>>>>>> depends on whether comparing_specializations is set we need to
>>>>>>> use structural equality anyway IIUC.
>>>>>>
>>>>>> Why not both?
>>>>>
>>>>> I figured the latter change isn't necessary/observable since
>>>>> comparing_specializations would only make a difference for complex
>>>>> exception specifications, and with this patch we won't even call
>>>>> cp_check_qualified_type on a complex eh spec.
>>>>
>>>> My concern is that if we're building a function type multiple times with
>>>> the
>>>> same noexcept-spec, this patch would mean creating multiple equivalent
>>>> function types instead of reusing one already created for the same
>>>> function.
>>>>
>>>>>>> +  bool complex_p = (cr && cr != noexcept_true_spec
>>>>>>> +		    && !UNPARSED_NOEXCEPT_SPEC_P (cr));
>>>>>>
>>>>>> Why treat unparsed specs differently from parsed ones?
>>>>>
>>>>> Unparsed specs are unique according to cp_tree_equal, so in turn
>>>>> function types with unparsed specs are unique, so it should be safe to
>>>>> treat such types as canonical.  I'm not sure if this optimization
>>>>> matters though; I'm happy to remove this case.
>>>>
>>>> The idea that this optimization could make a difference raised the concern
>>>> above.
>>>
>>> Aha, makes sense.  To that end it seems we could strengthen the ce_exact
>>> in comp_except_specs to require == instead of cp_tree_equal equality
>>> when comparing two noexcept-specs; the only ce_exact callers are
>>> cp_check_qualified_type and cxx_type_hash_eq, which should be fine with
>>> that strengthening.  This way, we at least do try to reuse a variant if
>>> the (complex or unparsed) noexcept-spec is exactly the same.
>>
>> Sounds good.
>>
>> Given that, we probably still want to move the canonical_eh_spec up in
>> build_cp_fntype_variant, and pass that to cp_check_qualified_type?
> 
> And compare the canonical spec directly from cp_check_qualified_type
> instead of using comp_except_specs?  Then IIUC for
> 
>    void f() throw(int);
>    void g() throw(char);
> 
> we'd give g the same function type as f, which seems wrong?

Good point, I was confused about what canonical_eh_spec was doing.  Your 
last patch is OK.

Jason