mpark / variant

C++17 `std::variant` for C++11/14/17
https://mpark.github.io/variant
Boost Software License 1.0
659 stars 88 forks source link

Optimization: If all alternatives are trivially copyable variant should never become valueless by exception #59

Open rolandschulz opened 5 years ago

rolandschulz commented 5 years ago

This would be equivalent to the optimization in libstdc++ https://gcc.gnu.org/viewcvs/gcc?view=revision&revision=267614 .

There are doubts that this specific optimization is conformant. Because even the trivial move has observable side-effect (the address). Instead it might be better to move the value into a temporary, create the new one in-place. If any exception occurred: move the the temporary back.

The extra moves can be avoided if not only all alternatives are trivially copyable but also the specific constructor is noexcept.

A possible extension (but non conformant) would be to make a variant never valueless not only for trivially copyable but also for nothrow move/copy assignable (behind an option). This would be similar to #25. In practice extremely few types used as an alternative would have an observe a side-effect (the type would either have to have an ill-formed copy/move assignment (is a probably for testing with traits), or is nothrow move/copy assignable but the constructor used isn't nothrow and the move/copy has an observable side-effect.

It seems it would also make sense to make variant guranteed non-valueless if MPARK_EXCEPTIONS is false (as an extension to #30).

Mike-Devel commented 5 years ago

Because even the trivial move has observable side-effect (the address).

Could you elaborate on that? Obersvable to whom?

rolandschulz commented 5 years ago

See https://wandbox.org/permlink/XHwzxHs2lzbi3J3k here (credit to arthur-odwyer for example). What's observable is the difference between the pointer during construction and anytime after the emplace.

Mike-Devel commented 5 years ago

Isn't that an issue with the type having an incorrect copy/move constructor? If my type holds a reference to itself, then that reference should definetly be updated during copy or the special member functions should be deleted.

I'm not a language laywer, so I don't know what is and what is not permissible according to the standard, but this is one of the cases, where it is imho much better to provide an optimization for the large majority of users instead of supporting some broken type.

rolandschulz commented 5 years ago

I agree it is an obvious stupid type. I asked about that on cpplang.slack (Feb 24 on #general). The conversation (answer by Arthur):

Does that mean the libstdc++ optimization is non conforming? Above you wrote "may be non conforming". Why "may"?

Well, I think it is, but I don't quite remember and don't want to imply that LWG definitely thought it was. My recollection is "yeah it is non conforming but such pathological types are stupid and if the vendor doesn't want to conform in this case I don't think anyone will lose sleep over it."

So it depends on how strict one wants to be with conformance. I think my suggestion allows an optimization which is as good as the GCC one. And it is fully conforming as far as I can see and I also don't see a disadvantage. So I think it is the slightly better option. I agree thought that either would be a good option because I also agree it isn't really worth worrying too much about such a pathological type.