destructor is called twice for variant-based implementation

Question

I have a variadic variant_callable class object that I want to use for a runtime polymorphism. Inside it uses a visitor pattern with std::variant.
However, I came by a rather strange behavior, that is object's destructor is called twice!.

#include <utility>
#include <variant>
#include <tuple>

namespace detail
{
    template<typename... Impl>
    class variadic_callable
    {
    public:
        template<typename T>
        constexpr explicit variadic_callable(T &&t)   //
          : varImpl_(std::forward<T>(t))
        {}

        variadic_callable(const variadic_callable &) = delete;
        variadic_callable(variadic_callable &&) = delete;

        template<typename... Args>
        constexpr decltype(auto) operator()(Args &&...args) const
        {
            return std::visit(
                [argsTuple = std::forward_as_tuple(args...)](const auto &visitor) {
                    return std::apply(
                        [&visitor](auto &&...args) {
                            return visitor(std::forward<decltype(args)>(args)...);
                        },
                        argsTuple);
                },
                varImpl_);
        }

    private:
        std::variant<Impl...> varImpl_;
    };
}   // namespace detail

#include <string>
#include <iostream>

int main(int, char **)
{
    struct callable
    {
        std::string str = "Long enough string to be allocated. Oceanic"; 

        callable()
        {
            std::cout << "callable()" << std::endl;
        }

        void operator()(int i) const
        {
            std::cout << str << " " << i << '\n';
        }

        ~callable()
        {
            std::cout << "~callable()" << std::endl;
        }
    };

    {
        std::cout << "expcected:\n";
        const auto &c = callable();
        c(815);
        std::cout << "finished\n";
    }
    std::cout << '\n';

    {
        std::cout << "actual\n";
        const auto &w = detail::variadic_callable<callable>{callable()};
        w(815);
        std::cout << "finished\n";
    }

}

The output:

Program returned: 0
expcected:
callable()
Long enough string to be allocated. Oceanic 815
finished
~callable()

actual
callable()
~callable()
Long enough string to be allocated. Oceanic 815
finished
~callable()

https://godbolt.org/z/d849EaqbE

I guess an UB is in-place, but I can't spot it.
What I find the most peculiar is the fact that in the "actual" case std::string resources are not destroyed after the first destructor invocation!

Lets add in a copy constructor and some sanitizers and see what happens: https://godbolt.org/z/rKG5nG95x Looks like a copy was made somewhere, explaining the extra destructor. Running the program in a debugger and placing a breakpoint in the copy constructor should tell you where and why. — user4581301, Sep 16 '22 at 00:52
@user4581301 I assumed that mandatory copy elision would kick in. — Sergey Kolesnik, Sep 16 '22 at 01:02
Where, exactly, do you believe that mandatory copy elision happens here? — Sam Varshavchik, Sep 16 '22 at 01:05
I'd expect elision to take place with `callable()` here: `detail::variadic_callable{callable()}`, but the innards of `variant` get... complicated. Following my own advice, it looks like the copy is happening 10 or 11 function calls into the stuff happening behind the scenes in the constructor call. — user4581301, Sep 16 '22 at 01:08
@SergeyKolesnik Sam already knew this. He was challenging you to look deeper. — user4581301, Sep 16 '22 at 01:11
Mandatory copy elision occurs in very limited circumstances. Which specific use case do you believe applies here? "Perfect forwarding" never involves copy elision. — Sam Varshavchik, Sep 16 '22 at 01:45

user17732522 · Accepted Answer · 2022-09-16T01:39:29.013

4

variadic_callable's constructor is being passed an object of type callable. This is a temporary object that cannot be the same object as the one stored in the std::variant (no matter how it is passed).

The callable inside the std::variant must therefore be move-constructed from the passed temporary object. Both of these objects need to be eventually destroyed, requiring two calls to callable's destructor.

To prevent this you need to pass the arguments from which callable is supposed to be constructed to variadic_callable's constructor instead (here an empty list) and then pass these on to std::variants in-place constructor, i.e.

 template<typename T, typename... Args>
 constexpr explicit variadic_callable(std::in_place_type_t<T> t, Args&&... args)   //
   : varImpl_(t, std::forward<Args>(args)...)
 {}

called as

detail::variadic_callable<callable>{std::in_place_type<callable>};

Here I copied std::variant's constructor design for the in-place overload.

edited Sep 16 '22 at 01:39

answered Sep 16 '22 at 01:17

user17732522

53,019
2
56
105

Does it mean that elision can not work with perfect forwarding? – Sergey Kolesnik Sep 16 '22 at 01:20
1

@SergeyKolesnik You can never elide copies through a function call. You can elide copies into the by-value parameter of a function and into the by-value return value. But you can not identify an object in a parameter with the return value or, as here, an object somewhere outside the function with an object inside the function or passed as reference (as here) to the function. – user17732522 Sep 16 '22 at 01:24
how would one workaround in_place construction given a factory callable? https://godbolt.org/z/v455echME - the third example – Sergey Kolesnik Sep 16 '22 at 10:52
I just remembered that you answered a similar question of mine: https://stackoverflow.com/a/73153603/9363996 – Sergey Kolesnik Sep 16 '22 at 10:54
@SergeyKolesnik `std::variant` doesn't offer an interface of the kind I mentioned in my other answer (I don't think any of the standard library does). You'll have to return a full `std::variant` from the lambda. For example: https://godbolt.org/z/n8vW4Yz83. (The second `std::in_place_type` is now redundant, but you still need some tag to distinguish the constructors.) This is probably not a good solution because it requires knowing about an implementation detail of the class. – user17732522 Sep 16 '22 at 14:45
https://stackoverflow.com/q/73744465/9363996 I opened a question regarding variant's construction. There's an answer that suggests an approach which seems promising – Sergey Kolesnik Sep 16 '22 at 14:50
@SergeyKolesnik Ah ok, I was just about to suggest something similar to that as a more elaborate alternative. – user17732522 Sep 16 '22 at 14:54
Your suggestion would be very appreciated :) – Sergey Kolesnik Sep 16 '22 at 15:01
@SergeyKolesnik It would have been equivalent to what the answer in your other question already gives only a bit less elegantly written. – user17732522 Sep 16 '22 at 15:04

destructor is called twice for variant-based implementation

1 Answers1