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

[Simon Marchi <simon.marchi@polymtl.ca>]

Just some nits:

> @@ -475,6 +479,7 @@ add_set_or_show_cmd (const char *name,
>  
>    gdb_assert (type == set_cmd || type == show_cmd);
>    c->type = type;
> +
>    c->var.emplace (var_type, arg);

Spurious new line added.

> @@ -219,18 +249,42 @@ struct setting
>       (see VAR_TYPE_USES).  */
>    setting (var_types var_type, const erased_args &args)
>      : m_var_type (var_type),
> -      m_var (args.var)
> +      m_var (args.var),
> +      m_getter (args.getter),
> +      m_setter (args.setter)
>    {
>    }
>  
> -  /* Access the type of the current setting.  */
> -  var_types type () const
> +  /* Create a setting backed by setter and getter functions.
> +
> +     Type T must match the var type VAR_TYPE (see VAR_TYPE_USES).  */
> +  template<typename T>
> +  setting (var_types var_type,
> +	   setting_setter_ftype<T> setter,
> +	   setting_getter_ftype<T> getter)
> +    : m_var_type (var_type)
>    {
> -    return m_var_type;
> +    gdb_assert (var_type_uses<T> (var_type));
> +
> +    /* Getters and setters are cast to and from the arbitrary `void (*) ()`
> +       function pointer type.  Make sure that the two types are really of the
> +       same size.  */
> +    gdb_static_assert (sizeof (m_getter) == sizeof (getter));
> +    gdb_static_assert (sizeof (m_setter) == sizeof (setter));
> +
> +    m_getter = reinterpret_cast<erased_func> (getter);
> +    m_setter = reinterpret_cast<erased_func> (setter);
>    }
>  
> +  /* Access the type of the current setting.  */
> +  var_types type () const
> +  { return m_var_type; }
> +
>    /* Return the current value.
>  
> +     Calling this method is only valid if the current setting is stored in a
> +     variable referenced by m_var (i.e. 'storage () == storage::variable').

That part of the comment seems false.

Simon