[C++ PATCH] Improve C++ error recovery (PR c++/59655)

[C++ PATCH] Improve C++ error recovery (PR c++/59655)

[Jakub Jelinek]

Hi!

On the following testcase, we emit 2 errors and 1 warning, when the user
really should see one error.  The desirable error is static_assert failure,
the bogus error is during error recovery, complaining that a no_linkage
template isn't defined when it really is defined, but we haven't bothered
instantiating it, because limit_bad_template_recursion sad so.
And finally a warning from the middle-end about function being used even
when it is not defined.

The last one already checks TREE_NO_WARNING on the function decl to avoid
the warning, so this patch just uses TREE_NO_WARNING to signal this case,
both to that warning and to no_linkage_error.

Now, I admit I'm not 100% sure if using TREE_NO_WARNING is the best thing
for no_linkage_error, another possibility might be adding some bit in
lang_decl_base or so and setting that bit in addition to TREE_NO_WARNING,
where that new bit would mean this decl might be defined if we didn't decide
not to instantiate it.  With the patch as is, there is a risk if
TREE_NO_WARNING is set for some other reason on a fndecl (or variable
template instantiation?) and some errors have been reported already that we
won't report another error for it when we should.  But perhaps that is
acceptable, once users fix the original errors even if TREE_NO_WARNING will
be set, errorcount + sorrycount will be zero and thus no_linkage_error will
still report it.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
Or do you want to use an additional bit for that?

2019-12-10  Jakub Jelinek  <jakub@redhat.com>

	PR c++/59655
	* pt.c (push_tinst_level_loc): If limit_bad_template_recursion,
	set TREE_NO_WARNING on tldcl.
	* decl2.c (no_linkage_error): Treat templates with TREE_NO_WARNING
	as defined during error recovery.

	* g++.dg/cpp0x/diag3.C: New test.

--- gcc/cp/pt.c.jj	2019-12-10 00:52:39.017449262 +0100
+++ gcc/cp/pt.c	2019-12-10 19:20:11.046062705 +0100
@@ -10640,7 +10640,12 @@ push_tinst_level_loc (tree tldcl, tree t
      anything else.  Do allow deduction substitution and decls usable in
      constant expressions.  */
   if (!targs && limit_bad_template_recursion (tldcl))
-    return false;
+    {
+      /* Avoid no_linkage_errors and unused function warnings for this
+	 decl.  */
+      TREE_NO_WARNING (tldcl) = 1;
+      return false;
+    }
 
   /* When not -quiet, dump template instantiations other than functions, since
      announce_function will take care of those.  */
--- gcc/cp/decl2.c.jj	2019-11-28 09:05:13.376262983 +0100
+++ gcc/cp/decl2.c	2019-12-10 19:33:05.555237052 +0100
@@ -4414,7 +4414,14 @@ decl_maybe_constant_var_p (tree decl)
 void
 no_linkage_error (tree decl)
 {
-  if (cxx_dialect >= cxx11 && decl_defined_p (decl))
+  if (cxx_dialect >= cxx11
+      && (decl_defined_p (decl)
+	  /* Treat templates which limit_bad_template_recursion decided
+	     not to instantiate as if they were defined.  */
+	  || (errorcount + sorrycount > 0
+	      && DECL_LANG_SPECIFIC (decl)
+	      && DECL_TEMPLATE_INFO (decl)
+	      && TREE_NO_WARNING (decl))))
     /* In C++11 it's ok if the decl is defined.  */
     return;
   tree t = no_linkage_check (TREE_TYPE (decl), /*relaxed_p=*/false);
--- gcc/testsuite/g++.dg/cpp0x/diag3.C.jj	2019-12-10 19:39:06.659722663 +0100
+++ gcc/testsuite/g++.dg/cpp0x/diag3.C	2019-12-10 19:37:30.617189316 +0100
@@ -0,0 +1,20 @@
+// PR c++/59655
+// { dg-do compile { target c++11 } }
+
+template<typename T> struct A { static constexpr bool value = false; };
+
+struct B {
+  template<typename T>
+  B (T t)
+  {
+    static_assert (A<T>::value, "baz");		// { dg-error "static assertion failed" }
+    foo (t);
+  }
+  template<typename T> void foo (T) {}		// { dg-bogus "used but never defined" }
+};
+
+int
+main ()
+{
+  B t([](int) { });
+}

	Jakub

C++ Patch Ping (was Re: [C++ PATCH] Improve C++ error recovery (PR c++/59655))

[Jakub Jelinek]

Hi!

On Tue, Dec 10, 2019 at 10:02:47PM +0100, Jakub Jelinek wrote:
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> Or do you want to use an additional bit for that?
> 
> 2019-12-10  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR c++/59655
> 	* pt.c (push_tinst_level_loc): If limit_bad_template_recursion,
> 	set TREE_NO_WARNING on tldcl.
> 	* decl2.c (no_linkage_error): Treat templates with TREE_NO_WARNING
> 	as defined during error recovery.
> 
> 	* g++.dg/cpp0x/diag3.C: New test.

I'd like to ping this patch (or whether you want a special bit for it
in addition to TREE_NO_WARNING).

Thanks.

	Jakub

Re: [C++ PATCH] Improve C++ error recovery (PR c++/59655)

[Jason Merrill]

On 12/10/19 4:02 PM, Jakub Jelinek wrote:
> Hi!
> 
> On the following testcase, we emit 2 errors and 1 warning, when the user
> really should see one error.  The desirable error is static_assert failure,
> the bogus error is during error recovery, complaining that a no_linkage
> template isn't defined when it really is defined, but we haven't bothered
> instantiating it, because limit_bad_template_recursion sad so.
> And finally a warning from the middle-end about function being used even
> when it is not defined.
> 
> The last one already checks TREE_NO_WARNING on the function decl to avoid
> the warning, so this patch just uses TREE_NO_WARNING to signal this case,
> both to that warning and to no_linkage_error.
> 
> Now, I admit I'm not 100% sure if using TREE_NO_WARNING is the best thing
> for no_linkage_error, another possibility might be adding some bit in
> lang_decl_base or so and setting that bit in addition to TREE_NO_WARNING,
> where that new bit would mean this decl might be defined if we didn't decide
> not to instantiate it.  With the patch as is, there is a risk if
> TREE_NO_WARNING is set for some other reason on a fndecl (or variable
> template instantiation?) and some errors have been reported already that we
> won't report another error for it when we should.  But perhaps that is
> acceptable, once users fix the original errors even if TREE_NO_WARNING will
> be set, errorcount + sorrycount will be zero and thus no_linkage_error will
> still report it.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> Or do you want to use an additional bit for that?
> 
> 2019-12-10  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR c++/59655
> 	* pt.c (push_tinst_level_loc): If limit_bad_template_recursion,
> 	set TREE_NO_WARNING on tldcl.
> 	* decl2.c (no_linkage_error): Treat templates with TREE_NO_WARNING
> 	as defined during error recovery.
> 
> 	* g++.dg/cpp0x/diag3.C: New test.
> 
> --- gcc/cp/pt.c.jj	2019-12-10 00:52:39.017449262 +0100
> +++ gcc/cp/pt.c	2019-12-10 19:20:11.046062705 +0100
> @@ -10640,7 +10640,12 @@ push_tinst_level_loc (tree tldcl, tree t
>        anything else.  Do allow deduction substitution and decls usable in
>        constant expressions.  */
>     if (!targs && limit_bad_template_recursion (tldcl))
> -    return false;
> +    {
> +      /* Avoid no_linkage_errors and unused function warnings for this
> +	 decl.  */
> +      TREE_NO_WARNING (tldcl) = 1;
> +      return false;
> +    }
>   
>     /* When not -quiet, dump template instantiations other than functions, since
>        announce_function will take care of those.  */
> --- gcc/cp/decl2.c.jj	2019-11-28 09:05:13.376262983 +0100
> +++ gcc/cp/decl2.c	2019-12-10 19:33:05.555237052 +0100
> @@ -4414,7 +4414,14 @@ decl_maybe_constant_var_p (tree decl)
>   void
>   no_linkage_error (tree decl)
>   {
> -  if (cxx_dialect >= cxx11 && decl_defined_p (decl))
> +  if (cxx_dialect >= cxx11
> +      && (decl_defined_p (decl)
> +	  /* Treat templates which limit_bad_template_recursion decided
> +	     not to instantiate as if they were defined.  */
> +	  || (errorcount + sorrycount > 0
> +	      && DECL_LANG_SPECIFIC (decl)
> +	      && DECL_TEMPLATE_INFO (decl)
> +	      && TREE_NO_WARNING (decl))))

I'm not sure we need to check DECL_TEMPLATE_INFO; if we've seen errors 
they could have interfered with non-template definitions, too.  OK 
either way.

Jason