Re: [PATCH v2] gdbtypes: improve documentation of composite type helpers

On 17/08/2026 19:50, Simon Marchi wrote:
> On 8/14/26 6:49 AM, Matthieu Longo wrote:
>> Document how arch_composite_type handles a null NAME, and clarify the
>> semantics of an empty field name for the append_composite_type* helpers.
>>
>> Suggested-By: Simon Marchi <[email protected]>
>> ---
>>  gdb/gdbtypes.c | 12 ++++--------
>>  gdb/gdbtypes.h | 30 ++++++++++++++++++++++++++----
>>  2 files changed, 30 insertions(+), 12 deletions(-)
>>
>> diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
>> index 4b6c01910f4..e0c25c58d83 100644
>> --- a/gdb/gdbtypes.c
>> +++ b/gdb/gdbtypes.c
>> @@ -5412,8 +5412,7 @@ append_flags_type_flag (struct type *type, int bitpos, const char *name)
>>  			   name);
>>  }
>>  
>> -/* Allocate a TYPE_CODE_STRUCT or TYPE_CODE_UNION type structure (as
>> -   specified by CODE) associated with GDBARCH.  NAME is the type name.  */
>> +/* See gdbtypes.h.  */
>>  
>>  struct type *
>>  arch_composite_type (struct gdbarch *gdbarch, const char *name,
>> @@ -5428,9 +5427,7 @@ arch_composite_type (struct gdbarch *gdbarch, const char *name,
>>    return t;
>>  }
>>  
>> -/* Add new field with name NAME and type FIELD to composite type T.
>> -   Do not set the field's position or adjust the type's length;
>> -   the caller should do so.  Return the new field.  */
>> +/* See gdbtypes.h.  */
>>  
>>  struct field *
>>  append_composite_type_field_raw (struct type *t, const char *name,
>> @@ -5448,8 +5445,7 @@ append_composite_type_field_raw (struct type *t, const char *name,
>>    return f;
>>  }
>>  
>> -/* Add new field with name NAME and type FIELD to composite type T.
>> -   ALIGNMENT (if non-zero) specifies the minimum field alignment.  */
>> +/* See gdbtypes.h.  */
>>  
>>  void
>>  append_composite_type_field_aligned (struct type *t, const char *name,
>> @@ -5489,7 +5485,7 @@ append_composite_type_field_aligned (struct type *t, const char *name,
>>      }
>>  }
>>  
>> -/* Add new field with name NAME and type FIELD to composite type T.  */
>> +/* See gdbtypes.h.  */
>>  
>>  void
>>  append_composite_type_field (struct type *t, const char *name,
>> diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
>> index dd2d24fa8e2..f7853430d64 100644
>> --- a/gdb/gdbtypes.h
>> +++ b/gdb/gdbtypes.h
>> @@ -2431,20 +2431,42 @@ extern struct type *init_pointer_type (type_allocator &alloc, int bit,
>>  extern struct type *init_fixed_point_type (type_allocator &, int, int,
>>  					   const char *);
>>  
>> -/* Helper functions to construct a struct or record type.  An
>> -   initially empty type is created using arch_composite_type().
>> -   Fields are then added using append_composite_type_field*().  A union
>> -   type has its size set to the largest field.  A struct type has each
>> +/* Helper functions to construct a struct or record type.  An initially empty
>> +   type is created using arch_composite_type().  Fields are then added using
>> +   append_composite_type_field*().
>> +   A union type has its size set to the largest field.  A struct type has each
>>     field packed against the previous.  */
> 
> I would get rid of this generic comment and move the information to the
> other comments.
> 
>  - The doc of arch_composite_type can mention that the type initially
>    has no fields, and that fields can be added with the
>    append_composite_type_field*() functions
> 
>  - The part about union and struct sizes can be moved to the doc of
>    append_composite_type_field().
> 
> To avoid repeating things between all the three variants of
> append_composite_type_field*(), I would suggest using a form where the
> common information is documented at only one place (probably
> append_composite_type_field()) and the other functions refer to it
> 
> This is what I propose:
> 
> /* Allocate a structure or union type (as specified by CODE) associated with
>    GDBARCH.
> 
>    NAME is the type name.  If it is nullptr, the type is anonymous.
> 
>    The new type initially has no fields.  Fields can be added by calling
>    append_composite_type_field*.  */
> 
> extern struct type *arch_composite_type (struct gdbarch *gdbarch,
> 					 const char *name, enum type_code code);
> 
> /* Add a new field named NAME with type FIELD to composite type T.
> 
>    If NAME is an empty string and the field's type is a structure or a union,
>    the fields of that structure or union are visible directly in T.
> 
>    This function updates the size of T:
> 
>      - A union type has its size set to the largest field.
>      - A structure type has each field packed against the previous.  */
> 
> extern void append_composite_type_field (struct type *t, const char *name,
> 					 struct type *field);
> 
> /* Like append_composite_type_field, except that ALIGNMENT (if non-zero)
>    specifies the minimum alignment of the new field.  */
> 
> extern void append_composite_type_field_aligned (struct type *t,
> 						 const char *name,
> 						 struct type *field,
> 						 int alignment);
> 
> /* Like append_composite_type_field, except that this function does not
>    set the field's position or adjust the length of T; the caller is
>    responsible for doing so.
> 
>    Return the newly added field.  */
> 
> struct field *append_composite_type_field_raw (struct type *t, const char *name,
> 					       struct type *field);
> 
> Simon

Fixed as suggested above in r3:
https://inbox.sourceware.org/gdb-patches/[email protected]/

Matthieu