From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pj1-x1035.google.com (mail-pj1-x1035.google.com [IPv6:2607:f8b0:4864:20::1035]) by sourceware.org (Postfix) with ESMTPS id 83D723857020 for ; Sat, 17 Jul 2021 19:19:21 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 83D723857020 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=adacore.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=adacore.com Received: by mail-pj1-x1035.google.com with SMTP id g24so8725997pji.4 for ; Sat, 17 Jul 2021 12:19:21 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=adacore-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=0xLrnyIx7iPvBYdFBAGQwo42v3o0cAx4ia76CvvQiM0=; b=ABWNl5g1wrlVUdOfwnB3ujLS9Si4AaZ3RLkAAKnOeic39m43I741EgxGwB3w76qsNV VfOba1ji7Tr/j7M6S5LUZtA6xEyunyLFV6rUaW8eJUlIPbvuEXzMUDwjrrca3ZZmL5+U eOJgeiTal3om/SLuAdcwur1i3Fj9TDFi6D8U6ZQcG7IgpDUwLskxPAka5QO3s6YYq4Ou zy445VRhsAQ/+yKS+y3MFnzFw11Tby6MauDbMzy0LZ0d4F3l6otl3OfsI4BLIooCrNHO MwG44hEJzULuC1jcOlUczo/RPF+YuUbg7WGJ1wUxAYKfPyd3Dzj95QAfkT6HfNxFNwrr m/Kw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=0xLrnyIx7iPvBYdFBAGQwo42v3o0cAx4ia76CvvQiM0=; b=VtenUeXys2gdnVeactmtBwhvJ6YamiPRNBxSiohDG/REWVn10AK0y2LoBOocVviQIf TWKUjwrBnxUofIIRC7OfKwAHRzwxk8AZkzpcQoYpujBfRVxUs+5hEHIlRuL1hMBriDTp CF1r03O6KsbSSJ/6cC6/Dygl+FmKxlqOncnBXbE4lNOjq6hB+yRlXMQjYqsG9aAtG21q vyf1xueC5cJhJxyTvjCYhnrjzJ4OuXHMQkGdzy51gtlKRvfIKC6tTVVoKBntD/+VgrKX gA9Skp6td9bYZhZ7jiKTaOjweaQD8KW/e93LIAxFhF4qkEpeMkBSoYAPChr+Nq1SL2t0 5DxA== X-Gm-Message-State: AOAM532DlgjRpCete++K20iVy8KXS2M9/0jNO3Y9972oUi/enCxUwIOZ KpKEonoV3OTsJaaJB0wWnwXd X-Google-Smtp-Source: ABdhPJzPjxyg9pul9McWz0iJGOPXbardfiblwW8Vyk89Fg7KreCpPdqkiLL1RG/3+bWQxR9FfDoSIA== X-Received: by 2002:a17:902:710a:b029:12b:4e80:add0 with SMTP id a10-20020a170902710ab029012b4e80add0mr12887365pll.1.1626549560547; Sat, 17 Jul 2021 12:19:20 -0700 (PDT) Received: from takamaka.home ([184.69.131.86]) by smtp.gmail.com with ESMTPSA id g6sm14581303pfi.108.2021.07.17.12.19.18 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 17 Jul 2021 12:19:19 -0700 (PDT) Received: by takamaka.home (Postfix, from userid 1000) id DE7FE89CFD; Sat, 17 Jul 2021 12:19:17 -0700 (PDT) Date: Sat, 17 Jul 2021 12:19:17 -0700 From: Joel Brobecker To: Hannes Domani via Gdb-patches Cc: Joel Brobecker Subject: Re: [PATCHv2 2/2] Use method children instead of to_string in pretty printers Message-ID: <20210717191917.GB1448640@adacore.com> References: <20210604162400.3255-1-ssbssa@yahoo.de> <20210604162400.3255-2-ssbssa@yahoo.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210604162400.3255-2-ssbssa@yahoo.de> X-Spam-Status: No, score=-9.7 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 17 Jul 2021 19:19:23 -0000 Hi Hannes, On Fri, Jun 04, 2021 at 06:24:00PM +0200, Hannes Domani via Gdb-patches wrote: > With '-pretty on' the output is basically the same, but like this the > pointer members can be expanded in the TUI variable windows. > > gdb/ChangeLog: > > 2021-06-04 Hannes Domani > > * gdb-gdb.py.in: Use method children in pretty printers. It looks like no one has reviewed this patch, yet, so I took a look. Overall, this looks good to me, except for one area: > --- > v2: > - Added ChangeLog. > - Reformatted with black. > --- > gdb/gdb-gdb.py.in | 175 ++++++++++++++++++++-------------------------- > 1 file changed, 77 insertions(+), 98 deletions(-) > > diff --git a/gdb/gdb-gdb.py.in b/gdb/gdb-gdb.py.in > index af9fcfedc2f..1432d7bbcf8 100644 > --- a/gdb/gdb-gdb.py.in > +++ b/gdb/gdb-gdb.py.in > @@ -107,17 +107,13 @@ class StructTypePrettyPrinter: > def __init__(self, val): > self.val = val > > - def to_string(self): > - fields = [] > - fields.append("pointer_type = %s" % self.val["pointer_type"]) > - fields.append("reference_type = %s" % self.val["reference_type"]) > - fields.append("chain = %s" % self.val["reference_type"]) > - fields.append( > - "instance_flags = %s" % TypeFlagsPrinter(self.val["m_instance_flags"]) > - ) > - fields.append("length = %d" % self.val["length"]) > - fields.append("main_type = %s" % self.val["main_type"]) > - return "\n{" + ",\n ".join(fields) + "}" > + def children(self): > + for f in self.val.type.fields(): > + name = f.name > + val = self.val[f] > + if name == "m_instance_flags": > + val = str(TypeFlagsPrinter(val)) > + yield name, val > > > class StructMainTypePrettyPrinter: > @@ -142,46 +138,6 @@ class StructMainTypePrettyPrinter: > ] > return "|".join(fields) > > - def owner_to_string(self): > - """Return an image of component "owner".""" > - if self.val["m_flag_objfile_owned"] != 0: > - return "%s (objfile)" % self.val["m_owner"]["objfile"] > - else: > - return "%s (gdbarch)" % self.val["m_owner"]["gdbarch"] > - > - def struct_field_location_img(self, field_val): > - """Return an image of the loc component inside the given field > - gdb.Value. > - """ > - loc_val = field_val["loc"] > - loc_kind = str(field_val["loc_kind"]) > - if loc_kind == "FIELD_LOC_KIND_BITPOS": > - return "bitpos = %d" % loc_val["bitpos"] > - elif loc_kind == "FIELD_LOC_KIND_ENUMVAL": > - return "enumval = %d" % loc_val["enumval"] > - elif loc_kind == "FIELD_LOC_KIND_PHYSADDR": > - return "physaddr = 0x%x" % loc_val["physaddr"] > - elif loc_kind == "FIELD_LOC_KIND_PHYSNAME": > - return "physname = %s" % loc_val["physname"] > - elif loc_kind == "FIELD_LOC_KIND_DWARF_BLOCK": > - return "dwarf_block = %s" % loc_val["dwarf_block"] > - else: > - return "loc = ??? (unsupported loc_kind value)" > - > - def struct_field_img(self, fieldno): > - """Return an image of the main_type field number FIELDNO.""" > - f = self.val["flds_bnds"]["fields"][fieldno] > - label = "flds_bnds.fields[%d]:" % fieldno > - if f["artificial"]: > - label += " (artificial)" > - fields = [] > - fields.append("name = %s" % f["name"]) > - fields.append("type = %s" % f["m_type"]) > - fields.append("loc_kind = %s" % f["loc_kind"]) > - fields.append("bitsize = %d" % f["bitsize"]) > - fields.append(self.struct_field_location_img(f)) > - return label + "\n" + " {" + ",\n ".join(fields) + "}" > - > def bound_img(self, bound_name): > """Return an image of the given main_type's bound.""" > bounds = self.val["flds_bnds"]["bounds"].dereference() > @@ -197,17 +153,6 @@ class StructMainTypePrettyPrinter: > info.append("upper_bound_is_count") > return "{} ({})".format(str(b["m_data"]["baton"]), ",".join(info)) > > - def bounds_img(self): > - """Return an image of the main_type bounds.""" > - b = self.val["flds_bnds"]["bounds"].dereference() > - low = self.bound_img("low") > - high = self.bound_img("high") > - > - img = "flds_bnds.bounds = {%s, %s}" % (low, high) > - if b["flag_bound_evaluated"]: > - img += " [evaluated]" > - return img > - > def type_specific_img(self): > """Return a string image of the main_type type_specific union. > Only the relevant component of that union is printed (based on > @@ -216,54 +161,86 @@ class StructMainTypePrettyPrinter: > type_specific_kind = str(self.val["type_specific_field"]) > type_specific = self.val["type_specific"] > if type_specific_kind == "TYPE_SPECIFIC_NONE": > - img = "type_specific_field = %s" % type_specific_kind > + return "type_specific_field", type_specific_kind > elif type_specific_kind == "TYPE_SPECIFIC_CPLUS_STUFF": > - img = "cplus_stuff = %s" % type_specific["cplus_stuff"] > + return "cplus_stuff", type_specific["cplus_stuff"] > elif type_specific_kind == "TYPE_SPECIFIC_GNAT_STUFF": > - img = ( > - "gnat_stuff = {descriptive_type = %s}" > - % type_specific["gnat_stuff"]["descriptive_type"] > + return ( > + "gnat_stuff.descriptive_type", > + type_specific["gnat_stuff"]["descriptive_type"], > ) > elif type_specific_kind == "TYPE_SPECIFIC_FLOATFORMAT": > - img = "floatformat[0..1] = %s" % type_specific["floatformat"] > + return "floatformat[0..1]", type_specific["floatformat"] > elif type_specific_kind == "TYPE_SPECIFIC_FUNC": > - img = ( > - "calling_convention = %d" > - % type_specific["func_stuff"]["calling_convention"] > + return ( > + "calling_convention", > + type_specific["func_stuff"]["calling_convention"], > ) > # tail_call_list is not printed. > elif type_specific_kind == "TYPE_SPECIFIC_SELF_TYPE": > - img = "self_type = %s" % type_specific["self_type"] > - elif type_specific_kind == "TYPE_SPECIFIC_FIXED_POINT": > - # The scaling factor is an opaque structure, so we cannot > - # decode its value from Python (not without insider knowledge). > - img = ( > - "scaling_factor: (call __gmpz_dump with " > - " _mp_num and _mp_den fields if needed)" > - ) > - else: > - img = ( > - "type_specific = ??? (unknown type_secific_kind: %s)" > - % type_specific_kind > - ) > - return img > + return "self_type", type_specific["self_type"] > + return ( > + "type_specific", > + "??? (unknown type_secific_kind: %s)" % type_specific_kind, > + ) It looks to me like you collapsed the case of TYPE_SPECIFIC_FIXED_POINT with the "else:" part. While we are unable to print the actual data in both cases, I still think it's worth showing preserving the current behavior, because it confirms the type-specific kind, and also shows to the user that the value is opaque, with a tipe on how to get it in practice. The rest looked good to me. > > - def to_string(self): > - """Return a pretty-printed image of our main_type.""" > - fields = [] > - fields.append("name = %s" % self.val["name"]) > - fields.append("code = %s" % self.val["code"]) > - fields.append("flags = [%s]" % self.flags_to_string()) > - fields.append("owner = %s" % self.owner_to_string()) > - fields.append("target_type = %s" % self.val["target_type"]) > + def children(self): > + yield "name", self.val["name"].format_string(symbols=False, address=False) > + yield "code", self.val["code"] > + yield "flags", "[%s]" % self.flags_to_string() > + if self.val["m_flag_objfile_owned"] != 0: > + yield "owner.objfile", self.val["m_owner"]["objfile"] > + else: > + yield "owner.gdbarch", self.val["m_owner"]["gdbarch"] > + yield "target_type", self.val["target_type"] > if self.val["nfields"] > 0: > - for fieldno in range(self.val["nfields"]): > - fields.append(self.struct_field_img(fieldno)) > + fields = self.val["flds_bnds"]["fields"] > + field_array_type = ( > + fields.type.target().array(self.val["nfields"] - 1).pointer() > + ) > + yield "flds_bnds.fields", fields.cast(field_array_type).dereference() > if self.val["code"] == gdb.TYPE_CODE_RANGE: > - fields.append(self.bounds_img()) > - fields.append(self.type_specific_img()) > + b = self.val["flds_bnds"]["bounds"].dereference() > + low = self.bound_img("low") > + high = self.bound_img("high") > + > + img = "{%s, %s}" % (low, high) > + if b["flag_bound_evaluated"]: > + img += " [evaluated]" > + yield "flds_bnds.bounds", img > + yield self.type_specific_img() > + > + > +class StructFieldPrettyPrinter: > + """Pretty-print an objet of type field""" > + > + def __init__(self, val): > + self.val = val > + > + def children(self): > + if self.val["artificial"]: > + yield "artificial", self.val["artificial"] > + yield "name", self.val["name"].format_string(symbols=False, address=False) > + yield "type", self.val["m_type"] > + yield "loc_kind", self.val["loc_kind"] > + yield "bitsize", self.val["bitsize"] > > - return "\n{" + ",\n ".join(fields) + "}" > + loc_val = self.val["loc"] > + loc_kind = str(self.val["loc_kind"]) > + if loc_kind == "FIELD_LOC_KIND_BITPOS": > + yield "bitpos", loc_val["bitpos"] > + elif loc_kind == "FIELD_LOC_KIND_ENUMVAL": > + yield "enumval", loc_val["enumval"] > + elif loc_kind == "FIELD_LOC_KIND_PHYSADDR": > + yield "physaddr", loc_val["physaddr"] > + elif loc_kind == "FIELD_LOC_KIND_PHYSNAME": > + yield "physname", loc_val["physname"].format_string( > + symbols=False, address=False > + ) > + elif loc_kind == "FIELD_LOC_KIND_DWARF_BLOCK": > + yield "dwarf_block", loc_val["dwarf_block"] > + else: > + yield "loc", "???" > > > class CoreAddrPrettyPrinter: > @@ -284,6 +261,8 @@ def type_lookup_function(val): > return StructTypePrettyPrinter(val) > elif val.type.tag == "main_type": > return StructMainTypePrettyPrinter(val) > + elif val.type.tag == "field": > + return StructFieldPrettyPrinter(val) > elif val.type.name == "CORE_ADDR": > return CoreAddrPrettyPrinter(val) > return None > -- > 2.31.1 > -- Joel