Re: [PATCH] PR libstdc++/87431 re-adjust never-valueless optimizations

[Antony Polukhin <antoshkka at gmail dot com>]

Looks good. Covers most of the use cases.

Please consider adding filesystem::path, pair, tuple, string_view?,
error_code, list, deque (myabe all the other containers), optional, variant
itself (for cases when variant holds another variant).

It would be very useful for the _Never_valueless_alt to return true for
aggregates that hold only never valueles types. Not sure that there is a
compiler builtin for getting all the aggregates types, but it could be
implemented via metaprogramming.

When is the ABI freeze for the C++17 additions?


On Wed, Apr 3, 2019, 19:25 Jonathan Wakely <jwakely@redhat.com> wrote:

> Avoid creating arbitrarily large objects on the stack when creating
> temporaries in order to provide the strong exception-safety guarantee.
>
> Also implement Antony Polukhin's suggestion to whitelist specific types
> that can be efficiently move-assigned, so that emplacing those types
> never causes a variant to become valueless. The whitelisted types are:
> all trivially copyable types no greater than 256 bytes in size,
> std::shared_ptr, std::weak_ptr, std::unique_ptr, std::function, and
> std::any. Additionally, std::basic_string, std::vector, and
> __gnu_debug::vector are whitelisted if their allocator traits give them
> a non-throwing move assignment operator. Specifically, this means
> std::string is whitelisted, but std::pmr::string is not.
>
>         PR libstdc++/87431 (again)
>         * include/bits/basic_string.h (__variant::_Never_valueless_alt):
>         Define partial specialization for basic_string.
>         * include/bits/shared_ptr.h (_Never_valueless_alt): Likewise for
>         shared_ptr and weak_ptr.
>         * include/bits/std_function.h (_Never_valueless_alt): Likewise for
>         function.
>         * include/bits/stl_vector.h (_Never_valueless_alt): Likewise for
>         vector.
>         * include/bits/unique_ptr.h (_Never_valueless_alt): Likewise for
>         unique_ptr.
>         * include/debug/vector (_Never_valueless_alt): Likewise for debug
>         vector.
>         * include/std/any (_Never_valueless_alt): Define explicit
>         specialization for any.
>         * include/std/variant (_Never_valueless_alt): Define primary
> template.
>         (__never_valueless): Use _Never_valueless_alt instead of
>         is_trivially_copyable.
>         (variant::emplace<N>(Args&&...)): Add special case for non-throwing
>         initializations to avoid try-catch overhead. Add special case for
>         scalars produced by potentially-throwing conversions. Use
>         _Never_valueless_alt instead of is_trivially_copyable for the
>         remaining strong exception-safety cases.
>         (variant::emplace<N>(initializer_list<U>, Args&&...)): Likewise.
>         * testsuite/20_util/variant/87431.cc: Run both test functions.
>         * testsuite/20_util/variant/exception_safety.cc: New test.
>         * testsuite/20_util/variant/run.cc: Use pmr::string instead of
> string,
>         so the variant becomes valueless.
>
> I'd like to commit this to trunk this week. Does anybody see any
> problems with this approach?
>
> As the comment in <variant> says, the _Never_valueless_alt trait has
> ABI impact, so we should define it for more types now, or not at all.
> When we add new types to the library we should consider if they need a
> specialization of _Never_valueless_alt.
>
> Should we also whitelist some (all?) other containers, as they all
> have conditionally noexcept move assignment operators, and those
> assignments are efficient when noexcept (because they don't
> reallocate). Are there other types in the library we should consider
> it for?
>
> std::optional<T> is automatically whitelisted for trivially copyable
> types, because if T is trivially copyable then optional<T> should be
> too. We could also whitelist other specializations though:
>
> template<typename T>
> struct _Never_valueless_alt<optional<T>>
> : _Never_valueless_alt<T>
> { };
>
> We could also do:
>
> template<typename T, typename U>
> struct _Never_valueless_alt<pair<T, U>>
> : __and_<_Never_valueless_alt<T>, _Never_valueless_alt<U>>
> { };
>
> template<typename... T>
> struct _Never_valueless_alt<tuple<T...>>
> : __and_<_Never_valueless_alt<T>...>
> { };
>
> In case it isn't clear, under normal circumstances something like a
> variant<pair<int,int>,int> won't ever become valueless anyway, because
> initialization of those alternatives doesn't throw. But what can throw
> is a conversion operator that is invoked to produce one of those
> alternatives. The _Never_valueless_alt trait says that emplacing an
> alternative of that type should do some extra work to ensure the
> variant doesn't become valueless *ever*. And then a variant that only
> has never-valueless alternatives is itself never-valueless, and so the
> valueless_by_exception() member just returns false, and visitation is
> more efficient as it doesn't need to deal with the valueless state.
>
>