From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id DA25E3858D1E for ; Mon, 20 Feb 2023 15:28:28 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org DA25E3858D1E Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1676906908; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=Ml5U7i44OGTrkNNK3l47AlXGuGug5W3TVLkturyblPE=; b=gjrXXql2TM/fqk8YFBbFQjPwoPEgGUJRk4wEwJ3YJYxp1YLX4WtRb4DKNR6Jfsn5QwPyCd sDy9tsFwqGjxWmuAvXqScwA7tCVi4ScK0irxNwvS/kDpv5JAD22gmvKfHwZarSHi1tq7iO aXK7s0Yde592+lU5iIinOUCnI1mzbWo= Received: from mail-wm1-f70.google.com (mail-wm1-f70.google.com [209.85.128.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-230-S11BwbXuOQawrnyP0tHO8w-1; Mon, 20 Feb 2023 10:28:25 -0500 X-MC-Unique: S11BwbXuOQawrnyP0tHO8w-1 Received: by mail-wm1-f70.google.com with SMTP id e22-20020a05600c219600b003e000facbb1so625630wme.9 for ; Mon, 20 Feb 2023 07:28:25 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=mime-version:message-id:date:references:in-reply-to:subject:cc:to :from:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=Ml5U7i44OGTrkNNK3l47AlXGuGug5W3TVLkturyblPE=; b=l/vcFKgyCTDSFgL56Ou9JLQWNH3zMf+/I33/k3L+agjdVpxQlcspZrHSxpok9t/CYC YZCZIUz927kmQ3iFRG9RqXWd9rf68o7Vb9jw74bseIU/kxZBbowGP7JJ2XdH2thmlkDU UGlq+yOanIe19PkUBF0x9nqhr4NDQOKuBu8b50HETTAyYLpekKXjwoSugN8gN/e8K0Rg LsGnedG5yv4Kzoxny8gukT777k4fylHolQJXofH/aY7AwKCkOJrG5jQ0Kjq9dgndBPSP 9BKDbNHtRyhV7qwqeTU3Lan0sHt8rZl6Ky0f7d3SHyQ2bmXzg3I9NP76H3KzTYwTadq6 xSAw== X-Gm-Message-State: AO0yUKWiFBBPeLzFwq/js+FlEs977fX/jKNVS+pUReGdhPY9zGzRznyH 3Rz81sv3BCnMymWgjN5K8IksVuzUzMCwgftK3828aCr9CbqcSt2bJYkSq3Pi3em9ZJS8W/wxP5w YC9leBXTwakGwvJOPNzSKpBfeK00= X-Received: by 2002:adf:ef47:0:b0:2c6:5a4e:ea23 with SMTP id c7-20020adfef47000000b002c65a4eea23mr1275160wrp.61.1676906903852; Mon, 20 Feb 2023 07:28:23 -0800 (PST) X-Google-Smtp-Source: AK7set9mRKWz0GkOkP4dLREpMmK75CkkYBtXuFiiW6UCrEdicv2UA/Cq1nMVIsivDqg9jLhrGOvnQQ== X-Received: by 2002:adf:ef47:0:b0:2c6:5a4e:ea23 with SMTP id c7-20020adfef47000000b002c65a4eea23mr1275144wrp.61.1676906903530; Mon, 20 Feb 2023 07:28:23 -0800 (PST) Received: from localhost (95.72.115.87.dyn.plus.net. [87.115.72.95]) by smtp.gmail.com with ESMTPSA id a18-20020adfe5d2000000b002be505ab59asm2873573wrn.97.2023.02.20.07.28.22 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 20 Feb 2023 07:28:23 -0800 (PST) From: Andrew Burgess To: Pedro Alves , Tom Tromey Cc: Andrew Burgess via Gdb-patches 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) In-Reply-To: <1ee2dd51-6bcc-a917-ea52-9c1e9a7b9b29@palves.net> References: <20230210233604.2228450-1-pedro@palves.net> <20230210233604.2228450-3-pedro@palves.net> <878rh1tuyt.fsf@redhat.com> <87y1p0tgcb.fsf@tromey.com> <0165619c-2905-d164-954b-8a0237d1072b@palves.net> <87pmaarhoo.fsf@tromey.com> <1ee2dd51-6bcc-a917-ea52-9c1e9a7b9b29@palves.net> Date: Mon, 20 Feb 2023 15:28:22 +0000 Message-ID: <871qmkl5k9.fsf@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain X-Spam-Status: No, score=-11.6 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Pedro Alves 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 > 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 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