On Tue, 2024-08-06 at 17:52 -0400, Jason Merrill wrote:
> On 8/6/24 5:47 PM, Patrick Palka wrote:
> > 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_i
> > > > > > > > > > > > > > > nfo.
> > > > > > > > > > > > > > > +
> > > > > > > > > > > > > > > + 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");
> > + }
I confess I haven't been paying proper attention to this thread -
sorry, but please can you add an
auto_diagnostic_group d;
before the "error_at", so that the "inform" is assocated with it.
Thanks
Dave
>
> Let's factor this out instead of repeating it. OK with that change.
>
> Jason
>
