[v3 PATCH, RFC] Rewrite variant. Also PR libstdc++/85517

[Jonathan Wakely]

On 06/02/19 00:05 +0200, Ville Voutilainen wrote:
>Okay then. This patch takes the hopefully biggest steps towards
>a std::variant rewrite. The problem we have with the current
>approach is that we'd really like to write fairly straightforward
>code for variant's special member functions, but can't, because
>the specification suggests fairly straightforward compile-time-property
>queries and selection of behavior based on those, but variant's selected
>index is a run-time property. This leads to difficulties implementing
>what the standard requires, like the differences of handling throwing and
>non-throwing move/copy operations of the types held in variant.
>
>Well. We apply the hammer of Pattern Matching. variant's visit()
>can get us into code that can do the compile-time queries despite
>variant's selected index being a run-time property. We modify
>the visitation mechanism to not throw an exception if invoked
>with a special kind of visitor that can handle valueless variants,
>and then use polylambdas as such visitors, and do if-constexprs
>in those polylambdas.
>
>We can now get rid of the function-pointer tables that are used for
>similar kind of dispatching in the original approach. Visitation
>generates similar tables, so we keep the O(1) indexing into the right
>function based on the current selected index, and there's the same
>amount of indirect calls through a pointer-to-function, one.
>Our code for e.g. variant's copy assignment is now straightforward;
>it visits both the this-variant and the rhs-variant at the same time,
>and does the right thing based on the compile-time properties of the
>selected type,
>and whether either variant is valueless. Modulo the polylambda wrapping
>and the fact that all ifs in it are if-constexprs, it looks like it's just
>doing conditional code based on what the nothrow-properties, for example,
>of the type of the object held by the variant are.

Did you compare the codegen?

I think the assumption was this would produce smaller code. Does that
hold true?

>This patch also partially gets rid of some of the cases where an object
>is self-destroyed before it's reconstructed. We shouldn't do that.

Agreed, that's been concerning me since I noticed it.

>We should be doing _M_reset followed by _M_construct, so that we only
>ever destroy a variant member of a union object held in the variant's
>storage, and then try to reconstruct that (or some other variant member),
>and we should never self-destroy any object in the inheritance chain of
>variant. The rest of that is work-in-progress, as is changing the rest
>of the special member functions to use visitation.
>
>Thoughts?

I like it. As you know, I've never liked that _S_vtable[] array of
function pointers, and the this->~variant() calls are undefined.

This is also necessary to implement the P0602R4 changes, and seems to
do so without ABI changes to our std::variant (its layout is not
changed by the patch).

As it only touches a C++17 component I'm inclined to approve it for
trunk before gcc-9.

Are you still working on the rest of the special member functions? It
seems to me that they could be fixed separately anyway, it doesn't
need to be done all at once. Each special member function is
independent of the others.