RE: [PATCH 05/16] gdb: inline address_space_{name, type_instance_flags}_to_{type_instance_flags, name}

AMD General

Hi Tom,

Thank you for your review.

On Tuesday, July 21, 2026 8:30 PM, Tom Tromey wrote:
...
> > +  enum type_pieces piece;
> > +  int int_val;
> > +  unsigned int aclass;
> > +
> > +  /* Check for Harvard address space delimiters.  */
> > +  if (streq (string, "code"))
> > +    {
> > +      piece = tp_harvard_aspace_identifier;
> > +      int_val = TYPE_INSTANCE_FLAG_CODE_SPACE;
> > +    }
> > +  else if (streq (string, "data"))
> > +    {
> > +      piece = tp_harvard_aspace_identifier;
> > +      int_val = TYPE_INSTANCE_FLAG_DATA_SPACE;
> > +    }
> > +  else if (gdbarch_address_class_name_to_id_p (gdbarch)
> > +        && gdbarch_address_class_name_to_id (gdbarch,
> > +                                             string,
> > +                                             aclass))
> > +    {
> > +      piece = tp_aclass_identifier;
> > +      int_val = (enum type_instance_flag_value) (aclass << 4);
> > +    }
> > +  else
> > +    error (_("Unknown address space/class specifier: \"%s\""), string);
> > +
> > +  element.piece = piece;
> >    insert_into (slot, element);
> > -  element.int_val
> > -    = address_space_name_to_type_instance_flags (gdbarch, string);
> > +  element.int_val = int_val;
>
> I think the various 'if' branches might as well just assign directly to
> element.*; and 'aclass' can be moved into the if like
>
>   else if (unsigned int aclass = 0;
>            gdbarch_address_class_name_to_id_p (...)
>            && ...)

I had chosen to write it the way I posted because we're inserting
two elements back-to-back, which are of different union kinds.
Looking at the code again based on your comments, I think we can
do better if we have an overload of the insert_into method.  I'm
adding a new refactoring patch and this patch will look better
based on that, IMHO.  Please check in v2.

>
> I didn't read the whole series yet but if we're going to use different
> words, as is done in that error message, then some spot in the manual
> ought to explain this.

I went through the GDB manual, and I don't see address classes or Harvard
address spaces described anywhere.  The built-in `@code` and `@data` syntax
is also not explained.  This seems to be a gap in the documentation.

I can write a small section for this, but let me do that in a separate
submission.

Regards,
-Baris