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

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