Re: [PATCH 1/2] gdb: return *this in target_waitstatus setters

[Simon Marchi via Gdb-patches <gdb-patches@sourceware.org>]

On 2021-12-03 08:10, Pedro Alves wrote:
> On 2021-12-02 01:40, Simon Marchi via Gdb-patches wrote:
>> From: Simon Marchi <simon.marchi@efficios.com>
>>
>> While playing with some code creating target_waitstatus objects, I was
>> mildly annoyed by the fact that we can't just return a new
>> target_waitstatus object.  We have to do:
>>
>>     target_waitstatus ws;
>>     ws.set_exited (123);
>>     return ws;
>>
>> Make the setters return the "this" object as a reference, such that it's
>> possible to do:
>>
>>     return target_waitstatus ().set_exited (123);
>>
>> I initially thought of adding static creation functions, which you would
>> use like:
>>
>>     return target_waitstatus::make_exited (123);
>>
>> However, making the setters return a reference to the object achieves
>> pretty much the same thing, with less new code.
> 
> I think the static methods would be nicer (I'd maybe drop the "make_" to keep
> it short), but I'm fine with this too, it doesn't seem mutually exclusive.

I started with this idea to add static methods and remove the setters,
making target_waitstatus essentially immutable, except through the
assignment operator.  So to change an existing target_waitstatus object,
you would do use static method + assignment operator:

  lp->waitstatus = target_waitstatus::make_stopped (...);

instead of today:

  lp->waitstatus.set_stopped ();

But that would have needed a lot of unnecessary churn again, updating
all existing calls (which I did when introducing the setters).  And
having two ways of setting a target_waitstatus seems redundant, I prefer
if there's just one way.  It makes the code more consistent, easier to
grep for (you can grep for `set_exited`, for example), etc.  So I went
for the solution presented in this patch, but if someone wants to
convert to static methods later, I will be fine with it.

> Series LGTM.

Thanks, pushed.

Simon