Search code examples
c++iteratorc++17variadic-templateslifetime

Zip Class in C++ (Internal Object Lifetime)


I have the following Zip class in C++, which works the same as Python's zip.

When I run the below code, I get this output:

1 | 11
2 | 22
3 | 33

1 | 11 | 0             <--- problematic
2 | 22 | 6.91092e-317  <--- problematic 
3 | 33 | 9

In the second case, where I additionally create a temporary std::vector<int>{7, 8, 9}, it appears that the lifetime of this vector ends as soon as the Zip constructor is exited. Which is why we see the wrong values 0 and 6.91092e-317.

I wonder why that is. Because internally Zip stores its elements as const Ts&. So a const reference. Should that not extend the lifetime of the temporary vector until the parent Zip object is destructed (= after the for loop)? Is there any way to fix this?


Sample code, https://godbolt.org/z/4n6j83fn5:

#include <tuple>
#include <vector>
#include <iostream>

template<class... Ts>
class Zip
{
public:
    explicit Zip(const Ts&... objs)
        : m_data(objs...) { }

    struct ZipIterator
    {
    public:
        explicit ZipIterator(const std::tuple<const Ts&...>& data, std::size_t idx)
            : m_data(data), m_idx(idx) { }

        ZipIterator& operator++()
        {
            ++m_idx;
            return *this;
        }

        bool operator!=(const ZipIterator& rhs) const
        {
            return m_idx != rhs.m_idx;
        }

        auto operator*() const
        {
            return std::apply([this](auto const&... obj) { return std::forward_as_tuple(obj.at(m_idx)...); }, m_data);
        }

    private:
        const std::tuple<const Ts&...>& m_data;
        std::size_t m_idx;
    };

    ZipIterator begin() const
    {
        return ZipIterator(m_data, 0);
    }

    ZipIterator end() const
    {
        return ZipIterator(m_data, std::get<0>(m_data).size());
    }

private:
    std::tuple<const Ts&...> m_data;
};


int main()
{
    const std::vector<double> vec1{1,  2,  3};
    const std::vector<double> vec2{11, 22, 33};

    for (const auto& [v1, v2] : Zip(vec1, vec2))
    {
        std::cout << v1 << " | " << v2 << std::endl;
    }
    std::cout << std::endl;

    for (const auto& [v1, v2, v3] : Zip(vec1, vec2, std::vector<double>{7, 8, 9}))
    {
        std::cout << v1 << " | " << v2 << " | " << v3 << std::endl;
    }

    return 0;
}

Solution

  • @BoP answers your question perfectly regarding the "Why". This answer provides a possible fix to your issue.

    The fix involves a MaybeOwning class, which stores by reference if it is constructed from a non-temporary and stores by value if it is constructed from a temporary. To get this to work, you need to wrap the constructor in a factory function zip that properly forwards the correct types. That's why the constructor of Zip in this implementation is private.

    #include <tuple>
    #include <vector>
    #include <iostream>
    
    template <typename E>
    struct MaybeOwning
    {};
    
    template <typename E>
    struct MaybeOwning<E&&>
    {
        MaybeOwning(E&& e) : _val(e) {}
        E& get() { return _val; };
        E const& get() const { return _val; };
        E _val;
    };
    
    template <typename E>
    struct MaybeOwning<E&>
    {
        MaybeOwning(E& e) : _val(e) {}
        E& get() { return _val; };
        E const& get() const { return _val; };
        E& _val;
    };
    
    
    template<class... Ts>
    class Zip
    {
        template <typename... Args>
        friend auto zip(Args&&...);
    
    private:
        explicit Zip(Ts&&... objs)
            : m_data(MaybeOwning<Ts>(std::forward<Ts>(objs))...) { }
    
    public:
        struct ZipIterator
        {
        public:
            explicit ZipIterator(const std::tuple<MaybeOwning<Ts>...>& data, std::size_t idx)
                : m_data(data), m_idx(idx) { }
    
            ZipIterator& operator++()
            {
                ++m_idx;
                return *this;
            }
    
            bool operator!=(const ZipIterator& rhs) const
            {
                return m_idx != rhs.m_idx;
            }
    
            auto operator*() const
            {
                return std::apply([this](auto const&... obj) { return std::forward_as_tuple(obj.get().at(m_idx)...); }, m_data);
            }
    
        private:
            const std::tuple<MaybeOwning<Ts>...>& m_data;
            std::size_t m_idx;
        };
    
        ZipIterator begin() const
        {
            return ZipIterator(m_data, 0);
        }
    
        ZipIterator end() const
        {
            return ZipIterator(m_data, std::get<0>(m_data).get().size());
        }
    
    private:
        std::tuple<MaybeOwning<Ts>...> m_data;
    };
    
    template <typename... Ts>
    auto zip(Ts&&... ts){
        return Zip<Ts&&...>(std::forward<Ts>(ts)...);
    }
    
    
    int main()
    {
        const std::vector<double> vec1{1,  2,  3};
        const std::vector<double> vec2{11, 22, 33};
    
        for (const auto& [v1, v2] : zip(vec1, vec2))
        {
            std::cout << v1 << " | " << v2 << std::endl;
        }
        std::cout << std::endl;
    
        for (const auto& [v1, v2, v3] : zip(vec1, vec2, std::vector<double>{7, 8, 9}))
        {
            std::cout << v1 << " | " << v2 << " | " << v3 << std::endl;
        }
    
        return 0;
    }
    
    1 | 11
    2 | 22
    3 | 33
    
    1 | 11 | 7
    2 | 22 | 8
    3 | 33 | 9
    

    https://godbolt.org/z/Tsa8W31v7