On Thu, 24 Apr 2025 at 15:48, Jonathan Wakely <jwak...@redhat.com> wrote:
>
> On Fri, 18 Apr 2025 at 23:08, Jason Merrill <ja...@redhat.com> wrote:
> >
> > limit_bad_template_recursion currently avoids immediate instantiation of
> > templates from uses in an already ill-formed instantiation, but we still can
> > get unnecessary recursive instantiation in pending_templates if the
> > instantiation was queued before the error.
> >
> > Currently this regresses several libstdc++ tests which seem to rely on a
> > static_assert in a function called from another that is separately 
> > ill-formed.
> > For instance, in the 48101_neg.cc tests, we first get an error in find(), 
> > then
> > later instantiate _S_key() (called from find) and get the static_assert 
> > error
> > from there.
> >
> > Thoughts?  Is this a desirable change, or is the fact that the use precedes 
> > the
> > error reason to go ahead with the instantiation?
> >
> > > FAIL: 23_containers/map/48101_neg.cc  -std=gnu++17  (test for errors, 
> > > line )
> > > FAIL: 23_containers/multimap/48101_neg.cc  -std=gnu++17  (test for 
> > > errors, line )
> > > FAIL: 23_containers/multiset/48101_neg.cc  -std=gnu++17  (test for 
> > > errors, line )
> > > FAIL: 23_containers/set/48101_neg.cc  -std=gnu++17  (test for errors, 
> > > line )
>
> As discussed the other day, I will tweak the diagnostics for the
> associative containers so that these tests don't regress. I'll post
> that for review shortly.
>
> > > FAIL: 30_threads/packaged_task/cons/dangling_ref.cc  -std=gnu++17  (test 
> > > for errors, line )
> > > FAIL: 30_threads/packaged_task/cons/lwg4154_neg.cc  -std=gnu++17  (test 
> > > for errors, line )
>
> These two don't strictly need to use dg-error for the messages that
> disappear with your patch, they could have used dg-prune-output
> instead. What the test intends to check is that we get a failed
> static_assert for std::is_invocable_r_v, and that's still the case.
> The other errors are collateral damage, and your patch prevents that.
> Which is a good thing. So we can just remove the failing dg-error
> lines.
>
> I'll finish testing my associative container changes, then push your
> patch after mine lands.

The front end patch has been pushed as r16-133-g8acea9ffa82ed8


>
> >
> >
> > gcc/cp/ChangeLog:
> >
> >         * cp-tree.h (struct tinst_level): Add had_errors bit.
> >         * pt.cc (push_tinst_level_loc): Clear it.
> >         (pop_tinst_level): Set it.
> >         (reopen_tinst_level): Check it.
> >         (instantiate_pending_templates): Call limit_bad_template_recursion.
> >
> > gcc/testsuite/ChangeLog:
> >
> >         * g++.dg/template/recurse5.C: New test.
> > ---
> >  gcc/cp/cp-tree.h                         | 10 ++++++++--
> >  gcc/cp/pt.cc                             | 10 ++++++++--
> >  gcc/testsuite/g++.dg/template/recurse5.C | 17 +++++++++++++++++
> >  3 files changed, 33 insertions(+), 4 deletions(-)
> >  create mode 100644 gcc/testsuite/g++.dg/template/recurse5.C
> >
> > diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> > index 7798efba3db..856202c65dd 100644
> > --- a/gcc/cp/cp-tree.h
> > +++ b/gcc/cp/cp-tree.h
> > @@ -6755,8 +6755,14 @@ struct GTY((chain_next ("%h.next"))) tinst_level {
> >    /* The location where the template is instantiated.  */
> >    location_t locus;
> >
> > -  /* errorcount + sorrycount when we pushed this level.  */
> > -  unsigned short errors;
> > +  /* errorcount + sorrycount when we pushed this level.  If the value
> > +     overflows, it will always seem like we currently have more errors, so 
> > we
> > +     will limit template recursion even from non-erroneous templates.  In 
> > a TU
> > +     with over 32k errors, that's fine.  */
> > +  unsigned short errors : 15;
> > +
> > +  /* set in pop_tinst_level if there have been errors since we pushed.  */
> > +  bool had_errors : 1;
> >
> >    /* Count references to this object.  If refcount reaches
> >       refcount_infinity value, we don't increment or decrement the
> > diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> > index a71705fd085..e8d342f99f6 100644
> > --- a/gcc/cp/pt.cc
> > +++ b/gcc/cp/pt.cc
> > @@ -11418,6 +11418,7 @@ push_tinst_level_loc (tree tldcl, tree targs, 
> > location_t loc)
> >    new_level->targs = targs;
> >    new_level->locus = loc;
> >    new_level->errors = errorcount + sorrycount;
> > +  new_level->had_errors = false;
> >    new_level->next = NULL;
> >    new_level->refcount = 0;
> >    new_level->path = new_level->visible = nullptr;
> > @@ -11468,6 +11469,9 @@ pop_tinst_level (void)
> >    /* Restore the filename and line number stashed away when we started
> >       this instantiation.  */
> >    input_location = current_tinst_level->locus;
> > +  if (unsigned errs = errorcount + sorrycount)
> > +    if (errs > current_tinst_level->errors)
> > +      current_tinst_level->had_errors = true;
> >    set_refcount_ptr (current_tinst_level, current_tinst_level->next);
> >    --tinst_depth;
> >  }
> > @@ -11487,7 +11491,7 @@ reopen_tinst_level (struct tinst_level *level)
> >
> >    set_refcount_ptr (current_tinst_level, level);
> >    pop_tinst_level ();
> > -  if (current_tinst_level)
> > +  if (current_tinst_level && !current_tinst_level->had_errors)
> >      current_tinst_level->errors = errorcount+sorrycount;
> >
> >    tree decl = level->maybe_get_node ();
> > @@ -28072,7 +28076,9 @@ instantiate_pending_templates (int retries)
> >           tree instantiation = reopen_tinst_level ((*t)->tinst);
> >           bool complete = false;
> >
> > -         if (TYPE_P (instantiation))
> > +         if (limit_bad_template_recursion (instantiation))
> > +           /* Do nothing.  */;
> > +         else if (TYPE_P (instantiation))
> >             {
> >               if (!COMPLETE_TYPE_P (instantiation))
> >                 {
> > diff --git a/gcc/testsuite/g++.dg/template/recurse5.C 
> > b/gcc/testsuite/g++.dg/template/recurse5.C
> > new file mode 100644
> > index 00000000000..7bfe5239f0a
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/template/recurse5.C
> > @@ -0,0 +1,17 @@
> > +// Test that we don't bother to instantiate add since there were errors in
> > +// checked_add.
> > +
> > +template <class T> T add (T t) { return t+1; } // { dg-bogus "no match" }
> > +
> > +template <class T> T checked_add (T t)
> > +{
> > +  add (t);
> > +  return t+1;                  // { dg-error "no match" }
> > +}
> > +
> > +struct A { };
> > +
> > +int main()
> > +{
> > +  checked_add (A());
> > +}
> >
> > base-commit: 6808f74b4f07decb3727624f0e62e7c57ae87022
> > --
> > 2.49.0
> >

Reply via email to