Re: [PATCH 4/4] gdb: make internalvar use a variant

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

> It requires more C++ expertise than most of other things to know to write such code.  Something that should be
> simple is complicated.  C++ is really lacking proper pattern matching and builtin variant types, to make
> variants easy to use.  But if we use it in a contained way like basically glorified tagged union to get rid
> of some boilerplace we have to write today,  I guess it's OK.

Yes, that was my thought.  The intent is replacing those instances of
"enum kind + union data" with something that can hold non-trivial
objects, calls constructors / destructors on data fields automatically,
validates that we don't access a wrong (inactive) field.

That being said, it's likely that many instances of this pattern are in
structures that aren't even new'ed / delete'd today, so it's not as if
we could use variants in them tomorrow.

> How debuggeable is nonstd::variant?  When printing a tagged union in GDB, it's easy to figure out the
> union's current value.  What does printing a nonstd::variant (not mapped to the std variant) look
> like? 

With nonstd:


$1 = {
  next = 0x60600000ff20,
  name = 0x602000085330 "salut",
  v = {
    data = {
      __data = '\276' <repeats 32 times>,
      __align = {<No data fields>}
    },
    type_index = 0 '\000'
  }
}

With std:


$1 = {
  next = 0x60600000ff20,
  name = 0x602000085350 "salut",
  v = std::variant<internalvar_void, internalvar_value, internalvar_make_value, internalvar_function, internalvar_integer, internalvar_string> [index 0] = {{<No data fields>}}
}

> AFAICT from variant.cpp source file, the variant storage is an untyped buffer:
> 
>     enum { data_align = detail::typelist_max_alignof< variant_types >::value };
> 
>     using aligned_storage_t = typename std::aligned_storage< data_size, data_align >::type;
>     aligned_storage_t data;
> 
> so I guess printing a nonstd::variant results in pretty opaque output.  We'd need a pretty
> printer to fix this.  Or maybe we just assume that people developing/debugging GDB build
> it against a C++17 or higher compiler?  (Not sure that's a great assumption.)

Do you know off-hand if the std::variant pretty printer is supposed to
show the active data field?

> I wonder whether we should do:
> 
>  namespace gdb {
>    using namespace nonstd;
>  }
> 
> and then use gdb::variant throughout, instead of "nonstd::variant".  At least "gdb"
> is three letters, and thus less code/indentation churn when we someday switch to "std".
> Also less churn if we move to some other variant type, though not sure that's likely
> after we start using one.

Good idea.

But now I'm not even convinced myself of the usefulness of having an
std::variant-like in the tree.  The internalvar change is really just an
example, but it doesn't really need changing (and Tom suggeted what's
probably a better way to change it if we wanted to).  I wanted to use a
variant in some TUI patch that I started, but haven't finished.  So, I
guess we have it here as a reference if the need ever comes up again.

Simon