Re: [PATCH v2 3/4] gdb: Have setter and getter callbacks for settings

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

On 2021-08-08 3:23 p.m., Lancelot SIX via Gdb-patches wrote:
> The main motivation behind this improvement is to help the
> implementation of a patch Simon Marchi is preparing to fix a bug when
> MI or Python try to access parameters that are inferior dependent (see
> PR/28085).
> 
> This commit extends the previous one, which introduces the setting
> object to represent a static variable that can be set or shown with the
> appropriate commands.  This patch proposes that a setting can either
> contain a pointer to a static variable holding a setting, or pointers to
> a pair of setter and getter callback functions.
> 
> The callbacks functions can be used to generate values on the fly when
> the setting is accessed (or set the value in a context dependent way).
> This is useful when the source of truth is not contained in the
> variable pointed to by the setting instance.
> 
> Given that the callback function call is hidden within the setting
> abstraction introduced earlier, none of the sites accessing the setting
> needs to be updated.  The registered getter or setter is used whatever
> the way to access it is (through MI, Python, Guild, the "with" command
> and the $_gdb_setting / $_gdb_setting_str convenience functions).
> 
> All the add_setshow_*_cmd are given a new overload that will accept the
> pair of function pointers (set / get functions) instead of the pointer
> to a global variable.

LGTM, except a small issue I found while rebasing my patch on top of
your series:

> @@ -231,25 +427,50 @@ struct base_setting
>      gdb_assert (var_type_uses<T> (this->m_var_type));
>      gdb_assert (!this->empty ());

I think this needs to call operator bool, like so:

  gdb_assert (*this);

Otherwise, the assert fails when using a getter/setter (empty only
checks for m_var to be non-NULL, which is false when using
getter/setter).

I don't find this notation super clear, it might be clearer if we could
call a named method instead of operator bool.  Maybe "empty" could mean
"does not have a buffer nor getter/setter"?

Simon