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.
