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