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.
Jason
