Should I return gsl::span<const T> instead of const std::vector<T>&

Question

I have a class with a std::vector<int> member and a member function returning a const reference to that vector.

class demo {
public:
    //...

    const std::vector<int> & test() const {
        return iv;
    }

private:

    std::vector<int> iv;
};

I plan to change the member type to a different array like container type with just enough functionality and a smaller memory footprint (e.g. std::experimental::dynarray, std::unique_ptr<int[]>). Therefore I thought it would be a good idea to not return the real container as a const reference but to return a view to the elements as a gsl::span<const int>.

class demo {
public:
    //...

    gsl::span<const int> test() const {
        return iv;
    }

private:

    std::vector<int> iv;
};

But this breaks code that worked with the const vector<int>& because two span instances of the same unmodified vector can't be used to iterate over the elements:

demo d;

std::cout << (d.test().begin() == d.test().begin()) << "\n";
std::cout << (d.test().end() == d.test().end()) << "\n";

for( auto it = d.test().begin(), end = d.test().end(); it != end; ++it )
    std::cout << *it << "\n";

This prints 0 0 and then crashes because the test it != end never fails. A range based for loop works of course, but this loop is valid and therefore must also work as expected. I had expected, that all spans from the same range of the same container are equal so that iterators of any of these spans are comparable (container not modified of course). Certainly there is a good reason why this isn't so.

So my question is, what is the best way to return such a view to elements of a array like container whose type should not be visible to the caller.

Stoma

Answer 1

You use iterator of temporary, so your iterator become invalided directly after affectation.

You may use the following:

auto&& view = d.test();
for (auto it = view.begin(), end = view.end(); it != end; ++it) {
    std::cout << *it << "\n";
}

Baneberry

your right. The problem is, that the for loop code was perfectly valid, when I returned the std::vector<int>&. I want to use this in a large code base, where many such loops exist and I don't want to break any code. I thought span is more or less only a pair of pointers to the underlying container and begin() and end() only return these pointers. Would this still be a problem with the temporary? – Stoma

something like this: class my_span { public: my_span( const std::vector<int> & iv ) : m_begin( &iv[0] ), m_end( &iv[0] + iv.size() ) {} my_span( const int * begin, const int * end ) : m_begin( begin ), m_end( end ) {} const int * begin() const { return m_begin;} const int * end() const { return m_end;} private: const int * m_begin; const int * m_end; }; – Stoma

@StefanF.: If you look at span.h:519, you see that iterator takes a pointer on span. Your implementation seems a valid one, but I'm not sure of the requirements of span::iterator. – Baneberry

Answer 2

So the answer, is NO, because it breaks code, that was valid before.

Stoma