On Wed, 30 Jul 2025, Nathaniel Shead wrote:
> On Tue, Jul 29, 2025 at 03:45:29PM -0400, Patrick Palka wrote:
> > On Tue, 29 Jul 2025, Nathaniel Shead wrote:
> >
> > > Tested on x86_64-pc-linux-gnu, OK for trunk if full bootstrap+regtest
> > > passes?
> > >
> > > -- >8 --
> > >
> > > For the sake of determining if there are other errors in user code to
> > > report early, many trait functions don't return error_mark_node if not
> > > called in a SFINAE context (i.e., tf_error is set). This patch removes
> > > some assumptions on this behaviour I'd made when improving diagnostics
> > > of builtin traits.
> > >
> > > PR c++/121291
> > >
> > > gcc/cp/ChangeLog:
> > >
> > > * constraint.cc (diagnose_trait_expr): Remove assumption about
> > > failures returning error_mark_node.
> > > * method.cc (build_invoke): Adjust comment.
> > > (is_nothrow_convertible): Check for errorcount changes.
> > >
> > > gcc/testsuite/ChangeLog:
> > >
> > > * g++.dg/ext/is_invocable7.C: New test.
> > > * g++.dg/ext/is_nothrow_convertible5.C: New test.
> > >
> > > Signed-off-by: Nathaniel Shead <nathanielosh...@gmail.com>
> > > ---
> > > gcc/cp/constraint.cc | 9 +++++---
> > > gcc/cp/method.cc | 9 ++++++--
> > > gcc/testsuite/g++.dg/ext/is_invocable7.C | 21 +++++++++++++++++++
> > > .../g++.dg/ext/is_nothrow_convertible5.C | 15 +++++++++++++
> > > 4 files changed, 49 insertions(+), 5 deletions(-)
> > > create mode 100644 gcc/testsuite/g++.dg/ext/is_invocable7.C
> > > create mode 100644 gcc/testsuite/g++.dg/ext/is_nothrow_convertible5.C
> > >
> > > diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
> > > index d4a83e429e5..94238c9e8f1 100644
> > > --- a/gcc/cp/constraint.cc
> > > +++ b/gcc/cp/constraint.cc
> > > @@ -3176,8 +3176,7 @@ diagnose_trait_expr (location_t loc, tree expr,
> > > tree args)
> > > inform (loc, "%qT is not invocable, because", t1);
> > > else
> > > inform (loc, "%qT is not invocable by %qT, because", t1, t2);
> > > - tree call = build_invoke (t1, t2, tf_error);
> > > - gcc_assert (call == error_mark_node);
> > > + build_invoke (t1, t2, tf_error);
> > > }
> > > break;
> > > case CPTK_IS_LAYOUT_COMPATIBLE:
> > > @@ -3221,8 +3220,12 @@ diagnose_trait_expr (location_t loc, tree expr,
> > > tree args)
> > > inform (loc, "%qT is not nothrow invocable, because", t1);
> > > else
> > > inform (loc, "%qT is not nothrow invocable by %qT, because", t1, t2);
> > > + /* We can't rely on build_invoke returning error_mark_node, because
> > > + when passing tf_error in some cases a valid call may be returned
> > > + anyway. */
> > > + int errs = errorcount + sorrycount;
> > > tree call = build_invoke (t1, t2, tf_error);
> > > - if (call != error_mark_node)
> > > + if (call != error_mark_node && errs == errorcount + sorrycount)
> > > explain_not_noexcept (call);
> >
> > What do you think about just checking expr_noexcept_p here instead (or
> > removing the assert in explain_not_noexcept)? So that if a call is not
> > nothrow invocable because the selected candidate is private _and_
> > because it's not noexcept, we'd report both errors. I see that
> > tsubst_compound_requirement already behaves this way.
>
> Good point, I think I like this more; it's definitely the less complex
> behaviour and is also potentially useful. Here's an adjusted patch that
> does this; bootstrapped and regtested on x86_64-pc-linux-gnu, OK for
> trunk?
>
> -- >8 --
>
> For the sake of determining if there are other errors in user code to
> report early, many trait functions don't always return error_mark_node
> if not called in a SFINAE context (i.e., tf_error is set). This patch
> removes some assumptions on this behaviour I'd made when improving
> diagnostics of builtin traits.
>
> PR c++/121291
>
> gcc/cp/ChangeLog:
>
> * constraint.cc (diagnose_trait_expr): Remove assumption about
> failures returning error_mark_node.
> * except.cc (explain_not_noexcept): Allow expr not being
> noexcept.
> * method.cc (build_invoke): Adjust comment.
> (is_trivially_xible): Always note non-trivial components if expr
> is not null or error_mark_node.
> (is_nothrow_xible): Likewise for non-noexcept components.
> (is_nothrow_convertible): Likewise.
>
> gcc/testsuite/ChangeLog:
>
> * g++.dg/ext/is_invocable7.C: New test.
> * g++.dg/ext/is_nothrow_convertible5.C: New test.
>
> Signed-off-by: Nathaniel Shead <nathanielosh...@gmail.com>
> Reviewed-by: Patrick Palka <ppa...@redhat.com>
> ---
> gcc/cp/constraint.cc | 3 +-
> gcc/cp/except.cc | 7 ++--
> gcc/cp/method.cc | 33 +++++--------------
> gcc/testsuite/g++.dg/ext/is_invocable7.C | 21 ++++++++++++
> .../g++.dg/ext/is_nothrow_convertible5.C | 15 +++++++++
> 5 files changed, 50 insertions(+), 29 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/ext/is_invocable7.C
> create mode 100644 gcc/testsuite/g++.dg/ext/is_nothrow_convertible5.C
>
> diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
> index d4a83e429e5..cbdfafc90c0 100644
> --- a/gcc/cp/constraint.cc
> +++ b/gcc/cp/constraint.cc
> @@ -3176,8 +3176,7 @@ diagnose_trait_expr (location_t loc, tree expr, tree
> args)
> inform (loc, "%qT is not invocable, because", t1);
> else
> inform (loc, "%qT is not invocable by %qT, because", t1, t2);
> - tree call = build_invoke (t1, t2, tf_error);
> - gcc_assert (call == error_mark_node);
> + build_invoke (t1, t2, tf_error);
> }
> break;
> case CPTK_IS_LAYOUT_COMPATIBLE:
> diff --git a/gcc/cp/except.cc b/gcc/cp/except.cc
> index 2c1ef4c3af6..c0b93df9456 100644
> --- a/gcc/cp/except.cc
> +++ b/gcc/cp/except.cc
> @@ -1218,13 +1218,14 @@ expr_noexcept_p (tree expr, tsubst_flags_t complain)
> return true;
> }
>
> -/* Explain why EXPR is not noexcept. */
> +/* If EXPR is not noexcept, explain why. */
>
> void explain_not_noexcept (tree expr)
While we're changing this function could you also fix the formatting
here (newline b/t return type and function name)? OK with that change,
thanks!
> {
> tree fn = cp_walk_tree_without_duplicates (&expr, check_noexcept_r, 0);
> - gcc_assert (fn);
> - if (DECL_P (fn))
> + if (!fn)
> + /* The call was noexcept, nothing to do. */;
> + else if (DECL_P (fn))
> inform (DECL_SOURCE_LOCATION (fn), "%qD is not %<noexcept%>", fn);
> else
> inform (location_of (fn), "%qT is not %<noexcept%>", TREE_TYPE (fn));
> diff --git a/gcc/cp/method.cc b/gcc/cp/method.cc
> index 62f8d80c781..397e496ed47 100644
> --- a/gcc/cp/method.cc
> +++ b/gcc/cp/method.cc
> @@ -1952,7 +1952,8 @@ build_trait_object (tree type, tsubst_flags_t complain)
> }
>
> /* [func.require] Build an expression of INVOKE(FN_TYPE, ARG_TYPES...). If
> the
> - given is not invocable, returns error_mark_node. */
> + given is not invocable, returns error_mark_node, unless COMPLAIN includes
> + tf_error. */
>
> tree
> build_invoke (tree fn_type, const_tree arg_types, tsubst_flags_t complain)
> @@ -2460,21 +2461,13 @@ bool
> is_trivially_xible (enum tree_code code, tree to, tree from,
> bool explain/*=false*/)
> {
> - /* In some cases, when producing errors is_xible_helper may not return
> - error_mark_node, so check if it looks like we've already emitted any
> - diagnostics to ensure we don't do so multiple times. */
> - int errs = errorcount + sorrycount;
> -
> tree expr = is_xible_helper (code, to, from, explain);
> if (expr == NULL_TREE || expr == error_mark_node)
> return false;
>
> tree nt = cp_walk_tree_without_duplicates (&expr, check_nontriv, NULL);
> - if (explain && errs == (errorcount + sorrycount))
> - {
> - gcc_assert (nt);
> - inform (location_of (nt), "%qE is non-trivial", nt);
> - }
> + if (explain && nt)
> + inform (location_of (nt), "%qE is non-trivial", nt);
> return !nt;
> }
>
> @@ -2487,9 +2480,6 @@ bool
> is_nothrow_xible (enum tree_code code, tree to, tree from,
> bool explain/*=false*/)
> {
> - /* As with is_trivially_xible. */
> - int errs = errorcount + sorrycount;
> -
> ++cp_noexcept_operand;
> tree expr = is_xible_helper (code, to, from, explain);
> --cp_noexcept_operand;
> @@ -2497,11 +2487,8 @@ is_nothrow_xible (enum tree_code code, tree to, tree
> from,
> return false;
>
> bool is_noexcept = expr_noexcept_p (expr, tf_none);
> - if (explain && errs == (errorcount + sorrycount))
> - {
> - gcc_assert (!is_noexcept);
> - explain_not_noexcept (expr);
> - }
> + if (explain && !is_noexcept)
> + explain_not_noexcept (expr);
> return is_noexcept;
> }
>
> @@ -2601,12 +2588,10 @@ is_nothrow_convertible (tree from, tree to, bool
> explain/*=false*/)
> tree expr = is_convertible_helper (from, to, explain);
> if (expr == NULL_TREE || expr == error_mark_node)
> return false;
> +
> bool is_noexcept = expr_noexcept_p (expr, tf_none);
> - if (explain)
> - {
> - gcc_assert (!is_noexcept);
> - explain_not_noexcept (expr);
> - }
> + if (explain && !is_noexcept)
> + explain_not_noexcept (expr);
> return is_noexcept;
> }
>
> diff --git a/gcc/testsuite/g++.dg/ext/is_invocable7.C
> b/gcc/testsuite/g++.dg/ext/is_invocable7.C
> new file mode 100644
> index 00000000000..5c852fc8428
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/ext/is_invocable7.C
> @@ -0,0 +1,21 @@
> +// PR c++/121291
> +// { dg-do compile { target c++17 } }
> +
> +template <typename T>
> +constexpr bool is_invocable = __is_invocable(T);
> +
> +template <typename T>
> +constexpr bool is_nothrow_invocable = __is_nothrow_invocable(T);
> +
> +struct S {
> +private:
> + int operator()() noexcept; // { dg-message "here" }
> +};
> +
> +static_assert(is_invocable<S>); // { dg-error "assert" }
> +// { dg-message "not invocable" "" { target *-*-* } .-1 }
> +// { dg-error "private within this context" "" { target *-*-* } .-2 }
> +
> +static_assert(is_nothrow_invocable<S>); // { dg-error "assert" }
> +// { dg-message "not nothrow invocable" "" { target *-*-* } .-1 }
> +// { dg-error "private within this context" "" { target *-*-* } .-2 }
> diff --git a/gcc/testsuite/g++.dg/ext/is_nothrow_convertible5.C
> b/gcc/testsuite/g++.dg/ext/is_nothrow_convertible5.C
> new file mode 100644
> index 00000000000..0ce8fb82191
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/ext/is_nothrow_convertible5.C
> @@ -0,0 +1,15 @@
> +// PR c++/121291
> +// { dg-do compile { target c++17 } }
> +
> +template <typename T, typename U>
> +constexpr bool is_nothrow_convertible = __is_nothrow_convertible(T, U);
> +
> +struct A {};
> +struct B {
> +private:
> + operator A() noexcept; // { dg-message "here" }
> +};
> +
> +static_assert(is_nothrow_convertible<B, A>); // { dg-error "assert" }
> +// { dg-message "not nothrow convertible" "" { target *-*-* } .-1 }
> +// { dg-error "private within this context" "" { target *-*-* } .-2 }
> --
> 2.47.0
>
>
