From 596d1ed9d40b1081fcc6c6161a0135952829b88f Mon Sep 17 00:00:00 2001
From: Patrick Palka <ppalka@redhat.com>
Date: Tue, 6 Aug 2024 20:54:03 -0400
Subject: [PATCH] c++: permit errors inside uninstantiated templates [PR116064]

In recent versions of GCC we've been diagnosing more and more kinds of
errors inside a template ahead of time.  This is a largely good thing
because it catches bugs, typos, dead code etc sooner.

But if the template never gets instantiated then such errors are harmless
and can be inconvenient to work around if say the code in question is
third party and in maintenance mode.  So it'd be handy to be able to
prevent these template errors from rendering the entire TU uncompilable.
(Note that such code is "ill-formed no diagnostic required" according
the standard.)

To that end this patch turns any errors issued within a template into
permerrors associated with a new -Wtemplate-body flag so that they can
be downgraded via e.g. -fpermissive or -Wno-error=template-body.  If
the template containing a downgraded error later needs to be instantiated,
we'll issue an error then.  But if the template never gets instantiated
then the downgraded error won't affect validity of the rest of the TU.

This is implemented via a diagnostic hook that gets called for each
diagnostic, and which adjusts an error diagnostic appropriately if we
detect it's occurring from a template context, and additionally flags
the template as erroneous.

For example the new testcase permissive-error1a.C gives:

gcc/testsuite/g++.dg/template/permissive-error1a.C: In function 'void f()':
gcc/testsuite/g++.dg/template/permissive-error1a.C:7:5: warning: increment of read-only variable 'n' [-Wtemplate-body]
    7 |   ++n;
      |     ^
...
gcc/testsuite/g++.dg/template/permissive-error1a.C: In instantiation of 'void f() [with T = int]':
gcc/testsuite/g++.dg/template/permissive-error1a.C:26:9:   required from here
   26 |   f<int>();
      |   ~~~~~~^~
gcc/testsuite/g++.dg/template/permissive-error1a.C:5:6: error: instantiating erroneous template
    5 | void f() {
      |      ^
gcc/testsuite/g++.dg/template/permissive-error1a.C:7:5: note: first error appeared here
    7 |   ++n; // {
      |     ^
...

	PR c++/116064

gcc/c-family/ChangeLog:

	* c.opt (Wtemplate-body): New warning.

gcc/cp/ChangeLog:

	* cp-tree.h (erroneous_templates_t): Declare.
	(erroneous_templates): Declare.
	(cp_seen_error): Declare.
	(seen_error): #define to cp_seen_error.
	* error.cc (get_current_template): Define.
	(relaxed_template_errors): Define.
	(cp_adjust_diagnostic_info): Define.
	(cp_seen_error): Define.
	(cxx_initialize_diagnostics): Set
	diagnostic_context::m_adjust_diagnostic_info.
	* module.cc (finish_module_processing): Don't write the
	module if it contains an erroneous template.
	* pt.cc (maybe_diagnose_erroneous_template): Define.
	(instantiate_class_template): Call it.
	(instantiate_decl): Likewise.

gcc/ChangeLog:

	* diagnostic.cc (diagnostic_context::initialize): Set
	m_adjust_diagnostic_info.
	(diagnostic_context::report_diagnostic): Call
	m_adjust_diagnostic_info.
	* diagnostic.h (diagnostic_context::m_adjust_diagnostic_info):
	New data member.
	* doc/invoke.texi (-Wno-template-body): Document.
	(-fpermissive): Mention -Wtemplate-body.

gcc/testsuite/ChangeLog:

	* g++.dg/ext/typedef-init.C: Downgrade error inside template
	to warning due to -fpermissive.
	* g++.dg/pr84492.C: Likewise.
	* g++.old-deja/g++.pt/crash51.C: Remove unneeded dg-options.
	* g++.dg/template/permissive-error1.C: New test.
	* g++.dg/template/permissive-error1a.C: New test.
	* g++.dg/template/permissive-error1b.C: New test.
	* g++.dg/template/permissive-error1c.C: New test.

Reviewed-by: Jason Merrill <jason@redhat.com>
---
 gcc/c-family/c.opt                            |  4 ++
 gcc/cp/cp-tree.h                              |  7 ++
 gcc/cp/error.cc                               | 67 +++++++++++++++++++
 gcc/cp/module.cc                              |  5 +-
 gcc/cp/pt.cc                                  | 22 ++++++
 gcc/diagnostic.cc                             |  4 ++
 gcc/diagnostic.h                              |  4 ++
 gcc/doc/invoke.texi                           | 12 +++-
 gcc/testsuite/g++.dg/ext/typedef-init.C       |  2 +-
 gcc/testsuite/g++.dg/pr84492.C                |  4 +-
 .../g++.dg/template/permissive-error1.C       | 20 ++++++
 .../g++.dg/template/permissive-error1a.C      | 31 +++++++++
 .../g++.dg/template/permissive-error1b.C      | 30 +++++++++
 .../g++.dg/template/permissive-error1c.C      | 31 +++++++++
 gcc/testsuite/g++.old-deja/g++.pt/crash51.C   |  1 -
 15 files changed, 238 insertions(+), 6 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/template/permissive-error1.C
 create mode 100644 gcc/testsuite/g++.dg/template/permissive-error1a.C
 create mode 100644 gcc/testsuite/g++.dg/template/permissive-error1b.C
 create mode 100644 gcc/testsuite/g++.dg/template/permissive-error1c.C

diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 979f17a7e323..491aa02e1a39 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -1420,6 +1420,10 @@ Wtautological-compare
 C ObjC C++ ObjC++ Var(warn_tautological_compare) Warning LangEnabledBy(C ObjC C++ ObjC++,Wall)
 Warn if a comparison always evaluates to true or false.
 
+Wtemplate-body
+C++ ObjC++ Var(warn_template_body) Warning Init(1)
+Diagnose errors when parsing a template.
+
 Wtemplate-id-cdtor
 C++ ObjC++ Var(warn_template_id_cdtor) Warning
 Warn about simple-template-id in a constructor or destructor.
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index b81bc91208fa..b1693051231b 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -7185,6 +7185,13 @@ extern location_t location_of                   (tree);
 extern void qualified_name_lookup_error		(tree, tree, tree,
 						 location_t);
 
+using erroneous_templates_t
+  = hash_map<tree, location_t, simple_hashmap_traits<tree_decl_hash, location_t>>;
+extern erroneous_templates_t *erroneous_templates;
+
+extern bool cp_seen_error ();
+#define seen_error() cp_seen_error ()
+
 /* in except.cc */
 extern void init_terminate_fn			(void);
 extern void init_exception_processing		(void);
diff --git a/gcc/cp/error.cc b/gcc/cp/error.cc
index da853e203dbc..ee3868efaed6 100644
--- a/gcc/cp/error.cc
+++ b/gcc/cp/error.cc
@@ -165,6 +165,72 @@ class cxx_format_postprocessor : public format_postprocessor
   deferred_printed_type m_type_b;
 };
 
+/* Return the in-scope template that's currently being parsed, or
+   NULL_TREE otherwise.  */
+
+static tree
+get_current_template ()
+{
+  if (scope_chain && in_template_context && !current_instantiation ())
+    if (tree ti = get_template_info (current_scope ()))
+      return TI_TEMPLATE (ti);
+
+  return NULL_TREE;
+}
+
+/* A map from TEMPLATE_DECLs that we've determined to be erroneous
+   at parse time to the location of the first error within.  */
+
+erroneous_templates_t *erroneous_templates;
+
+/* Callback function diagnostic_context::m_adjust_diagnostic_info.
+
+   Errors issued when parsing a template are automatically treated like
+   permerrors associated with the -Wtemplate-body flag and can be
+   downgraded into warnings accordingly, in which case we'll still
+   issue an error if we later need to instantiate the template.  */
+
+static void
+cp_adjust_diagnostic_info (diagnostic_context *context,
+			   diagnostic_info *diagnostic)
+{
+  if (diagnostic->kind == DK_ERROR)
+    if (tree tmpl = get_current_template ())
+      {
+	diagnostic->option_index = OPT_Wtemplate_body;
+
+	if (context->m_permissive)
+	  diagnostic->kind = DK_WARNING;
+
+	bool existed;
+	location_t &error_loc
+	  = hash_map_safe_get_or_insert<false> (erroneous_templates,
+						tmpl, &existed);
+	if (!existed)
+	  /* Remember that this template had a parse-time error so
+	     that we'll ensure a hard error has been issued upon
+	     its instantiation.  */
+	  error_loc = diagnostic->richloc->get_loc ();
+      }
+}
+
+/* A generalization of seen_error which also returns true if we've
+   permissively downgraded an error to a warning inside a template.  */
+
+bool
+cp_seen_error ()
+{
+  if ((seen_error) ())
+    return true;
+
+  if (erroneous_templates)
+    if (tree tmpl = get_current_template ())
+      if (erroneous_templates->get (tmpl))
+	return true;
+
+  return false;
+}
+
 /* CONTEXT->printer is a basic pretty printer that was constructed
    presumably by diagnostic_initialize(), called early in the
    compiler's initialization process (in general_init) Before the FE
@@ -187,6 +253,7 @@ cxx_initialize_diagnostics (diagnostic_context *context)
   diagnostic_starter (context) = cp_diagnostic_starter;
   /* diagnostic_finalizer is already c_diagnostic_finalizer.  */
   diagnostic_format_decoder (context) = cp_printer;
+  context->m_adjust_diagnostic_info = cp_adjust_diagnostic_info;
   pp_format_postprocessor (pp) = new cxx_format_postprocessor ();
 }
 
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index d1607a067575..7130faf26f52 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -20768,7 +20768,10 @@ finish_module_processing (cpp_reader *reader)
 
       cookie = new module_processing_cookie (cmi_name, tmp_name, fd, e);
 
-      if (errorcount)
+      if (errorcount
+	  /* Don't write the module if it contains an erroneous template.  */
+	  || (erroneous_templates
+	      && !erroneous_templates->is_empty ()))
 	warning_at (state->loc, 0, "not writing module %qs due to errors",
 		    state->get_flatname ());
       else if (cookie->out.begin ())
diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index 677ed7d12896..9a4ff553b2f3 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -12289,6 +12289,24 @@ perform_instantiation_time_access_checks (tree tmpl, tree targs)
       }
 }
 
+/* If the template T that we're about to instantiate contained errors at
+   parse time that we downgraded into warnings or suppressed, diagnose the
+   error now to render the TU ill-formed (if the TU has not already been
+   deemed ill-formed by an earlier error).  */
+
+static void
+maybe_diagnose_erroneous_template (tree t)
+{
+  if (erroneous_templates && !(seen_error) ())
+    if (location_t *error_loc = erroneous_templates->get (t))
+      {
+	auto_diagnostic_group d;
+	location_t decl_loc = location_of (t);
+	error_at (decl_loc, "instantiating erroneous template");
+	inform (*error_loc, "first error appeared here");
+      }
+}
+
 tree
 instantiate_class_template (tree type)
 {
@@ -12358,6 +12376,8 @@ instantiate_class_template (tree type)
   if (! push_tinst_level (type))
     return type;
 
+  maybe_diagnose_erroneous_template (templ);
+
   int saved_unevaluated_operand = cp_unevaluated_operand;
   int saved_inhibit_evaluation_warnings = c_inhibit_evaluation_warnings;
 
@@ -27251,6 +27271,8 @@ instantiate_decl (tree d, bool defer_ok, bool expl_inst_class_mem_p)
 	}
     }
 
+  maybe_diagnose_erroneous_template (td);
+
   code_pattern = DECL_TEMPLATE_RESULT (td);
 
   /* We should never be trying to instantiate a member of a class
diff --git a/gcc/diagnostic.cc b/gcc/diagnostic.cc
index 3fc81ad47f56..92bd4f808453 100644
--- a/gcc/diagnostic.cc
+++ b/gcc/diagnostic.cc
@@ -219,6 +219,7 @@ diagnostic_context::initialize (int n_opts)
   m_warn_system_headers = false;
   m_max_errors = 0;
   m_internal_error = nullptr;
+  m_adjust_diagnostic_info = nullptr;
   m_text_callbacks.m_begin_diagnostic = default_diagnostic_starter;
   m_text_callbacks.m_start_span = default_diagnostic_start_span_fn;
   m_text_callbacks.m_end_diagnostic = default_diagnostic_finalizer;
@@ -1429,6 +1430,9 @@ diagnostic_context::report_diagnostic (diagnostic_info *diagnostic)
   if (was_warning && m_inhibit_warnings)
     return false;
 
+  if (m_adjust_diagnostic_info)
+    m_adjust_diagnostic_info (this, diagnostic);
+
   if (diagnostic->kind == DK_PEDWARN)
     {
       diagnostic->kind = m_pedantic_errors ? DK_ERROR : DK_WARNING;
diff --git a/gcc/diagnostic.h b/gcc/diagnostic.h
index 83180ded414d..2a9f2751dca2 100644
--- a/gcc/diagnostic.h
+++ b/gcc/diagnostic.h
@@ -701,6 +701,10 @@ public:
   /* Client hook to report an internal error.  */
   void (*m_internal_error) (diagnostic_context *, const char *, va_list *);
 
+  /* Client hook to adjust properties of the given diagnostic that we're
+     about to issue, such as its kind.  */
+  void (*m_adjust_diagnostic_info)(diagnostic_context *, diagnostic_info *);
+
 private:
   /* Client-supplied callbacks for working with options.  */
   struct {
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 0fe99ca8ef6e..27539a017851 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -270,7 +270,8 @@ in the following sections.
 -Wno-non-template-friend  -Wold-style-cast
 -Woverloaded-virtual  -Wno-pmf-conversions -Wself-move -Wsign-promo
 -Wsized-deallocation  -Wsuggest-final-methods
--Wsuggest-final-types  -Wsuggest-override  -Wno-template-id-cdtor
+-Wsuggest-final-types  -Wsuggest-override  -Wno-template-body
+-Wno-template-id-cdtor
 -Wno-terminate  -Wno-vexing-parse  -Wvirtual-inheritance
 -Wno-virtual-move-assign  -Wvolatile  -Wzero-as-null-pointer-constant}
 
@@ -4634,6 +4635,14 @@ namespaces, and this may be used to enforce that rule.  The warning is
 inactive inside a system header file, such as the STL, so one can still
 use the STL.  One may also use using directives and qualified names.
 
+@opindex Wtemplate-body
+@opindex Wno-template-body
+@item -Wno-template-body @r{(C++ and Objective-C++ only)}
+Disable diagnosing errors when parsing a template, and instead issue an
+error only upon instantiation of the template.  This flag can also be
+used to downgrade such errors into warnings with @option{Wno-error=} or
+@option{-fpermissive}.
+
 @opindex Wtemplate-id-cdtor
 @opindex Wno-template-id-cdtor
 @item -Wno-template-id-cdtor @r{(C++ and Objective-C++ only)}
@@ -6361,6 +6370,7 @@ that have their own flag:
 -Wint-conversion @r{(C and Objective-C only)}
 -Wnarrowing @r{(C++ and Objective-C++ only)}
 -Wreturn-mismatch @r{(C and Objective-C only)}
+-Wtemplate-body @r{(C++ and Objective-C++ only)}
 }
 
 The @option{-fpermissive} option is the default for historic C language
diff --git a/gcc/testsuite/g++.dg/ext/typedef-init.C b/gcc/testsuite/g++.dg/ext/typedef-init.C
index 153303d217b5..47a6642de510 100644
--- a/gcc/testsuite/g++.dg/ext/typedef-init.C
+++ b/gcc/testsuite/g++.dg/ext/typedef-init.C
@@ -32,5 +32,5 @@ struct S {
 
 template<int> void foo()
 {
-    typedef int i = 0; /* { dg-error "is initialized" } */
+    typedef int i = 0; /* { dg-warning "is initialized" } */
 }
diff --git a/gcc/testsuite/g++.dg/pr84492.C b/gcc/testsuite/g++.dg/pr84492.C
index 1a2922096d19..08f368ff29b6 100644
--- a/gcc/testsuite/g++.dg/pr84492.C
+++ b/gcc/testsuite/g++.dg/pr84492.C
@@ -3,7 +3,7 @@
 
 template<int> int foo()
 {
-  return ({ foo; }); // { dg-error "insufficient context" }
+  return ({ foo; }); // { dg-warning "insufficient context" }
 }
 
 int bar()
@@ -35,6 +35,6 @@ class C
   }
   bool g(int)
   {
-    return ({ g; }); // { dg-error "insufficient context" }
+    return ({ g; }); // { dg-warning "insufficient context" }
   }
 };
diff --git a/gcc/testsuite/g++.dg/template/permissive-error1.C b/gcc/testsuite/g++.dg/template/permissive-error1.C
new file mode 100644
index 000000000000..e4536a8b90e0
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/permissive-error1.C
@@ -0,0 +1,20 @@
+// PR c++/116064
+// { dg-additional-options -fpermissive }
+
+template<class T>
+void f() {
+  const int n = 42;
+  ++n; // { dg-warning "read-only" }
+}
+
+template<class T>
+struct A {
+  void f(typename A::type); // { dg-warning "does not name a type" }
+};
+
+template<class T>
+struct B {
+  void f() {
+    this->g(); // { dg-warning "no member" }
+  }
+};
diff --git a/gcc/testsuite/g++.dg/template/permissive-error1a.C b/gcc/testsuite/g++.dg/template/permissive-error1a.C
new file mode 100644
index 000000000000..ac47151f0dba
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/permissive-error1a.C
@@ -0,0 +1,31 @@
+// PR c++/116064
+// { dg-additional-options -fpermissive }
+// Like permissive-error1.C but verify instantiating the errorneous
+// templates gives an error after all.
+
+template<class T>
+void f() {  // { dg-error "instantiating erroneous template" }
+  const int n = 42;
+  ++n; // { dg-warning "read-only variable 'n' .-Wtemplate-body." }
+       // { dg-message "first error appeared here" "" { target *-*-* } .-1 }
+       // { dg-error "read-only variable 'n'\[\n\r\]" "" { target *-*-* } .-2 }
+}
+
+template<class T>
+struct A {
+  void f(typename A::type); // { dg-warning "does not name a type" }
+};
+
+template<class T>
+struct B {
+  void f() {
+    this->g(); // { dg-warning "no member" }
+  }
+};
+
+int main() {
+  f<int>(); // { dg-message "required from here" }
+  A<int> a;
+  B<int> b;
+  b.f();
+}
diff --git a/gcc/testsuite/g++.dg/template/permissive-error1b.C b/gcc/testsuite/g++.dg/template/permissive-error1b.C
new file mode 100644
index 000000000000..a67b73746256
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/permissive-error1b.C
@@ -0,0 +1,30 @@
+// PR c++/116064
+// { dg-additional-options -Wno-template-body }
+// Like permissive-error1a.C but verify -Wno-template-body suppresses
+// diagnostics.
+
+template<class T>
+void f() {  // { dg-error "instantiating erroneous template" }
+  const int n = 42;
+  ++n; // { dg-message "first error appeared here" "" { target *-*-* } }
+       // { dg-error "read-only variable 'n'\[\n\r\]" "" { target *-*-* } .-1 }
+}
+
+template<class T>
+struct A {
+  void f(typename A::type);
+};
+
+template<class T>
+struct B {
+  void f() {
+    this->g();
+  }
+};
+
+int main() {
+  f<int>(); // { dg-message "required from here" }
+  A<int> a;
+  B<int> b;
+  b.f();
+}
diff --git a/gcc/testsuite/g++.dg/template/permissive-error1c.C b/gcc/testsuite/g++.dg/template/permissive-error1c.C
new file mode 100644
index 000000000000..fd5f26655e54
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/permissive-error1c.C
@@ -0,0 +1,31 @@
+// PR c++/116064
+// { dg-additional-options -Wno-error=template-body }
+// Like permissive-error1a.C but verify the diagnostics can also
+// be downgraded via Wno-error=template-body.
+
+template<class T>
+void f() {  // { dg-error "instantiating erroneous template" }
+  const int n = 42;
+  ++n; // { dg-warning "read-only variable 'n' .-Wtemplate-body." }
+       // { dg-message "first error appeared here" "" { target *-*-* } .-1 }
+       // { dg-error "read-only variable 'n'\[\n\r\]" "" { target *-*-* } .-2 }
+}
+
+template<class T>
+struct A {
+  void f(typename A::type); // { dg-warning "does not name a type" }
+};
+
+template<class T>
+struct B {
+  void f() {
+    this->g(); // { dg-warning "no member" }
+  }
+};
+
+int main() {
+  f<int>(); // { dg-message "required from here" }
+  A<int> a;
+  B<int> b;
+  b.f();
+}
diff --git a/gcc/testsuite/g++.old-deja/g++.pt/crash51.C b/gcc/testsuite/g++.old-deja/g++.pt/crash51.C
index a3fbc17f163b..c5cbde521ade 100644
--- a/gcc/testsuite/g++.old-deja/g++.pt/crash51.C
+++ b/gcc/testsuite/g++.old-deja/g++.pt/crash51.C
@@ -1,5 +1,4 @@
 // { dg-do assemble  }
-// { dg-options "-fpermissive -w" }
 // Origin: Mark Mitchell <mark@codesourcery.com>
 
 char foo[26];
-- 
GitLab