On 8/1/24 2:52 PM, Patrick Palka wrote:
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 maintenence mode.  So it'd be useful to

"maintenance"

diff --git a/gcc/cp/error.cc b/gcc/cp/error.cc
index d80bac822ba..0bb0a482e28 100644
--- a/gcc/cp/error.cc
+++ b/gcc/cp/error.cc
@@ -165,6 +165,58 @@ class cxx_format_postprocessor : public 
format_postprocessor
    deferred_printed_type m_type_b;
  };
+/* A map from TEMPLATE_DECL to the location of the first error (if any)
+   within the template that we permissivly downgraded to a warning.  */

"permissively"

+relaxed_template_errors_t *relaxed_template_errors;
+
+/* Callback function diagnostic_context::m_adjust_diagnostic_info.
+
+   In -fpermissive mode we downgrade errors within a template to
+   warnings, and only issue an error if we later need to instantiate
+   the template.  */
+
+static void
+cp_adjust_diagnostic_info (diagnostic_context *context,
+                          diagnostic_info *diagnostic)
+{
+  tree ti;
+  if (diagnostic->kind == DK_ERROR
+      && context->m_permissive
+      && !current_instantiation ()
+      && in_template_context
+      && (ti = get_template_info (current_scope ())))
+    {
+      if (!relaxed_template_errors)
+       relaxed_template_errors = new relaxed_template_errors_t;
+
+      tree tmpl = TI_TEMPLATE (ti);
+      if (!relaxed_template_errors->get (tmpl))
+       relaxed_template_errors->put (tmpl, diagnostic->richloc->get_loc ());
+      diagnostic->kind = DK_WARNING;

Rather than check m_permissive directly and downgrade to DK_WARNING, how about downgrading to DK_PERMERROR? That way people will get the [-fpermissive] clue.

...though I suppose DK_PERMERROR doesn't work where you call this hook in report_diagnostic, at which point we've already reassigned it into DK_WARNING or DK_ERROR in diagnostic_impl.

But we could still set diagnostic->option_index even for DK_ERROR, whether to context->m_opt_permissive or to its own warning flag, perhaps -Wno-template-body?

+/* 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 ()
+{
+#undef seen_error
+  if (seen_error ())
+    return true;
+
+  tree ti;
+  if (relaxed_template_errors
+      && in_template_context
+      && (ti = get_template_info (current_scope ()))

Let's factor the "in template body" checks in this function and the previous one into a separate function; I expect we want the !current_instantiation test in this case as well?

diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index 77fa5907c3d..b58ccb318d5 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -12376,6 +12376,22 @@ instantiate_class_template (tree type)
    if (! push_tinst_level (type))
      return type;
+ if (relaxed_template_errors)
+    if (location_t *error_loc = relaxed_template_errors->get (templ))
+      {
+       /* We're trying to instantiate a template pattern containing
+          an error that we've permissively downgraded to a warning.
+          Issue a hard error now.  */
+       location_t decl_loc = location_of (templ);
+       error_at (decl_loc, "instantiating erroneous template pattern");

I wouldn't use the internal term "pattern" in diagnostics, I think just "erroneous template" is enough.

@@ -27291,6 +27307,20 @@ instantiate_decl (tree d, bool defer_ok, bool 
expl_inst_class_mem_p)
        }
      }
+ if (relaxed_template_errors)
+    if (location_t *error_loc = relaxed_template_errors->get (td))
+      {
+       /* We're trying to instantiate a template pattern containing
+          an error that we've permissively downgraded to a warning.
+          Issue a hard error now.  */
+       location_t decl_loc = location_of (td);
+       error_at (decl_loc, "instantiating erroneous template pattern");

Likewise.

diff --git a/gcc/diagnostic.cc b/gcc/diagnostic.cc
index 71d2f44e40c..cbd2c82f19d 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;
@@ -1409,6 +1410,9 @@ diagnostic_context::report_diagnostic (diagnostic_info 
*diagnostic)
       flush diagnostics with on_end_group when the topmost group is ended.  */
    gcc_assert (m_diagnostic_groups.m_nesting_depth > 0);
+ if (m_adjust_diagnostic_info)
+    m_adjust_diagnostic_info (this, diagnostic);

I wonder about the ordering of calling the hook vs setting was_warning. I suppose it doesn't make a practical difference in this case; if it ends up as a warning it'll be suppressed by -w or -Wno-system-header whether or not was_warning is set.

It would make a difference if a hook wanted to upgrade warning to error; in this position -w would not effect such an upgraded diagnostic.

I lean toward moving the call after the was_warning bits, so was_warning reflects whether the diagnostic was a warning before this adjustment just as it does before other adjustments.

Jason

Reply via email to