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

[Joel Brobecker <brobecker@adacore.com>]

[-- Attachment #1: Type: text/plain, Size: 1087 bytes --]

Hi again, Simon,

> > @@ -1821,6 +1845,9 @@ extern void set_type_vptr_basetype (struct type *, struct type *);
> >    (TYPE_CPLUS_SPECIFIC(thistype)->virtual_field_bits == NULL ? 0 \
> >      : B_TST(TYPE_CPLUS_SPECIFIC(thistype)->virtual_field_bits, (index)))
> >
> > +#define TYPE_FIXED_POINT_INFO(thistype) \
> > +  (TYPE_MAIN_TYPE(thistype)->type_specific.fixed_point_info)
> 
> Do you think you can make this macro a method on struct type in the
> style of the ones that were added recently?  It will be one less macro
> to convert later :).  That method should contain an assert that the type
> code is indeed TYPE_CODE_FIXED_POINT.  I think it could return a
> `fixed_point_info &`, since there's no way it can fail and need to
> return nullptr (now I see that type::bounds should probably return a
> `range_bounds &` instead of a `range_bounds *`).

Can you tell me if the attached patch corresponds to what you had
in mind? As the revlog suggests, it's missing a bit of doc around it,
but I wanted to make sure I did this correctly before adding the forms.

Thank you,
-- 
Joel

[-- Attachment #2: 0001-WIP-convert-TYPE_FIXED_POINT_INFO-to-methods-of-stru.patch --]
[-- Type: text/x-diff, Size: 5574 bytes --]

From ad39478edc5d344414ab1ebd5f1eccc13de5a0ce Mon Sep 17 00:00:00 2001
From: Joel Brobecker <brobecker@adacore.com>
Date: Sat, 14 Nov 2020 06:27:25 -0500
Subject: [PATCH] WIP: convert TYPE_FIXED_POINT_INFO to methods of struct type

FIXME: Add proper revision log.

Change-Id: I4259542cc084775648accd135a70829b8ac09615
FIXME: Document the new methods.
---
 gdb/dwarf2/read.c |  4 ++--
 gdb/gdbtypes.c    | 16 ++++++++--------
 gdb/gdbtypes.h    | 23 +++++++++++++++++------
 3 files changed, 27 insertions(+), 16 deletions(-)

diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c
index 4a447d4..248f59b 100644
--- a/gdb/dwarf2/read.c
+++ b/gdb/dwarf2/read.c
@@ -18218,7 +18218,7 @@ get_dwarf2_unsigned_rational_constant (struct die_info *die,
 }
 
 /* Assuming DIE corresponds to a fixed point type, finish the creation
-   of the corresponding TYPE by setting its TYPE_FIXED_POINT_INFO.
+   of the corresponding TYPE by setting its type-specific data.
    CU is the DIE's CU.  */
 
 static void
@@ -18289,7 +18289,7 @@ finish_fixed_point_type (struct type *type, struct die_info *die,
 		 sect_offset_str (die->sect_off));
     }
 
-  gdb_mpq &scaling_factor = TYPE_FIXED_POINT_INFO (type)->scaling_factor;
+  gdb_mpq &scaling_factor = type->fixed_point_info ().scaling_factor;
   mpz_set (mpq_numref (scaling_factor.val), scale_num.val);
   mpz_set (mpq_denref (scaling_factor.val), scale_denom.val);
   mpq_canonicalize (scaling_factor.val);
diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index 2f92887..7d6e22d 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -5498,8 +5498,8 @@ copy_type_recursive (struct objfile *objfile,
       break;
     case TYPE_SPECIFIC_FIXED_POINT:
       INIT_FIXED_POINT_SPECIFIC (new_type);
-      TYPE_FIXED_POINT_INFO (new_type)->scaling_factor
-	= TYPE_FIXED_POINT_INFO (type)->scaling_factor;
+      new_type->fixed_point_info ().scaling_factor
+	= type->fixed_point_info ().scaling_factor;
       break;
     case TYPE_SPECIFIC_INT:
       TYPE_SPECIFIC_FIELD (new_type) = TYPE_SPECIFIC_INT;
@@ -5821,11 +5821,11 @@ static const struct objfile_key<fixed_point_type_storage>
 
 /* See gdbtypes.h.  */
 
-fixed_point_type_info *
+void
 allocate_fixed_point_type_info (struct type *type)
 {
   std::unique_ptr<fixed_point_type_info> up (new fixed_point_type_info);
-  fixed_point_type_info *result;
+  fixed_point_type_info *info;
 
   if (TYPE_OBJFILE_OWNED (type))
     {
@@ -5833,17 +5833,17 @@ allocate_fixed_point_type_info (struct type *type)
 	= fixed_point_objfile_key.get (TYPE_OBJFILE (type));
       if (storage == nullptr)
 	storage = fixed_point_objfile_key.emplace (TYPE_OBJFILE (type));
-      result = up.get ();
+      info = up.get ();
       storage->push_back (std::move (up));
     }
   else
     {
       /* We just leak the memory, because that's what we do generally
 	 for non-objfile-attached types.  */
-      result = up.release ();
+      info = up.release ();
     }
 
-  return result;
+  type->set_fixed_point_info (info);
 }
 
 /* See gdbtypes.h.  */
@@ -5878,7 +5878,7 @@ fixed_point_scaling_factor (struct type *type)
 {
   type = fixed_point_type_base_type (type);
 
-  return TYPE_FIXED_POINT_INFO (type)->scaling_factor;
+  return type->fixed_point_info ().scaling_factor;
 }
 
 \f
diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index 9cf847b..375a516 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -1216,6 +1216,21 @@ struct type
     this->main_type->m_flag_endianity_not_default = endianity_is_not_default;
   }
 
+  struct fixed_point_type_info &fixed_point_info () const
+  {
+    gdb_assert (this->code () == TYPE_CODE_FIXED_POINT
+		&& this->main_type->type_specific.fixed_point_info != nullptr);
+
+    return *this->main_type->type_specific.fixed_point_info;
+  }
+
+  void set_fixed_point_info (struct fixed_point_type_info *info) const
+  {
+    gdb_assert (this->code () == TYPE_CODE_FIXED_POINT);
+
+    this->main_type->type_specific.fixed_point_info = info;
+  }
+
   /* * Return the dynamic property of the requested KIND from this type's
      list of dynamic properties.  */
   dynamic_prop *dyn_prop (dynamic_prop_node_kind kind) const;
@@ -1769,7 +1784,7 @@ extern void allocate_gnat_aux_type (struct type *);
    handled.  */
 #define INIT_FIXED_POINT_SPECIFIC(type) \
   (TYPE_SPECIFIC_FIELD (type) = TYPE_SPECIFIC_FIXED_POINT, \
-   TYPE_FIXED_POINT_INFO (type) = allocate_fixed_point_type_info (type))
+   allocate_fixed_point_type_info (type))
 
 #define TYPE_MAIN_TYPE(thistype) (thistype)->main_type
 #define TYPE_TARGET_TYPE(thistype) TYPE_MAIN_TYPE(thistype)->target_type
@@ -1867,9 +1882,6 @@ extern void set_type_vptr_basetype (struct type *, struct type *);
   (TYPE_CPLUS_SPECIFIC(thistype)->virtual_field_bits == NULL ? 0 \
     : B_TST(TYPE_CPLUS_SPECIFIC(thistype)->virtual_field_bits, (index)))
 
-#define TYPE_FIXED_POINT_INFO(thistype) \
-  (TYPE_MAIN_TYPE(thistype)->type_specific.fixed_point_info)
-
 #define FIELD_NAME(thisfld) ((thisfld).name)
 #define FIELD_LOC_KIND(thisfld) ((thisfld).loc_kind)
 #define FIELD_BITPOS_LVAL(thisfld) ((thisfld).loc.bitpos)
@@ -2597,8 +2609,7 @@ extern const gdb_mpq &fixed_point_scaling_factor (struct type *type);
 
 /* Allocate a fixed-point type info for TYPE.  This should only be
    called by INIT_FIXED_POINT_SPECIFIC.  */
-extern fixed_point_type_info *allocate_fixed_point_type_info
-  (struct type *type);
+extern void allocate_fixed_point_type_info (struct type *type);
 
 /* * When the type includes explicit byte ordering, return that.
    Otherwise, the byte ordering from gdbarch_byte_order for 
-- 
2.1.4