0

In my code, I would like to create a new class named Span by extending the std::span class, but the new Span has some extra member functions which does not exist in std::span.

One is adding the remove_prefix function to std::span, which is implemented and works OK. See my previous question for details: what is the way to remove the first element from a std::span?

Now I would like to add another member function named substr(I try to imitates some member functions of std::string_view because I need to support both std::span<T> and std::string_view in my design). In the old code, I have only std::string_view support, so it has some code snippet like sv.substr(), now I would like sv could be general, which means it could be either std::string_view or std::span<T>(Span<T>).

Here is the minimal sample code I used to solve this issue:

#include <iostream>
#include <span>

using namespace std;

// Span class which is derived class from std::span
// add remove_prefix and substr function which is similar like std::string_view
template<typename T>
class Span : public std::span<T>
{
public:
    // Inheriting constructors from std::span
    using std::span<T>::span;

    // add a public function which is similar to std::string_view::remove_prefix
    constexpr void remove_prefix(std::size_t n) {
        auto& self = static_cast<std::span<T>&>(*this);
        self = self.subspan(n);
    }

    // add another public function substr, which is the span::subspan
    constexpr Span substr(std::size_t pos, std::size_t count){
        auto& self = static_cast<std::span<T>&>(*this);
        auto ret = self.subspan(pos, count);
        return static_cast<Span<T>&>(ret);
    }
};

float arr[3] = {1.0, 2.0, 3.0};

int main()
{
    Span<float> a(arr, 2);
    Span<float> b = a.substr(0, 1);
    return 0;
}

The above code builds and runs OK under g++ with C++20 support. My question is: is my implementation correct? Because I use so many static_cast in the code. In-fact, std::span already has a function named subspan, what I do is just make a function name alias substr.

Any ideas? Thanks.

ollydbg23
  • 1,124
  • 1
  • 12
  • 38

2 Answers2

2

Your implementation of remove_prefix isn't faulty, but it can be simplified to

constexpr void remove_prefix(std::size_t n) {
    *this = subspan(n);
}

Your implementation of substr is faulty, because you are static_casting an lvalue reference to an object who's most derived type is std::span<T>, not Span<T>. You can fix it:

constexpr Span substr(std::size_t pos, std::size_t count){
    return subspan(pos, count);
}

However, I wouldn't do this. I would instead change the calling code to use free functions that you've overloaded for std::span<T> and std::string_view (et. al.)

template<typename T>
constexpr void remove_prefix(std::span<T> & span, std::size_t n) {
    span = span.subspan(n);
}

template<typename CharT, typename Traits>
constexpr void remove_prefix(std::basic_string_view<CharT, Traits> & view, std::size_t n) {
    view.remove_prefix(n);
}

template<typename T>
constexpr std::span<T> substr(std::span<T> span, std::size_t n) {
    return span.subspan(n);
}

template<typename CharT, typename Traits>
constexpr std::basic_string_view<CharT, Traits> substr(std::basic_string_view<CharT, Traits> view, std::size_t n) {
    return view.substr(n);
}
Caleth
  • 52,200
  • 2
  • 44
  • 75
  • "I would instead change the calling code to use free functions that you've overloaded", thanks, I will try your methods. – ollydbg23 Feb 24 '23 at 12:58
  • Hi, Caleth, you method works correctly. There is one issue in your code, the `substr` need two arguments to locate a sub string or sub span, such as: `std::size_t pos, std::size_t count`, I have changed that in my local code, and it works well. Thanks for you help. – ollydbg23 Feb 25 '23 at 02:51
1

So, I think you should consider changing your design rather than changing STL.

First, it is unsafe to derive from std::span, because it does not have a virtual destructor, so there is a chance to leak a memory: Is it okay to inherit implementation from STL containers, rather than delegate?

Secondly, they are not virtual at all meaning that inheritance is used incorrectly, because, as it seems to me, its purpose is to create interfaces to override, so that details of modules can be hidden from each other.

It's hard to say what would I do without knowing your design, but there are some solutions I'd suggest:

  • Use polymorphism or explicit template specialization (with constexpr if) to define methods for both classes.
  • Create an interface for your concrete desing (solving your final goal) hiding the detail of containing one of this views.
  • You can construct std::span<const char> from std::string_view and use just one of them.
Adrian Mole
  • 49,934
  • 160
  • 51
  • 83
Georgij Krajnyukov
  • 21
  • 2
  • `std::span` isn't a container, so it's very unlikely anyone `delete`s one. You can even make sure by privately inheriting, and exposing all the members, to ensure no-one gets a `std::span *` pointing to a `Span` – Caleth Feb 24 '23 at 11:35
  • Hi, Georgij Krajnyukov, my final goal is to implement a PEG parser which can parse both `std::string_view` and a `std::span`, I have some demo code shown in https://stackoverflow.com/questions/75331349/what-is-the-way-to-remove-the-first-element-from-a-stdspant. Thanks for the help, I will read and consider your suggestion. – ollydbg23 Feb 24 '23 at 12:46
  • @Caleth, thanks for replying! Yeah, semantically `std::span` is a view, so I made a little correction to my answer. But I want to note that the first statement is correct for any type. It's a common practice to declare destructors virtual in polymorphic base classes. This is one of the topics in Scott Meyers' Effective C++. – Georgij Krajnyukov Feb 24 '23 at 14:12
  • Hi, Georgij Krajnyukov, I now mark Caleth's answer as the best answer, which uses free functions to handle the different types. But your answer was also very helpful, I mean I don't use my derived class `Span` from the `std::span` now, I just directly use the standard std container now. Thanks. – ollydbg23 Feb 25 '23 at 02:54