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

[Tom Tromey <tom@tromey.com>]

Simon> The advantage of a variant here is that we can more easily use non-POD
Simon> types in the alternatives.  And since it remembers which alternative is
Simon> the current one, accesses are checked so that accesses to the wrong
Simon> alternative throw an exception (see question about that below).

I was wondering if we really need internalvars that can "switch" like
this.  Like, could we just subclass internalvar instead and get rid of
the "kind" / payload stuff.

[...]
Simon> with some functor + std::visit.

FWIW I tend to dislike the visitor pattern anyhow.  IME it is just a
pain to use well -- lots of callbacks, the code becomes unreadable.

Simon>  - The library uses std::variant if it is available (so, if building
Simon>    with C++17).  I think there are slight differences between the std
Simon>    and nonstd versions of variant.  I can't really explain what they
Simon>    are, I am not expert enough in C++.  But for example, when I had not
Simon>    made my visitor's methods const, the code would compile with
Simon>    std::variant but not nonstd::variant.  This means we could get some
Simon>    occasional build failures if some people build in C++11/14 and others
Simon>    C++17.  One way to avoid this would be to define the macro
Simon>    variant_CONFIG_SELECT_VARIANT so that we always use nonstd::variant,
Simon>    regardless of the C++ version.  The small downside is that we
Simon>    wouldn't be testing against std::variant, so some day in the future
Simon>    we want to make the switch to use std::variant, we'll have some fixes
Simon>    to do.

We already have a problem along these lines with string_view, I think,
where we imported one that allows nullptr, then the standard changed(?).
On the whole I'd prefer we try to stick to the standard stuff, if
possible, because once these divergences come in they seem hard to root
out.

Simon>    With exceptions enabled, bad_variant_access is thrown.  Without
Simon>    exceptions enabled, an assert() fails.  In our context, it would be
Simon>    nice if we could call gdb_assert fails, such that we get the usual
Simon>    "internal error" message if that ever happens.  I don't see an easy
Simon>    way to do this other than modifying the header file itself.

Yeah, not sure what to do about that except write some kind of wrapper.

Tom