martinmoene / span-lite

span lite - A C++20-like span for C++98, C++11 and later in a single-file header-only library
Boost Software License 1.0
495 stars 40 forks source link

extent_t is ptrdiff_t, which ruins SFINAE for people expecting std::size_t #45

Closed ThePhD closed 4 years ago

ThePhD commented 4 years ago

I wrote a template

template <typename T, std::size_t Extent>
void process(span<T, Extent> ...) {

}

This didn't work properly, because extent_t in span-lite is ptrdiff_t. Can I change this to std::size_t and make a PR, or is there a specific reason span-lite made this choice?

martinmoene commented 4 years ago

Related to Review span in face of p1024 and p1089 (Continued from #14).

It's so due to span's history (see e.g. p0122r1).

Currently, cppreference says:

inline constexpr std::size_t dynamic_extent = std::numeric_limits<std::size_t>::max(); (since C++20)

and:

Since std::size_t is an unsigned type, an equivalent definition is:

inline constexpr std::size_t dynamic_extent = -1;

So it seems like a good idea to change extent_t to be std::size_t.

I'll try and update span lite in this respect.

ThePhD commented 4 years ago

I updated it for you. Let me know if anything is wrong. :D

martinmoene commented 4 years ago

Thanks @ThePhD,

In the end the changes you offered in PR #46 were handled in 54ae6bac45e7c6afe2996e9877ff7ead214a8c61, 9becd6980fa74c3f39e95d2dee472d4573b91d56 (extent) and in 0c95c2994823a2ee43c2b476af2ac5e74ab4fff1 (index_type -> size_type) and 17abefb0ad0c9c02df3e243fecca62462c941a7b (change default type to std::size_t).

ThePhD commented 4 years ago

Yaaay! 🎉

Thanks for doing this so quickly. It certainly covers all of my use cases. I appreciate it! ❤️

AntoinePrv commented 4 years ago

Hi, thank you for this project! I would be grateful if we could see this change in a new release.

martinmoene commented 4 years ago

Released v0.7.0.