On Fri, 18 Apr 2025, Jason Merrill 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?
Not saying we're at this point yet, but I do worry that being too
aggressive about avoiding error cascades could make the compiler less
noisy at the expense of making it less predictable, and make it more
likely that multiple edit+recompile cycles are needed before a TU is
error-free.
In recurse5.C the emitted error (in checked_add) and the suppressed
error (in add) are caused by the same bug so the suprression is clearly
an improvement, but if they're different bugs then the user will first
see only the add error, which they will fix and rightfully hope the TU
is now error-free, but recompilation will then reveal the suppressed
error in checked_add and the user might wonder why wasn't this error
wasn't reported during the first compile.
When it comes to avoiding too many errors from deferred instantiations
in particular, -fmax-errors works nicely for that since such errors are
naturally the last to be emitted.
>
> > 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 )
> > 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 )
>
>
> 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" }
FWIW I noticed if we need to eagerly instantiate add (e.g. if we give it
a deduced return type), then we again emit two "no match" errors. I guess
this is expected/intended?
> +}
> +
> +struct A { };
> +
> +int main()
> +{
> + checked_add (A());
> +}
>
> base-commit: 6808f74b4f07decb3727624f0e62e7c57ae87022
> --
> 2.49.0
>
>
