[PATCH 5/9] Add support for printing value of DWARF-based fixed-point type objects

Joel Brobecker brobecker@adacore.com
Sat Nov 14 10:48:29 GMT 2020


Hi Simon,

> > +   of the corresponding TYPE by setting its TYPE_FIXED_POINT_INFO.
> > +   CU is the DIE's CU.  */
> > +
> > +static void
> > +finish_fixed_point_type (struct type *type, struct die_info *die,
> > +			 struct dwarf2_cu *cu)
> 
> Unless there's a good reason not to (coming up in the following
> patches), I would make this function create the "struct type", instead
> of having the caller create it and pass it.  In other words, this
> signature:
> 
> struct type *create_fixed_point_type (die_info *die, dwarf2_cu *cu)
> 
> That just makes it more obvious that it's a simple "die" to "type"
> transform.

While implementing this suggestion, I'm realizing that doing so
requires either:

(1) Recomputing the following information for the CU: encoding,
    bitsize if any, and type name, which means:

        | attr = dwarf2_attr (die, DW_AT_encoding, cu);
        | if (attr != nullptr && attr->form_is_constant ())
        |   encoding = attr->constant_value (0);
        | attr = dwarf2_attr (die, DW_AT_byte_size, cu);
        | if (attr != nullptr)
        |   bits = attr->constant_value (0) * TARGET_CHAR_BIT;
        | name = dwarf2_name (die, cu);
        | if (!name)
        |   complaint (_("DW_AT_name missing from DW_TAG_base_type"));

    Or:

(2) Passing that information as arguments to the function.

I find option (1) really sad because of the code duplication.
And I find option (2) somewhat unsatisfactory, because we then
run the risk of passing inconsistent information.

This might explain partly why the init + finish approach isn't
new in this unit...

I'm attaching the patch which shows how option (1) looks like.
For me, the current approach strikes a better balance between
API cleanliness and implementation considerations. But I don't
have a strong opinion against implementing what you suggest,
including through option (2) (or other options I missed).

Let me know what you think.

-- 
Joel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-make-finish_fixed_point_type-create-the-type-renamin.patch
Type: text/x-diff
Size: 2751 bytes
Desc: not available
URL: <https://sourceware.org/pipermail/gdb-patches/attachments/20201114/0054f4ac/attachment.bin>


More information about the Gdb-patches mailing list