[PR86397] set p_t_decl while canonicalizing eh specs for mangling

Message ID orzhu0r8a3.fsf@lxoliva.fsfla.org
State New
Headers show
Series
  • [PR86397] set p_t_decl while canonicalizing eh specs for mangling
Related show

Commit Message

Alexandre Oliva Nov. 22, 2018, 11:40 p.m.
Mangling visits the base template function type, prior to template
resolution, and on such types, exception specifications may contain
unresolved noexcept expressions.  nothrow_spec_p is called on them
even whe exception specifications are not part of function types, and
it rejects unresolved noexcept expressions if processing_template_decl
is not set.

Setting processing_template_decl while mangling what is indeed the
(generic) type of a function template would solve this problem, but it
would cause others: no_linkage_check, for example, returns early if
processing_template_decl, and this regresses g++.dg/pr79091.C and
g++.dg/abi/no-linkage-expr1.C.

So, instead of setting processing_template_decl throughout, I'm
introducing another flag, processing_template_function_signature, and
using it to set processing_template_decl only while canonicalizing the
exception spec, which is where nothrow_spec_p gets called.

Regstrapped on i686- and x86_64-linux-gnu.  Ok to install?


for  gcc/cp/ChangeLog

	PR c++/86397
	* mangle.c (processing_template_function_signature): New var.
	(write_encoding): Set it while mangling generic template type.
	(canonicalize_for_substitution): Set processing_template_decl
	while handling exception specs if
	processing_template_function_signature.

for gcc/testsuite/ChangeLog

	PR c++/86397
	* g++.dg/cpp0x/pr86397-1.C: New.
	* g++.dg/cpp0x/pr86397-2.C: New.
---
 gcc/cp/mangle.c                        |   19 +++++++++++++++++++
 gcc/testsuite/g++.dg/cpp0x/pr86397-1.C |    4 ++++
 gcc/testsuite/g++.dg/cpp0x/pr86397-2.C |    4 ++++
 3 files changed, 27 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/pr86397-1.C
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/pr86397-2.C



-- 
Alexandre Oliva, freedom fighter   https://FSFLA.org/blogs/lxo
Be the change, be Free!         FSF Latin America board member
GNU Toolchain Engineer                Free Software Evangelist
Hay que enGNUrecerse, pero sin perder la terGNUra jamás-GNUChe

Comments

Jason Merrill Nov. 27, 2018, 10:40 p.m. | #1
On 11/22/18 6:40 PM, Alexandre Oliva wrote:
> Mangling visits the base template function type, prior to template

> resolution, and on such types, exception specifications may contain

> unresolved noexcept expressions.  nothrow_spec_p is called on them

> even when exception specifications are not part of function types, and

> it rejects unresolved noexcept expressions if processing_template_decl

> is not set.


The problem here is that the noexcept expression is unresolved even 
though it isn't dependent; it should have been resolved to 'true'. 
Ideally with a warning, since using the name of a function as a boolean 
value is pretty strange.

nothrow_spec_p is right to reject this.

Jason
Alexandre Oliva Nov. 28, 2018, 4:54 a.m. | #2
On Nov 27, 2018, Jason Merrill <jason@redhat.com> wrote:

> On 11/22/18 6:40 PM, Alexandre Oliva wrote:

>> Mangling visits the base template function type, prior to template

>> resolution, and on such types, exception specifications may contain

>> unresolved noexcept expressions.  nothrow_spec_p is called on them

>> even when exception specifications are not part of function types, and

>> it rejects unresolved noexcept expressions if processing_template_decl

>> is not set.


> The problem here is that the noexcept expression is unresolved even

> though it isn't dependent


Yeah, but that seems to be on purpose, according to these comments, that
precede the hunk below.

  /* This isn't part of the signature, so don't bother trying to evaluate
     it until instantiation.  */

Taking out the 'flag_noexcept_type && ' subexpr fixes the problem, but
defeats the intended deferral of unnecessary computation:

diff --git a/gcc/cp/except.c b/gcc/cp/except.c
index 3449b59b3cc0..dbd233c94c3a 100644
--- a/gcc/cp/except.c
+++ b/gcc/cp/except.c
@@ -1193,7 +1193,7 @@ build_noexcept_spec (tree expr, tsubst_flags_t complain)
      it until instantiation.  */
   if (TREE_CODE (expr) != DEFERRED_NOEXCEPT
       && (!processing_template_decl
-	  || (flag_noexcept_type && !value_dependent_expression_p (expr))))
+	  || !value_dependent_expression_p (expr)))
     {
       expr = perform_implicit_conversion_flags (boolean_type_node, expr,
 						complain,


In order to retain that deferral, we could change the mangling logic to
also refrain from canonicalizing the EH spec when it's not part of the
type:

diff --git a/gcc/cp/mangle.c b/gcc/cp/mangle.c
index 64415894bc57..4c8086c9f9bd 100644
--- a/gcc/cp/mangle.c
+++ b/gcc/cp/mangle.c
@@ -418,9 +418,12 @@ canonicalize_for_substitution (tree node)
 	  || TREE_CODE (node) == METHOD_TYPE)
 	{
 	  node = build_ref_qualified_type (node, type_memfn_rqual (orig));
-	  tree r = canonical_eh_spec (TYPE_RAISES_EXCEPTIONS (orig));
+	  tree r = TYPE_RAISES_EXCEPTIONS (orig);
 	  if (flag_noexcept_type)
-	    node = build_exception_variant (node, r);
+	    {
+	      r = canonical_eh_spec (r);
+	      node = build_exception_variant (node, r);
+	    }
 	  else
 	    /* Set the warning flag if appropriate.  */
 	    write_exception_spec (r);

This would bypass the nothrow_spec_p call in canonical_eh_spec at
C++1[14], but it might produce unintended -Wnoexcept-type warnings when
the noexcept expression would resolve to false.  The canonical_eh_spec
call wouldn't have avoided it anyway.


Which one?

-- 
Alexandre Oliva, freedom fighter   https://FSFLA.org/blogs/lxo
Be the change, be Free!         FSF Latin America board member
GNU Toolchain Engineer                Free Software Evangelist
Hay que enGNUrecerse, pero sin perder la terGNUra jamás-GNUChe
Jason Merrill Nov. 29, 2018, 3:27 a.m. | #3
On 11/27/18 11:54 PM, Alexandre Oliva wrote:
> On Nov 27, 2018, Jason Merrill <jason@redhat.com> wrote:

> 

>> On 11/22/18 6:40 PM, Alexandre Oliva wrote:

>>> Mangling visits the base template function type, prior to template

>>> resolution, and on such types, exception specifications may contain

>>> unresolved noexcept expressions.  nothrow_spec_p is called on them

>>> even when exception specifications are not part of function types, and

>>> it rejects unresolved noexcept expressions if processing_template_decl

>>> is not set.

> 

>> The problem here is that the noexcept expression is unresolved even

>> though it isn't dependent

> 

> Yeah, but that seems to be on purpose, according to these comments, that

> precede the hunk below.

> 

>    /* This isn't part of the signature, so don't bother trying to evaluate

>       it until instantiation.  */


> Taking out the 'flag_noexcept_type && ' subexpr fixes the problem, but

> defeats the intended deferral of unnecessary computation:

> 

> diff --git a/gcc/cp/except.c b/gcc/cp/except.c

> index 3449b59b3cc0..dbd233c94c3a 100644

> --- a/gcc/cp/except.c

> +++ b/gcc/cp/except.c

> @@ -1193,7 +1193,7 @@ build_noexcept_spec (tree expr, tsubst_flags_t complain)

>        it until instantiation.  */

>     if (TREE_CODE (expr) != DEFERRED_NOEXCEPT

>         && (!processing_template_decl

> -	  || (flag_noexcept_type && !value_dependent_expression_p (expr))))

> +	  || !value_dependent_expression_p (expr)))

>       {

>         expr = perform_implicit_conversion_flags (boolean_type_node, expr,

>   						complain,


Let's go with this.  And remove the comment.

And the !processing_template_decl is also redundant, since that's 
checked at the top of value_dependent_expression_p.

Jason
Alexandre Oliva Dec. 5, 2018, 6:32 a.m. | #4
On Nov 29, 2018, Jason Merrill <jason@redhat.com> wrote:

> Let's go with this.  And remove the comment.


> And the !processing_template_decl is also redundant, since that's

> checked at the top of value_dependent_expression_p.


I've tested this on i686- and x86_64-linux-gnu.  Ok to install?


[PR86397] resolve nondependent noexcept specs early in C++1[14]

From: Alexandre Oliva <aoliva@redhat.com>


build_noexcept_spec refrained from resolving nondependent noexcept
expressions when they were not part of the function types (C++ 11 and
14).  This caused problems during mangling: canonical_eh_spec, when
called on the template function type, would find an unresolved but not
explicitly deferred expression, and nothrow_spec_p would reject it.

We could relax the mangling logic to skip canonical_eh_spec, but since
-Wnoexcept-type warns when mangling function names that change as
noexcept specs become part of types and of mangling in C++17, and the
test at mangling time may give incorrect results if the spec is not
resolved, we might as well keep things simple and resolve nondependent
noexcept specs sooner rather than later.  This is what this patch does.


for  gcc/cp/ChangeLog

	PR c++/86397
	* except.c (build_noexcept_spec): Resolve nondependent
	expressions.

for gcc/testsuite/ChangeLog

	PR c++/86397
	* g++.dg/cpp0x/pr86397-1.C: New.
	* g++.dg/cpp0x/pr86397-2.C: New.
---
 gcc/cp/except.c                        |    5 +----
 gcc/testsuite/g++.dg/cpp0x/pr86397-1.C |    4 ++++
 gcc/testsuite/g++.dg/cpp0x/pr86397-2.C |    4 ++++
 3 files changed, 9 insertions(+), 4 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/pr86397-1.C
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/pr86397-2.C

diff --git a/gcc/cp/except.c b/gcc/cp/except.c
index 3449b59b3cc0..a6951baa35c6 100644
--- a/gcc/cp/except.c
+++ b/gcc/cp/except.c
@@ -1189,11 +1189,8 @@ type_throw_all_p (const_tree type)
 tree
 build_noexcept_spec (tree expr, tsubst_flags_t complain)
 {
-  /* This isn't part of the signature, so don't bother trying to evaluate
-     it until instantiation.  */
   if (TREE_CODE (expr) != DEFERRED_NOEXCEPT
-      && (!processing_template_decl
-	  || (flag_noexcept_type && !value_dependent_expression_p (expr))))
+      && !value_dependent_expression_p (expr))
     {
       expr = perform_implicit_conversion_flags (boolean_type_node, expr,
 						complain,
diff --git a/gcc/testsuite/g++.dg/cpp0x/pr86397-1.C b/gcc/testsuite/g++.dg/cpp0x/pr86397-1.C
new file mode 100644
index 000000000000..4f9f5fa7e4c8
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/pr86397-1.C
@@ -0,0 +1,4 @@
+// { dg-do compile { target c++11 } }
+void e();
+template <bool> void f(int() noexcept(e)) {}
+template void f<false>(int()); // { dg-error "does not match" "" { target c++17 } }
diff --git a/gcc/testsuite/g++.dg/cpp0x/pr86397-2.C b/gcc/testsuite/g++.dg/cpp0x/pr86397-2.C
new file mode 100644
index 000000000000..fb43499526e8
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/pr86397-2.C
@@ -0,0 +1,4 @@
+// { dg-do compile { target c++11 } }
+void e();
+template <bool> void f(int() noexcept(e)) {}
+template void f<false>(int() noexcept);


-- 
Alexandre Oliva, freedom fighter   https://FSFLA.org/blogs/lxo
Be the change, be Free!         FSF Latin America board member
GNU Toolchain Engineer                Free Software Evangelist
Hay que enGNUrecerse, pero sin perder la terGNUra jamás-GNUChe
Jason Merrill Dec. 5, 2018, 3:53 p.m. | #5
OK, thanks.
On Wed, Dec 5, 2018 at 1:32 AM Alexandre Oliva <aoliva@redhat.com> wrote:
>

> On Nov 29, 2018, Jason Merrill <jason@redhat.com> wrote:

>

> > Let's go with this.  And remove the comment.

>

> > And the !processing_template_decl is also redundant, since that's

> > checked at the top of value_dependent_expression_p.

>

> I've tested this on i686- and x86_64-linux-gnu.  Ok to install?

>

>

> [PR86397] resolve nondependent noexcept specs early in C++1[14]

>

> From: Alexandre Oliva <aoliva@redhat.com>

>

> build_noexcept_spec refrained from resolving nondependent noexcept

> expressions when they were not part of the function types (C++ 11 and

> 14).  This caused problems during mangling: canonical_eh_spec, when

> called on the template function type, would find an unresolved but not

> explicitly deferred expression, and nothrow_spec_p would reject it.

>

> We could relax the mangling logic to skip canonical_eh_spec, but since

> -Wnoexcept-type warns when mangling function names that change as

> noexcept specs become part of types and of mangling in C++17, and the

> test at mangling time may give incorrect results if the spec is not

> resolved, we might as well keep things simple and resolve nondependent

> noexcept specs sooner rather than later.  This is what this patch does.

>

>

> for  gcc/cp/ChangeLog

>

>         PR c++/86397

>         * except.c (build_noexcept_spec): Resolve nondependent

>         expressions.

>

> for gcc/testsuite/ChangeLog

>

>         PR c++/86397

>         * g++.dg/cpp0x/pr86397-1.C: New.

>         * g++.dg/cpp0x/pr86397-2.C: New.

> ---

>  gcc/cp/except.c                        |    5 +----

>  gcc/testsuite/g++.dg/cpp0x/pr86397-1.C |    4 ++++

>  gcc/testsuite/g++.dg/cpp0x/pr86397-2.C |    4 ++++

>  3 files changed, 9 insertions(+), 4 deletions(-)

>  create mode 100644 gcc/testsuite/g++.dg/cpp0x/pr86397-1.C

>  create mode 100644 gcc/testsuite/g++.dg/cpp0x/pr86397-2.C

>

> diff --git a/gcc/cp/except.c b/gcc/cp/except.c

> index 3449b59b3cc0..a6951baa35c6 100644

> --- a/gcc/cp/except.c

> +++ b/gcc/cp/except.c

> @@ -1189,11 +1189,8 @@ type_throw_all_p (const_tree type)

>  tree

>  build_noexcept_spec (tree expr, tsubst_flags_t complain)

>  {

> -  /* This isn't part of the signature, so don't bother trying to evaluate

> -     it until instantiation.  */

>    if (TREE_CODE (expr) != DEFERRED_NOEXCEPT

> -      && (!processing_template_decl

> -         || (flag_noexcept_type && !value_dependent_expression_p (expr))))

> +      && !value_dependent_expression_p (expr))

>      {

>        expr = perform_implicit_conversion_flags (boolean_type_node, expr,

>                                                 complain,

> diff --git a/gcc/testsuite/g++.dg/cpp0x/pr86397-1.C b/gcc/testsuite/g++.dg/cpp0x/pr86397-1.C

> new file mode 100644

> index 000000000000..4f9f5fa7e4c8

> --- /dev/null

> +++ b/gcc/testsuite/g++.dg/cpp0x/pr86397-1.C

> @@ -0,0 +1,4 @@

> +// { dg-do compile { target c++11 } }

> +void e();

> +template <bool> void f(int() noexcept(e)) {}

> +template void f<false>(int()); // { dg-error "does not match" "" { target c++17 } }

> diff --git a/gcc/testsuite/g++.dg/cpp0x/pr86397-2.C b/gcc/testsuite/g++.dg/cpp0x/pr86397-2.C

> new file mode 100644

> index 000000000000..fb43499526e8

> --- /dev/null

> +++ b/gcc/testsuite/g++.dg/cpp0x/pr86397-2.C

> @@ -0,0 +1,4 @@

> +// { dg-do compile { target c++11 } }

> +void e();

> +template <bool> void f(int() noexcept(e)) {}

> +template void f<false>(int() noexcept);

>

>

> --

> Alexandre Oliva, freedom fighter   https://FSFLA.org/blogs/lxo

> Be the change, be Free!         FSF Latin America board member

> GNU Toolchain Engineer                Free Software Evangelist

> Hay que enGNUrecerse, pero sin perder la terGNUra jamás-GNUChe

Patch

diff --git a/gcc/cp/mangle.c b/gcc/cp/mangle.c
index 64415894bc57..a499ef97b3c6 100644
--- a/gcc/cp/mangle.c
+++ b/gcc/cp/mangle.c
@@ -170,6 +170,17 @@  integer_type_codes[itk_none] =
   '\0', '\0', '\0', '\0', '\0', '\0', '\0', '\0'
 };
 
+/* Exception specifications may carry unresolved noexcept expressions
+   before template substitution, but nothrow_spec_p will fail an
+   assert check if it comes across such a spec while
+   !processing_template_decl.  So, when we start processing the
+   signature of a template function, that may contain a such
+   unresolved expressions, we set this variable.  Then, as we process
+   the exception spec, we set processing_template_decl.  We don't want
+   to set processing_template_decl throughout, becuase this affects
+   other relevant tests, such as no_linkage_check.  */
+static int processing_template_function_signature;
+
 static int decl_is_template_id (const tree, tree* const);
 
 /* Functions for handling substitutions.  */
@@ -418,7 +429,11 @@  canonicalize_for_substitution (tree node)
 	  || TREE_CODE (node) == METHOD_TYPE)
 	{
 	  node = build_ref_qualified_type (node, type_memfn_rqual (orig));
+	  if (processing_template_function_signature)
+	    processing_template_decl++;
 	  tree r = canonical_eh_spec (TYPE_RAISES_EXCEPTIONS (orig));
+	  if (processing_template_function_signature)
+	    processing_template_decl--;
 	  if (flag_noexcept_type)
 	    node = build_exception_variant (node, r);
 	  else
@@ -836,6 +851,7 @@  write_encoding (const tree decl)
 	     write_bare_function_type -- otherwise, it will get
 	     confused about which artificial parameters to skip.  */
 	  d = NULL_TREE;
+	  ++processing_template_function_signature;
 	}
       else
 	{
@@ -846,6 +862,9 @@  write_encoding (const tree decl)
       write_bare_function_type (fn_type,
 				mangle_return_type_p (decl),
 				d);
+
+      if (tmpl)
+	--processing_template_function_signature;
     }
 }
 
diff --git a/gcc/testsuite/g++.dg/cpp0x/pr86397-1.C b/gcc/testsuite/g++.dg/cpp0x/pr86397-1.C
new file mode 100644
index 000000000000..4f9f5fa7e4c8
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/pr86397-1.C
@@ -0,0 +1,4 @@ 
+// { dg-do compile { target c++11 } }
+void e();
+template <bool> void f(int() noexcept(e)) {}
+template void f<false>(int()); // { dg-error "does not match" "" { target c++17 } }
diff --git a/gcc/testsuite/g++.dg/cpp0x/pr86397-2.C b/gcc/testsuite/g++.dg/cpp0x/pr86397-2.C
new file mode 100644
index 000000000000..fb43499526e8
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/pr86397-2.C
@@ -0,0 +1,4 @@ 
+// { dg-do compile { target c++11 } }
+void e();
+template <bool> void f(int() noexcept(e)) {}
+template void f<false>(int() noexcept);