From: Andrew Burgess <aburgess@redhat.com>
To: Pedro Alves <pedro@palves.net>, Tom Tromey <tom@tromey.com>
Cc: Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org>
Subject: Re: [PATCH] Move TYPE_CODE_INTERNAL_FUNCTION type printing to common code (was: Re: [PATCH 2/6] Make "ptype INTERNAL_FUNCTION" in Ada print like other languages)
Date: Mon, 20 Feb 2023 15:28:22 +0000 [thread overview]
Message-ID: <871qmkl5k9.fsf@redhat.com> (raw)
In-Reply-To: <1ee2dd51-6bcc-a917-ea52-9c1e9a7b9b29@palves.net>
Pedro Alves <pedro@palves.net> writes:
> On 2023-02-15 4:56 p.m., Tom Tromey wrote:
>>>> I think this would be better.
>>
>> Pedro> My point with adding this check early is that these functions' type never
>> Pedro> has anything to do with Ada, so all that code at the beginning of ada_print_type,
>> Pedro> like decoded_type_name, ada_is_aligner_type, ada_is_constrained_packed_array_type,
>> Pedro> etc. is always a nop for internal functions, so might as well skip it all.
>>
>> I see. That makes sense to me, thanks.
>>
>> Sorry about the noise, as far as I'm concerned you can land either
>> version of the patch.
>
> No problem. I've merged the original version, along with the whole series.
>
>>
>> Pedro> I actually started out by considering moving TYPE_CODE_INTERNAL_FUNCTION printing
>> Pedro> to common code (say, rename the virtual language_defn::print_type to do_print_type,
>> Pedro> and add a new non-virtual wrapper language_defn::print_type method and print
>> Pedro> TYPE_CODE_INTERNAL_FUNCTION there), and I didn't pursue that as I couldn't really convince
>> Pedro> myself that a different language might want to print it differently (say, some other
>> Pedro> characters instead of "<>")
>>
>> I think this is something we can simply force on languages. The type of
>> these things isn't a language feature and probably isn't worth
>> customizing anyway.
>>
>> More generally I think it's better for gdb to try to do more in the
>> common code and leave less to the languages.
>
> I implemented the idea I mentioned above.
>
> WDYT?
>
> From 73958ac0ee47fd1a887515a47436cb3835514724 Mon Sep 17 00:00:00 2001
> From: Pedro Alves <pedro@palves.net>
> Date: Wed, 15 Feb 2023 20:19:10 +0000
> Subject: [PATCH] Move TYPE_CODE_INTERNAL_FUNCTION type printing to common code
>
> Internal functions should print the same for all languages. It is
> currently left to each language to make sure that is true. This patch
> centralizes the logic.
>
> Rename language_defn::print_type to language_defn::do_print_type and
> making it protected, and then add a new public non-virtual
> language_defn::print_type method that implements
> TYPE_CODE_INTERNAL_FUNCTION type printing before dispatching to the
> virtual language_defn::do_print_type.
Looks like a good change to me.
Reviewed-By: Andrew Burgess <aburgess@redhat.com>
Thanks,
Andrew
>
> Change-Id: Idd5fbb437cc91990b051e1a9a027c3909c09dd31
> ---
> gdb/ada-lang.c | 6 +++---
> gdb/ada-typeprint.c | 7 -------
> gdb/c-lang.c | 24 ++++++++++++------------
> gdb/d-lang.c | 6 +++---
> gdb/f-lang.h | 6 +++---
> gdb/f-typeprint.c | 6 +++---
> gdb/go-lang.h | 6 +++---
> gdb/go-typeprint.c | 6 +++---
> gdb/language.c | 25 ++++++++++++++++++++++---
> gdb/language.h | 21 +++++++++++++++++----
> gdb/m2-lang.h | 6 +++---
> gdb/objc-lang.c | 6 +++---
> gdb/opencl-lang.c | 6 +++---
> gdb/p-lang.h | 6 +++---
> gdb/p-typeprint.c | 6 +++---
> gdb/rust-lang.c | 6 +++---
> gdb/rust-lang.h | 6 +++---
> 17 files changed, 90 insertions(+), 65 deletions(-)
>
> diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
> index ec85729042f..e3bbfdeae73 100644
> --- a/gdb/ada-lang.c
> +++ b/gdb/ada-lang.c
> @@ -13625,9 +13625,9 @@ class ada_language : public language_defn
>
> /* See language.h. */
>
> - void print_type (struct type *type, const char *varstring,
> - struct ui_file *stream, int show, int level,
> - const struct type_print_options *flags) const override
> + void do_print_type (struct type *type, const char *varstring,
> + struct ui_file *stream, int show, int level,
> + const struct type_print_options *flags) const override
> {
> ada_print_type (type, varstring, stream, show, level, flags);
> }
> diff --git a/gdb/ada-typeprint.c b/gdb/ada-typeprint.c
> index 3866b2d35eb..e95034c9285 100644
> --- a/gdb/ada-typeprint.c
> +++ b/gdb/ada-typeprint.c
> @@ -941,13 +941,6 @@ ada_print_type (struct type *type0, const char *varstring,
> struct ui_file *stream, int show, int level,
> const struct type_print_options *flags)
> {
> - if (type0->code () == TYPE_CODE_INTERNAL_FUNCTION)
> - {
> - c_print_type (type0, "", stream, show, level,
> - language_ada, flags);
> - return;
> - }
> -
> struct type *type = ada_check_typedef (ada_get_base_type (type0));
> /* If we can decode the original type name, use it. However, there
> are cases where the original type is an internally-generated type
> diff --git a/gdb/c-lang.c b/gdb/c-lang.c
> index 6535ab93498..9832d676445 100644
> --- a/gdb/c-lang.c
> +++ b/gdb/c-lang.c
> @@ -836,9 +836,9 @@ class c_language : public language_defn
>
> /* See language.h. */
>
> - void print_type (struct type *type, const char *varstring,
> - struct ui_file *stream, int show, int level,
> - const struct type_print_options *flags) const override
> + void do_print_type (struct type *type, const char *varstring,
> + struct ui_file *stream, int show, int level,
> + const struct type_print_options *flags) const override
> {
> c_print_type (type, varstring, stream, show, level, la_language, flags);
> }
> @@ -993,9 +993,9 @@ class cplus_language : public language_defn
>
> /* See language.h. */
>
> - void print_type (struct type *type, const char *varstring,
> - struct ui_file *stream, int show, int level,
> - const struct type_print_options *flags) const override
> + void do_print_type (struct type *type, const char *varstring,
> + struct ui_file *stream, int show, int level,
> + const struct type_print_options *flags) const override
> {
> c_print_type (type, varstring, stream, show, level, la_language, flags);
> }
> @@ -1100,9 +1100,9 @@ class asm_language : public language_defn
>
> /* See language.h. */
>
> - void print_type (struct type *type, const char *varstring,
> - struct ui_file *stream, int show, int level,
> - const struct type_print_options *flags) const override
> + void do_print_type (struct type *type, const char *varstring,
> + struct ui_file *stream, int show, int level,
> + const struct type_print_options *flags) const override
> {
> c_print_type (type, varstring, stream, show, level, la_language, flags);
> }
> @@ -1159,9 +1159,9 @@ class minimal_language : public language_defn
>
> /* See language.h. */
>
> - void print_type (struct type *type, const char *varstring,
> - struct ui_file *stream, int show, int level,
> - const struct type_print_options *flags) const override
> + void do_print_type (struct type *type, const char *varstring,
> + struct ui_file *stream, int show, int level,
> + const struct type_print_options *flags) const override
> {
> c_print_type (type, varstring, stream, show, level, la_language, flags);
> }
> diff --git a/gdb/d-lang.c b/gdb/d-lang.c
> index 8286c5be646..a374fcfeb8f 100644
> --- a/gdb/d-lang.c
> +++ b/gdb/d-lang.c
> @@ -151,9 +151,9 @@ class d_language : public language_defn
>
> /* See language.h. */
>
> - void print_type (struct type *type, const char *varstring,
> - struct ui_file *stream, int show, int level,
> - const struct type_print_options *flags) const override
> + void do_print_type (struct type *type, const char *varstring,
> + struct ui_file *stream, int show, int level,
> + const struct type_print_options *flags) const override
> {
> c_print_type (type, varstring, stream, show, level, la_language, flags);
> }
> diff --git a/gdb/f-lang.h b/gdb/f-lang.h
> index 673e273d31a..7c44a46b981 100644
> --- a/gdb/f-lang.h
> +++ b/gdb/f-lang.h
> @@ -87,9 +87,9 @@ class f_language : public language_defn
>
> /* See language.h. */
>
> - void print_type (struct type *type, const char *varstring,
> - struct ui_file *stream, int show, int level,
> - const struct type_print_options *flags) const override;
> + void do_print_type (struct type *type, const char *varstring,
> + struct ui_file *stream, int show, int level,
> + const struct type_print_options *flags) const override;
>
> /* See language.h. This just returns default set of word break
> characters but with the modules separator `::' removed. */
> diff --git a/gdb/f-typeprint.c b/gdb/f-typeprint.c
> index e4aed6e5904..b3c675e2c2a 100644
> --- a/gdb/f-typeprint.c
> +++ b/gdb/f-typeprint.c
> @@ -46,9 +46,9 @@ f_language::print_typedef (struct type *type, struct symbol *new_symbol,
> /* See f-lang.h. */
>
> void
> -f_language::print_type (struct type *type, const char *varstring,
> - struct ui_file *stream, int show, int level,
> - const struct type_print_options *flags) const
> +f_language::do_print_type (struct type *type, const char *varstring,
> + struct ui_file *stream, int show, int level,
> + const struct type_print_options *flags) const
> {
> enum type_code code;
>
> diff --git a/gdb/go-lang.h b/gdb/go-lang.h
> index 1820b4c9658..df607d053f9 100644
> --- a/gdb/go-lang.h
> +++ b/gdb/go-lang.h
> @@ -110,9 +110,9 @@ class go_language : public language_defn
>
> /* See language.h. */
>
> - void print_type (struct type *type, const char *varstring,
> - struct ui_file *stream, int show, int level,
> - const struct type_print_options *flags) const override;
> + void do_print_type (struct type *type, const char *varstring,
> + struct ui_file *stream, int show, int level,
> + const struct type_print_options *flags) const override;
>
> /* See language.h. */
>
> diff --git a/gdb/go-typeprint.c b/gdb/go-typeprint.c
> index 0c4e9a26563..c664aaa8284 100644
> --- a/gdb/go-typeprint.c
> +++ b/gdb/go-typeprint.c
> @@ -42,9 +42,9 @@
> LEVEL indicates level of recursion (for nested definitions). */
>
> void
> -go_language::print_type (struct type *type, const char *varstring,
> - struct ui_file *stream, int show, int level,
> - const struct type_print_options *flags) const
> +go_language::do_print_type (struct type *type, const char *varstring,
> + struct ui_file *stream, int show, int level,
> + const struct type_print_options *flags) const
> {
> /* Borrowed from c-typeprint.c. */
> if (show > 0)
> diff --git a/gdb/language.c b/gdb/language.c
> index 50a53c647f5..23601c42ad6 100644
> --- a/gdb/language.c
> +++ b/gdb/language.c
> @@ -734,9 +734,9 @@ class auto_or_unknown_language : public language_defn
>
> /* See language.h. */
>
> - void print_type (struct type *type, const char *varstring,
> - struct ui_file *stream, int show, int level,
> - const struct type_print_options *flags) const override
> + void do_print_type (struct type *type, const char *varstring,
> + struct ui_file *stream, int show, int level,
> + const struct type_print_options *flags) const override
> {
> error (_("type printing not implemented for language \"%s\""),
> natural_name ());
> @@ -1094,6 +1094,25 @@ language_lookup_primitive_type_as_symbol (const struct language_defn *la,
> return sym;
> }
>
> +/* See language.h. */
> +
> +void
> +language_defn::print_type (struct type *type, const char *varstring,
> + struct ui_file *stream, int show, int level,
> + const struct type_print_options *flags) const
> +{
> + /* Print things here which should print the same for every language,
> + before dispatching to the virtual method. */
> + if (type->code () == TYPE_CODE_INTERNAL_FUNCTION)
> + {
> + c_print_type (type, varstring, stream, show, level,
> + la_language, flags);
> + return;
> + }
> +
> + do_print_type (type, varstring, stream, show, level, flags);
> +}
> +
> /* Initialize the language routines. */
>
> void _initialize_language ();
> diff --git a/gdb/language.h b/gdb/language.h
> index a51ddf97381..ebe3ffff00e 100644
> --- a/gdb/language.h
> +++ b/gdb/language.h
> @@ -461,11 +461,24 @@ struct language_defn
> /* Print TYPE to STREAM using syntax appropriate for this language.
> LEVEL is the depth to indent lines by. VARSTRING, if not NULL or the
> empty string, is the name of a variable and TYPE should be printed in
> - the form of a declaration of a variable named VARSTRING. */
> + the form of a declaration of a variable named VARSTRING.
>
> - virtual void print_type (struct type *type, const char *varstring,
> - struct ui_file *stream, int show, int level,
> - const struct type_print_options *flags) const = 0;
> + This is the public-facing method, which contains code that is
> + common to all languages, and then dispatches to the virtual
> + do_print_type method. */
> + void print_type (struct type *type, const char *varstring,
> + struct ui_file *stream, int show, int level,
> + const struct type_print_options *flags) const;
> +
> +protected:
> +
> + /* Implements actual language-specific parts of print_type.
> + Arguments and return are like the print_type method. */
> + virtual void do_print_type (struct type *type, const char *varstring,
> + struct ui_file *stream, int show, int level,
> + const struct type_print_options *flags) const = 0;
> +
> +public:
>
> /* PC is possibly an unknown languages trampoline.
> If that PC falls in a trampoline belonging to this language, return
> diff --git a/gdb/m2-lang.h b/gdb/m2-lang.h
> index cda6e241c8c..7d7556fabbd 100644
> --- a/gdb/m2-lang.h
> +++ b/gdb/m2-lang.h
> @@ -73,9 +73,9 @@ class m2_language : public language_defn
>
> /* See language.h. */
>
> - void print_type (struct type *type, const char *varstring,
> - struct ui_file *stream, int show, int level,
> - const struct type_print_options *flags) const override
> + void do_print_type (struct type *type, const char *varstring,
> + struct ui_file *stream, int show, int level,
> + const struct type_print_options *flags) const override
> {
> m2_print_type (type, varstring, stream, show, level, flags);
> }
> diff --git a/gdb/objc-lang.c b/gdb/objc-lang.c
> index f43d158a770..7964e6da8c5 100644
> --- a/gdb/objc-lang.c
> +++ b/gdb/objc-lang.c
> @@ -273,9 +273,9 @@ class objc_language : public language_defn
>
> /* See language.h. */
>
> - void print_type (struct type *type, const char *varstring,
> - struct ui_file *stream, int show, int level,
> - const struct type_print_options *flags) const override
> + void do_print_type (struct type *type, const char *varstring,
> + struct ui_file *stream, int show, int level,
> + const struct type_print_options *flags) const override
> {
> c_print_type (type, varstring, stream, show, level, la_language, flags);
> }
> diff --git a/gdb/opencl-lang.c b/gdb/opencl-lang.c
> index 3e4a9c360b2..14df2756360 100644
> --- a/gdb/opencl-lang.c
> +++ b/gdb/opencl-lang.c
> @@ -959,9 +959,9 @@ class opencl_language : public language_defn
>
> /* See language.h. */
>
> - void print_type (struct type *type, const char *varstring,
> - struct ui_file *stream, int show, int level,
> - const struct type_print_options *flags) const override
> + void do_print_type (struct type *type, const char *varstring,
> + struct ui_file *stream, int show, int level,
> + const struct type_print_options *flags) const override
> {
> /* We nearly always defer to C type printing, except that vector types
> are considered primitive in OpenCL, and should always be printed
> diff --git a/gdb/p-lang.h b/gdb/p-lang.h
> index 662079114ed..c16a4cd528e 100644
> --- a/gdb/p-lang.h
> +++ b/gdb/p-lang.h
> @@ -88,9 +88,9 @@ class pascal_language : public language_defn
>
> /* See language.h. */
>
> - void print_type (struct type *type, const char *varstring,
> - struct ui_file *stream, int show, int level,
> - const struct type_print_options *flags) const override;
> + void do_print_type (struct type *type, const char *varstring,
> + struct ui_file *stream, int show, int level,
> + const struct type_print_options *flags) const override;
>
> /* See language.h. */
>
> diff --git a/gdb/p-typeprint.c b/gdb/p-typeprint.c
> index 7458aa6c095..568ff61b607 100644
> --- a/gdb/p-typeprint.c
> +++ b/gdb/p-typeprint.c
> @@ -37,9 +37,9 @@
> /* See language.h. */
>
> void
> -pascal_language::print_type (struct type *type, const char *varstring,
> - struct ui_file *stream, int show, int level,
> - const struct type_print_options *flags) const
> +pascal_language::do_print_type (struct type *type, const char *varstring,
> + struct ui_file *stream, int show, int level,
> + const struct type_print_options *flags) const
> {
> enum type_code code;
> int demangled_args;
> diff --git a/gdb/rust-lang.c b/gdb/rust-lang.c
> index fb9db9fe31b..8424736e045 100644
> --- a/gdb/rust-lang.c
> +++ b/gdb/rust-lang.c
> @@ -1626,9 +1626,9 @@ rust_language::language_arch_info (struct gdbarch *gdbarch,
> /* See language.h. */
>
> void
> -rust_language::print_type (struct type *type, const char *varstring,
> - struct ui_file *stream, int show, int level,
> - const struct type_print_options *flags) const
> +rust_language::do_print_type (struct type *type, const char *varstring,
> + struct ui_file *stream, int show, int level,
> + const struct type_print_options *flags) const
> {
> print_offset_data podata (flags);
> rust_internal_print_type (type, varstring, stream, show, level,
> diff --git a/gdb/rust-lang.h b/gdb/rust-lang.h
> index 89e03550fb7..ef95fbd3f3d 100644
> --- a/gdb/rust-lang.h
> +++ b/gdb/rust-lang.h
> @@ -114,9 +114,9 @@ class rust_language : public language_defn
>
> /* See language.h. */
>
> - void print_type (struct type *type, const char *varstring,
> - struct ui_file *stream, int show, int level,
> - const struct type_print_options *flags) const override;
> + void do_print_type (struct type *type, const char *varstring,
> + struct ui_file *stream, int show, int level,
> + const struct type_print_options *flags) const override;
>
> /* See language.h. */
>
>
> base-commit: 141cd158423a5ee248245fb2075fd2e5a580cff2
> --
> 2.36.0
next prev parent reply other threads:[~2023-02-20 15:28 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-02-10 23:35 [PATCH 0/6] Don't throw quit while handling inferior events Pedro Alves
2023-02-10 23:35 ` [PATCH 1/6] Fix "ptype INTERNAL_FUNC" (PR gdb/30105) Pedro Alves
2023-02-13 16:02 ` Andrew Burgess
2023-02-14 15:26 ` Tom Tromey
2023-02-15 21:10 ` Pedro Alves
2023-02-15 22:04 ` Tom Tromey
2023-02-10 23:36 ` [PATCH 2/6] Make "ptype INTERNAL_FUNCTION" in Ada print like other languages Pedro Alves
2023-02-13 16:02 ` Andrew Burgess
2023-02-14 15:30 ` Tom Tromey
2023-02-15 13:38 ` Pedro Alves
2023-02-15 15:13 ` Pedro Alves
2023-02-15 16:56 ` Tom Tromey
2023-02-15 21:04 ` [PATCH] Move TYPE_CODE_INTERNAL_FUNCTION type printing to common code (was: Re: [PATCH 2/6] Make "ptype INTERNAL_FUNCTION" in Ada print like other languages) Pedro Alves
2023-02-20 15:28 ` Andrew Burgess [this message]
2023-02-10 23:36 ` [PATCH 3/6] Add new "$_shell(CMD)" internal function Pedro Alves
2023-02-11 8:02 ` Eli Zaretskii
2023-02-13 15:11 ` Pedro Alves
2023-02-13 15:36 ` Eli Zaretskii
2023-02-13 16:47 ` [PATCH] gdb/manual: Move @findex entries (was: Re: [PATCH 3/6] Add new "$_shell(CMD)" internal function) Pedro Alves
2023-02-13 17:00 ` Eli Zaretskii
2023-02-13 17:27 ` [PATCH 3/6] Add new "$_shell(CMD)" internal function Pedro Alves
2023-02-13 18:41 ` Eli Zaretskii
2023-02-14 15:38 ` Tom Tromey
2023-02-10 23:36 ` [PATCH 4/6] Don't throw quit while handling inferior events Pedro Alves
2023-02-14 15:50 ` Tom Tromey
2023-02-10 23:36 ` [PATCH 5/6] GC get_active_ext_lang Pedro Alves
2023-02-14 15:39 ` Tom Tromey
2023-02-10 23:36 ` [PATCH 6/6] Don't throw quit while handling inferior events, part II Pedro Alves
2023-02-14 15:54 ` Tom Tromey
2023-02-15 21:16 ` Pedro Alves
2023-02-15 21:24 ` Pedro Alves
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=871qmkl5k9.fsf@redhat.com \
--to=aburgess@redhat.com \
--cc=gdb-patches@sourceware.org \
--cc=pedro@palves.net \
--cc=tom@tromey.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).