Re: [PATCH v2 06/11] Add a more general version of lookup_struct_elt_type.

[Pedro Alves <palves@redhat.com>]

On 03/08/2019 12:04 AM, John Baldwin wrote:
> On 3/7/19 7:53 AM, Simon Marchi wrote:
>> Not really a big deal, but I find it a bit overkill to define a type 
>> just for this, I would have probably just returned the the offset as an 
>> output parameter.  But maybe this way is better, in that if we want to 
>> add something to the return type, we don't have to update the callers.
> 
> Honestly, I just modeled this after the similar code in the Linux
> kernel thread patches.  Using a reference parameter for the offset
> would be fine and I don't mind making that change.  I don't think that
> we are going to add more things to that structure in the future.  The
> field generally has everything else you want to know other than the
> offset.
> 

Note that you can keep the struct with just the data fields
and no ctor boilerplate if you use brace initialization.

I.e., with just:

struct struct_elt
{
  /* The field of the element, or NULL if no element was found.  */
  struct field *field;

  /* The bit offset of the element in the parent structure.  */
  LONGEST offset;
};

You write:

 return {nullptr, 0};
 return {&TYPE_FIELD (type, i), TYPE_FIELD_BITPOS (type, i)};

Instead of:

 return struct_elt ();
 return struct_elt (&TYPE_FIELD (type, i), TYPE_FIELD_BITPOS (type, i));




BTW, I noticed the missing space before parens below:

 > -	  return TYPE_FIELD_TYPE (type, i);
 > +	  return struct_elt (&TYPE_FIELD(type, i), TYPE_FIELD_BITPOS (type, i));

Thanks,
Pedro Alves