Re: [PATCH 073/203] Introduce class operation

Lancelot SIX via Gdb-patches
2021-01-03T13:55:54Z

Hi everyone

On 03/01/2021 07:09, Joel Brobecker wrote:
> Hi Tom,
>
>> * ax-gdb.c (operation::generate_ax): New method.
> Thanks for this!
>
> I don't have much comment to share on this, at least not just yet
> until I have had a chance to see this new API in action in the rest
> of the patch series. I do have one small-tiny-mini suggestion:
>
> It's about:
>
>> +typedef std::unique_ptr<operation> operation_up;
> FWIW, the connection between "up" and the rest of the declaration
> wasn't immediately obvious to me. I think this is because "up"
> is an english word, and spelled like that, all in lowercase,
> that's really what my brain keeps analyzing it as first. It takes
> an effort to adjust my thinking against this bias.
>
> It's not a problem for me if we decide to keep this like that,
> but what do you think about about renaming this to something like
> "operation_uptr"? I think it'll make the intention more immediately
> clear to the reader.
>
I am fairly new to this codebase, but as far as I can say, this sort of
naming convention (the _up suffix for a typedefed std::unique_ptr) is
already used here and there:

$ git grep 'typedef std::unique_ptr.*_up;'|wc -l
33

I have to admin that the first time I came across it it was not obvious
to me either, but it would make sense to keep it this way for
consistency (I guess).

BR
Lancelot.