From: Jason Merrill <jason@redhat.com>
To: Patrick Palka <ppalka@redhat.com>, gcc-patches@gcc.gnu.org
Subject: Re: [PATCH] c++: Fix pretty printing of function pointer type [PR98767]
Date: Mon, 19 Apr 2021 17:31:01 -0400 [thread overview]
Message-ID: <5d6a0776-5d4c-dfa0-d6bd-8621ce357837@redhat.com> (raw)
In-Reply-To: <20210416225830.3897124-1-ppalka@redhat.com>
On 4/16/21 6:58 PM, Patrick Palka wrote:
> When pretty printing a function pointer type via
> pp_cxx_parameter_declaration_clause, we end up always printing an empty
> parameter list because the loop that's supposed to print the parameter
> list iterates over 'args' instead of 'types', and 'args' is empty in
> this case when a FUNCTION_TYPE is passed to this routine (as opposed
> to a FUNCTION_DECL).
>
> This patch fixes this by making the loop iterator over 'types' instead.
> This patch also moves the retrofitted PARM_DECL printing from this
> routine to pp_cxx_requires_expr, the only caller that uses it. This
> simplification lets us easily output the trailing '...' in the parameter
> list of a variadic function, which this patch also implements.
>
> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
> trunk?
OK.
> gcc/cp/ChangeLog:
>
> PR c++/98767
> * cxx-pretty-print.c (pp_cxx_parameter_declaration_clause): Fix
> loop over parameter list to iterate over 'types' instead of
> 'args'. Output the trailing '...' for a variadic function.
> Remove PARM_DECL support.
> (pp_cxx_requires_expr): Pretty print the parameter list directly
> instead of going through pp_cxx_parameter_declaration_clause.
>
> gcc/testsuite/ChangeLog:
>
> PR c++/98767
> * g++.dg/concepts/diagnostic16.C: New test.
> ---
> gcc/cp/cxx-pretty-print.c | 48 ++++++++++++--------
> gcc/testsuite/g++.dg/concepts/diagnostic16.C | 17 +++++++
> 2 files changed, 46 insertions(+), 19 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/concepts/diagnostic16.C
>
> diff --git a/gcc/cp/cxx-pretty-print.c b/gcc/cp/cxx-pretty-print.c
> index a22eea5239c..894472e26e0 100644
> --- a/gcc/cp/cxx-pretty-print.c
> +++ b/gcc/cp/cxx-pretty-print.c
> @@ -1537,34 +1537,27 @@ pp_cxx_parameter_declaration (cxx_pretty_printer *pp, tree t)
> static void
> pp_cxx_parameter_declaration_clause (cxx_pretty_printer *pp, tree t)
> {
> - tree args;
> - tree types;
> - bool abstract;
> -
> - // For a requires clause or the explicit printing of a parameter list
> - // we expect T to be a chain of PARM_DECLs. Otherwise, the list of
> - // args and types are taken from the function decl T.
> - if (TREE_CODE (t) == PARM_DECL)
> + gcc_assert (FUNC_OR_METHOD_TYPE_P (t) || TREE_CODE (t) == FUNCTION_DECL);
> + tree types, args;
> + if (TYPE_P (t))
> {
> - args = t;
> - types = t;
> - abstract = false;
> + types = TYPE_ARG_TYPES (t);
> + args = NULL_TREE;
> }
> else
> {
> - bool type_p = TYPE_P (t);
> - args = type_p ? NULL : FUNCTION_FIRST_USER_PARM (t);
> - types = type_p ? TYPE_ARG_TYPES (t) : FUNCTION_FIRST_USER_PARMTYPE (t);
> - abstract = args == NULL || pp->flags & pp_c_flag_abstract;
> + types = FUNCTION_FIRST_USER_PARMTYPE (t);
> + args = FUNCTION_FIRST_USER_PARM (t);
> }
> - bool first = true;
> + bool abstract = !args || (pp->flags & pp_c_flag_abstract);
>
> /* Skip artificial parameter for non-static member functions. */
> if (TREE_CODE (t) == METHOD_TYPE)
> types = TREE_CHAIN (types);
>
> + bool first = true;
> pp_cxx_left_paren (pp);
> - for (; args; args = TREE_CHAIN (args), types = TREE_CHAIN (types))
> + for (; types && types != void_list_node; types = TREE_CHAIN (types))
> {
> if (!first)
> pp_cxx_separate_with (pp, ',');
> @@ -1577,6 +1570,14 @@ pp_cxx_parameter_declaration_clause (cxx_pretty_printer *pp, tree t)
> pp_cxx_whitespace (pp);
> pp->assignment_expression (TREE_PURPOSE (types));
> }
> + if (!abstract)
> + args = TREE_CHAIN (args);
> + }
> + if (!types)
> + {
> + if (!first)
> + pp_cxx_separate_with (pp, ',');
> + pp_cxx_ws_string (pp, "...");
> }
> pp_cxx_right_paren (pp);
> }
> @@ -2775,9 +2776,18 @@ void
> pp_cxx_requires_expr (cxx_pretty_printer *pp, tree t)
> {
> pp_string (pp, "requires");
> - if (tree parms = TREE_OPERAND (t, 0))
> + if (tree parms = REQUIRES_EXPR_PARMS (t))
> {
> - pp_cxx_parameter_declaration_clause (pp, parms);
> + bool first = true;
> + pp_cxx_left_paren (pp);
> + for (; parms; parms = TREE_CHAIN (parms))
> + {
> + if (!first)
> + pp_cxx_separate_with (pp, ',' );
> + first = false;
> + pp_cxx_parameter_declaration (pp, parms);
> + }
> + pp_cxx_right_paren (pp);
> pp_cxx_whitespace (pp);
> }
> pp_cxx_requirement_body (pp, TREE_OPERAND (t, 1));
> diff --git a/gcc/testsuite/g++.dg/concepts/diagnostic16.C b/gcc/testsuite/g++.dg/concepts/diagnostic16.C
> new file mode 100644
> index 00000000000..49d5733faea
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/concepts/diagnostic16.C
> @@ -0,0 +1,17 @@
> +// PR c++/98767
> +// { dg-do compile { target c++20 } }
> +
> +template <typename Function, typename... Args>
> +concept Callable = requires(Function func, Args... args) { func(args...); };
> +
> +static_assert(Callable<int(*)(), bool>); // { dg-error "failed" }
> +// { dg-message {Function = int \(\*\)\(\)} "" { target *-*-* } 5 }
> +
> +static_assert(Callable<char(*)(int*), bool>); // { dg-error "failed" }
> +// { dg-message {Function = char \(\*\)\(int\*\)} "" { target *-*-* } 5 }
> +
> +static_assert(Callable<short(*)(int*, int), bool>); // { dg-error "failed" }
> +// { dg-message {Function = short int \(\*\)\(int\*, int\)} "" { target *-*-* } 5 }
> +
> +static_assert(Callable<long(*)(int*, int, ...), bool>); // { dg-error "failed" }
> +// { dg-message {Function = long int \(\*\)\(int\*, int, \.\.\.\)} "" { target *-*-* } 5 }
>
prev parent reply other threads:[~2021-04-19 21:31 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-04-16 22:58 Patrick Palka
2021-04-19 21:31 ` Jason Merrill [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5d6a0776-5d4c-dfa0-d6bd-8621ce357837@redhat.com \
--to=jason@redhat.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=ppalka@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).