On Tue, 6 Aug 2024, Jason Merrill wrote:
On 8/6/24 2:00 PM, Patrick Palka wrote:
On Tue, 6 Aug 2024, Jason Merrill wrote:
On 8/5/24 6:09 PM, Patrick Palka wrote:
On Mon, 5 Aug 2024, Jason Merrill wrote:
On 8/5/24 3:47 PM, Patrick Palka wrote:
On Mon, 5 Aug 2024, Jason Merrill wrote:
On 8/5/24 1:14 PM, Patrick Palka wrote:
On Mon, 5 Aug 2024, Jason Merrill wrote:
On 8/2/24 4:18 PM, Patrick Palka wrote:
On Fri, 2 Aug 2024, Patrick Palka wrote:
On Fri, 2 Aug 2024, Jason Merrill wrote:
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"
Fixed
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"
Fixed
+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?
Fixed by adding an enabled-by-default -Wtemplate-body flag
and
setting
option_index to it for each downgraded error. Thus
-permissive
-Wno-template-body would suppress the downgraded warnings
entirely,
and
only issue a generic error upon instantiation of the
erroneous
template.
... or did you have in mind to set option_index even when
not
using
-fpermissive so that eligible non-downgraded errors get the
[-fpermissive] or [-Wtemplate-body] hint as well?
Yes.
IMHO I'm not sure that'd be worth the extra noise since the
vast
majority of users appreciate and expect errors to get
diagnosed
inside
templates.
But people trying to build legacy code should appreciate the
pointer
for
how
to make it compile, as with other permerrors.
And on second thought I'm not sure what extra value a new
warning
flag
adds either. I can't think of a good reason why one would
use
-fpermissive -Wno-template-body?
One would use -Wno-template-body (or -Wno-error=template-body)
without
-fpermissive, like with the various permerror_opt cases.
Since compiling legacy/unmaintained code is the only plausible
use
case,
why have a dedicated warning flag instead of just recommending
-fpermissive
when compiling legacy code? I don't quite understand the
motivation
for
adding a new permerror_opt flag for this class of errors.
It seems to me an interesting class of errors, but I don't mind
leaving it
under just -fpermissive if you prefer.
-Wnarrowing is an existing permerror_opt flag, but I can imagine
it's
useful to pass -Wno-error=narrowing etc when incrementally
migrating
C / C++98 code to modern C++ where you don't want any
conformance
errors
allowed by -fpermissive to sneak in. So being able to narrowly
control
this class of errors seems useful, so a dedicated flag makes
sense.
But there's no parallel for -Wtemplate-body here, since by
assumption
the code base is unmaintained / immutable. Otherwise the more
proper
fix would be to just fix and/or delete the uninstantiated
erroneous
template. If say you're #including a legacy header that has
such
errors, then doing #pragma GCC diagnostic "-fpermissive -w"
around
the #include should be totally fine too.
I just realized #pragma GCC diagnostic warning "-fpermissive" etc
doesn't actually work since -fpermissive isn't a warning flag. So
having a dedicated flag for the class of errors has at least one
clear
benefit -- we can use #pragmas to selectively disable the errors
now.
I just don't see the use case for being able to narrowly control
this
class of errors that justifies the extra implementation
complexity
(specifically for properly detecting -Wno-error=template-body in
the
callback hook)?
The hook shouldn't need to do anything special; report_diagnostic
handles
-Wno-error=whatever.
The issue was that the callback has to know in advance whether
-Wno-error=template-body is active so that it can flag the template
as
having a relaxed error. Checking !warning_enabled_at wasn't enough
because it will return true even for -Wno-error=template-body. I
think we just need to check diagnostic_enabled directly, like so?
Why not always flag it? It doesn't seem harmful to give the
"instatiating
erroneous template" error later even if we gave a hard error during
parsing.
Hmm, it just seems redundant to me I guess? The reason we need the
"instantiating erroneous template" error is to ensure that _some_ error
was issued rendering the TU ill-formed when an erroneous template gets
instantiated. If parse-time errors are hard errors then this is
guaranteed, but it's not if we're downgrading such errors.
Yes, it's redundant, but avoiding it doesn't seem worth duplicating all
the
logic to determine whether it will be an error or not.
Simpler might be to skip the "instantiating" error if seen_error()?
Sure. This makes the expected diagnostics in the -Wno-template-body
testcase a bit lacking since there's three instantiated templates with
one logical error each, but only the first error is diagnosed. But I
don't particularly mind that.
On that note it just occurred to me that we don't need to abort
instantiation after the "instantiating erroneous template" error --
for sake of error recovery we can proceed to instantiate as if we
issued a hard error at parse time.
Agreed.
Jason
-- >8 --
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
premerrors associated with a new -Wtemplate-body flag so that they can
perm
be downgraded via e.g. -fpermissive or -Wno-template=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 if we detect it's
occurring from a template context and additionally flags this template
context as erroneous.
As an example, 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 pattern
Drop "pattern" here too.
Fixed
5 | void f() {
| ^
gcc/testsuite/g++.dg/template/permissive-error1a.C:7:5: note: first error
appeared here
7 | ++n; // {
| ^
...
What happens if we compile an interface/header unit with an erroneous
uninstantiated template? I'd probably reject such a module rather than write
out the erroneity. Possibly just if the template isn't discarded, not sure if
that distinction is too much trouble to bother with.
I think it's good enough to uniformly reject the module given how small
the intersection -fpermissive and modules users is, I imagine.
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.
(basic_seen_error): Define as alias to existing seen_error.
Or we could use (seen_error)() where we mean the basic one?
Sure, changed.
(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.
* pt.cc (instantiate_class_template): Issue a hard error
when trying to instantiate a template pattern containing
a permissively downgraded error.
(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::diagnostic_enabled): Make
public.
You shouldn't need this change now?
Oops, removed.
+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;
+
+ if (!erroneous_templates)
+ erroneous_templates = new erroneous_templates_t;
+ if (!erroneous_templates->get (tmpl))
+ {
+ /* Remember that this template had a parse-time error so
+ that we'll ensure a hard error has been issued upon
+ its instantiation. */
+ location_t error_loc = diagnostic->richloc->get_loc ();
+ erroneous_templates->put (tmpl, error_loc);
Instead of get+put you could use the get return value to store error_loc?
We need to use get_or_insert if we want to combine the two lookups.
I went ahead and used hash_map_safe_get_or_insert here to get rid
of the construction boilerplate as well.
+@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}.
Please also refer to this flag in the -fpermissive documentation.
Done.
-- >8 --
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-template=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 if we detect it's
occurring from a template context and additionally flags this template
context as erroneous.
As an example, 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 (instantiate_class_template): Issue a hard error
when trying to instantiate a template containing a
permissively downgraded error.
(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.
---
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 | 24 +++++++
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, 240 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 a52682d835c..44117ba713c 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 238d786b067..08ee5895c6a 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -7190,6 +7190,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 d80bac822ba..7eab0bf76b4 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 d1607a06757..7130faf26f5 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 77fa5907c3d..b83922a530d 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -12376,6 +12376,18 @@ instantiate_class_template (tree type)
if (! push_tinst_level (type))
return type;
+ if (erroneous_templates && !(seen_error) ())
+ if (location_t *error_loc = erroneous_templates->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 to ensure the TU is considered
+ ill-formed. */
+ location_t decl_loc = location_of (templ);
+ error_at (decl_loc, "instantiating erroneous template");
+ inform (*error_loc, "first error appeared here");
+ }
