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