On Fri, 21 Aug 2020 at 20:08, Victor Khomenko
<victor.khomenko_at_[hidden]> wrote:
> Let's use "alternative" rather than "element" to be consistent with std::holds_alternative and std::variant_alternative[_t]. Also, as there can be many alternatives, it would be a pain (and error-prone) to list them all in the correct order, so I'd rather pass the variant type:
>
> template <class Variant, class Alternative>
> Variant& variant_from_alternative(Alternative& a);

>From the call site, it makes very little difference. You call this version with

variant_from_alternative<variant<int, string>>(something);
as opposed to
variant_from_alternative<int, string>(something);

But I suppose your point indeed is that you already have a typedef for
the variant, so you don't need to list the alternatives.
Granted, for that part, using just the variant as the template
parameter is better.

> > auto& var = variant_from_element<int, string>(my_arg);
>
> This is something like what I had in mind when saying that "one can theoretically make this mistake and still get the code compiled" - note that by using auto the programmer explicitly and willingly forfeits type checking. I agree there is a risk of UB, especially in your version of the function when one can list the alternatives in a wrong order - I think my version mitigates this, because (I speculate) in practice the code has only few variant types, and they have typedef'ed names as one would be too lazy to write the variant declaration more than once. With all these mitigations, I believe the risk/benefit ratio is very good.

The problem here is not auto. The problem is knowing what the
variant's alternative types are. Getting them wrong
is a run-time error. If the variant is passed in as a parameter, there
is no run-time error.

> > variant<int, string, string>, you wouldn't know which string element it is.
> This is not a problem - the reference to the variant does not depend on which of two strings is held in the variant, so the function returns the same result.

Ah, yes, of course. :) And because the structure of the variant is the
same for both string alternatives, the offset calculation
and cast for getting to the surrounding variant is the same.

> BTW, I thought about your solution and think there are more problems:
> * one in effect passes the same reference twice (with different types) to operator(), which creates redundancy and has runtime penalty, and somehow feels weird (I struggle to explain why :-); in contrast, variant_from_alternative is 0-cost.

I doubt that runtime penalty remains after inlining.

> * if the variant is passed by rvalue reference, the visitor member functions would presumably be declared as
> ret_type operator()(Variant&& v, Alternative&& a);
> where v and a have in fact the same address. This calls for trouble as by moving out from one of the parameters you tacitly change the other into an undefined state. The problem still remains if one uses Variant& instead of Variant&. (I guess there is no such flexibility for Alternative&& as one should be consistent with std::visit).

Well, then don't move from the variant if you still need to access its
guts. That doesn't seem like a massive problem to me.