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