Re: [PATCH 15/16] gdb: make cmd_list_element var an optional union

[Lancelot SIX <lsix@lancelotsix.com>]

On Wed, Jul 28, 2021 at 02:13:21PM -0600, Tom Tromey wrote:
> >>>>> "Lancelot" == Lancelot SIX via Gdb-patches <gdb-patches@sourceware.org> writes:
> 
> Lancelot> This patch proposes to add an abstraction around the pair of the
> Lancelot> var_type and the void* pointer in order to prevent the user having to
> Lancelot> handle the cast.  This is achieved by introducing the setting struct,
> Lancelot> and a set of templated member functions.  The template parameter is the
> Lancelot> VAR_TYPES we want to the access the data for.  The template ensures the
> Lancelot> return type of the get method and the parameter type of the set method
> Lancelot> matches the data type used to store the actual data.
> 
> Seems nice.
> 
> Lancelot> 	int get<var_integer, var_zinteger> ()
> 
> It seems to me that it might be better if the get<> template itself knew
> about the storage equivalence between different var_ types and so code
> like this:
> 
> Lancelot> +	    var = cmd->var.get<var_string,
> Lancelot> +			       var_string_noescape,
> Lancelot> +			       var_optional_filename,
> Lancelot> +			       var_filename> ();
> 
> ... could be simplified.
> 
> Tom

Hi,

Would you prefer to have something like var.get<var_string> () return a
string happily if var is lets say a var_filename but fail if it is a
var_boolean?

I guess it is possible, but I did intentionally went for a very explicit
approach.  The idea is to have the user of the class state that he is
aware he will access a data of a given type that can be interpreted in
different ways.  This kind of makes more sense when dealing with the
various var_*integer* types where the same value can have different
meaning depending on the effective var_type.

In a way, I guess this is covered by the various fall through of the
switch / case statement where this call is typically used.  The
limitation of this is that the switch will not complain if you try to
share a code path for incompatible types, you will need to wait for a
runtime error instead of a compile time error.

Anyway, if you find it more practical, I can try rework the interface
this way.

By the way, I just posted a complete series that contains this proposal,
plus other improvements[1].

I’ll probably wait a bit to see if I get different feedbacks and
come up with something I fill will work for the most. 

Thanks,
Lancelot.

[1] https://sourceware.org/pipermail/gdb-patches/2021-July/181183.html