Re: [PATCH v3 1/3] arc: Add ARCv2 XML target along with refactoring

[Simon Marchi <simark@simark.ca>]

On 2020-07-22 9:36 a.m., Shahab Vahedi wrote:
> On Thu, Jul 16, 2020 at 09:28:50AM -0400, Simon Marchi wrote:
>> On 2020-07-15 4:35 p.m., Shahab Vahedi wrote:
>> When I search for `arc_gdbarch_features_init` in that patch I don't find anything...
> 
> You're correct. I have it in my local branch. The patch that uses it is going
> to be submitted very soon. I hope you don't mind if it remains like this.

Let's just make it static in this patch, and make in non-static again in your future
patch, it's not a big change.

>> The code in GDB constructing an arc_gdbarch_features will use the BFD to determine
>> these two values, whereas the code in GDBserver will use something else (usually
>> looking at the current process' properties).
>>
>> In fact, the constructor is optional, you could just build a arc_gdbarch_features
>> using aggregate initialization and return it from that function:
>>
>>   arc_gdbarch_features features {reg_size, isa};
>>
>> It doesn't really matter.  I just happen to prefer the constructor method, because
>> that makes it so you can't "forget" a field and it ensures it can never be in an
>> uninitialized state.
> 
> I see now. Actually, I have usecases to not initialize it immediately,
> but in a few lines of code.

I'd be curious to see.  Because instead of declaring it immediately, you can keep
the fields separate until you initialize it:

  int reg_size;
  enum arc_isa isa;

  if (something)
    {
      reg_size = 8;
      isa = foo;
    }
  else
    {
      reg_size = 4;
      isa = bar;
    }

  arc_gdbarch_features features (reg_size, isa);

I think this is good, because the day you add a third axis to arc_gdbarch_features,
that code will not build and you'll be forced to updated it (you can't forget it).

Whereas with:

  arc_gdbarch_features features;

  if (something)
    {
      features.reg_size = 8;
      features.isa = foo;
    }
  else
    {
      features.reg_size = 4;
      features.isa = bar;
    }

It's possible to forget.

But maybe you have a different use case in mind?

Simon