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

>>>>> Tankut Baris Aktemur <[email protected]> writes:

> Inline the functions and remove them.  This is a step towards
> separating the handling of two concepts.

> In type-stack.c, which is used by the parser to convert user inputs
> into types, "@code" and "@data" and architecture-specific address
> class names are treated the same, too, blurring the difference and
> their storage in type instance flags.  While we inline the use of
> address_space_name_to_type_instance_flags there, we also separate the
> two topics by defining different tokens.  The patch still pushes type
> instance flags into the type stack.  The subsequent patch will further
> clean this up to store address class and address space ids.

On the one hand, this seems like a mild step backward in the sense that
if we ever wanted to support these things in non-C languages, it would
have to be reimplemented there.  OTOH, nobody has seen fit to do that in
the last 20 years or whatever it is.

> +  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 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.

Tom