size_type should be unsigned

[ascheglov]

GSL uses std::ptrdiff_t for size_type, which leads to the "signed/unsigned comparison" warnings.
It doesn't look like span::size can return negative values, so it should use std::size_t for size_type, as standard library containers do.

[neilmacintosh]

It is an explicit design decision for span to use std::ptrdiff_t as its size type.

A natural, portable implementation of span is as a pointer+length (or alternately, two pointers). The maximum addressable distance from a pointer can always be represented by std::ptrdiff_t so it is the correct choice.

std::ptrdiff_t is signed, but then so are nearly all integer literals and implicit conversion from signed literals to unsigned index types is a common source of error .

[KristofferHenriksson]

The maximum addressable distance can be signed but only because negative values are ok and wrapping is expected. Neither is true of the number of items in a collection where negative is a programming error and wrapping around can be catastrophic. You cannot address large numbers of items with a signed value.

The existing convention already set by std have containers return size_t for the count leading developers to naturally use size_t when indexing collections.

It sounds as this is already a done decision though which is too bad. Adopting GSL in our existing code would require lots of potentially unsafe casting and add extra complexity when indexing as one would have to carefully inspect code to see if the collection is a std one or a non-std collection with a different addressing.

[gdr-at-ms]

This is how I look at it: it is a fundamental mistake that the standard library uses an unsigned type as index type for operator[]. GSL fixes that by using signed integer type -- most literals are signed, even if you didn't write the - sign in front of them. Now, if you are going to iterate from 0 to the maximum number of element (a common operation), you don't want to hit the signed/unsigned comparison you speak of. Consequently, there is a practical advantage to having size() return a signed integer type. Furthermore, since the difference between two locations in an object has to be representable by ptrdiff_t, it means that it offers a superior engineering design, no matter what the standard containers are doing today.

Also, I personally find catching bugs related to signed -> unsigned implicit conversions coming from out-of-bound accesses much more important than a platonic uniformity with a standard container practice that we know are mistakes.

[ascheglov]

Certainly it is a mistake that the standard library uses size_t for size_type in containers. Unsigned numbers are good only for bit manipulations, or bigger numbers if you cannot use int64_t. Many real-life applications of size types work better with signed size types and signed index types, because int is the default type and almost everything else is signed. Even negative index or size values have their use in many APIs as special (error) values.

But we're writing C++ code, which heavily uses standard containers (or at least that's what we're supposed to do). And the standard containers use unsigned size_type. So we have to deal with those anyway, and the only way to it is to write static_cast<std::size_t>(span.size()):

std::string str;
gsl::span<char> span;
if (static_cast<std::size_t>(span.size()) > container.size()) ... // Compare sizes

std::string::size_type pos = str.find(...); // Can return npos witch is (size_type)-1
if (pos < static_cast<std::size_t>(span.size())) ... // Compare span size and string index

The code above is no good. It's not easy to read, it has long_ugly_casts, many braces, that static_cast<std::size_t> is just longer than the pos < span.size() expression. It doesn't look like a code that we want to enforce with introducing the C++ core guidelines.

What can we do?

It might not look good, but I think that we should add the usize() function, which would return unsigned size type.

class basic_span {
  // Use instead of static_cast<std::size_t>(span.size())
  std::make_unsigned_t<size_type> usize() const;
};

Yes it looks like a crutch and it is a crutch. That "u" in usize can stand for "ugly". But it would make the code more clean and readable. For a small price of learning "another size() function which should be used when you get a signed/unsigned warning".

[morrisonlevi]

You cannot address large numbers of items with a signed value.

If I am reading this GCC bug report correctly, GCC, glibc, and Bionic libc do not permit allocating more than half of the available address space in a single allocation. A significant reason seems to be that values in this range can cause various issues, some of which are security sensitive (just read the report; don't want to mis-summarize).

So it seems that this statement is, in practice, wrong anyway.

[KristofferHenriksson]

The maximum addressable distance can be signed but only because negative values are ok and wrapping is expected. Neither is true of the number of items in a collection where negative is a programming error and wrapping around can be catastrophic. You cannot address large numbers of items with a signed value.

The existing convention already set by std have containers return size_t for the count leading developers to naturally use size_t when indexing collections.

It sounds as this is already a done decision though which is too bad. Adopting GSL in our existing code would require lots of potentially unsafe casting and add extra complexity when indexing as one would have to carefully inspect code to see if the collection is a std one or a non-std collection with a different addressing.

[gdr-at-ms]

This is how I look at it: it is a fundamental mistake that the standard library uses an unsigned type as index type for operator[]. GSL fixes that by using signed integer type -- most literals are signed, even if you didn't write the - sign in front of them. Now, if you are going to iterate from 0 to the maximum number of element (a common operation), you don't want to hit the signed/unsigned comparison you speak of. Consequently, there is a practical advantage to having size() return a signed integer type. Furthermore, since the difference between two locations in an object has to be representable by ptrdiff_t, it means that it offers a superior engineering design, no matter what the standard containers are doing today.

Also, I personally find catching bugs related to signed -> unsigned implicit conversions coming from out-of-bound accesses much more important than a platonic uniformity with a standard container practice that we know are mistakes.

[ascheglov]

Certainly it is a mistake that the standard library uses size_t for size_type in containers. Unsigned numbers are good only for bit manipulations, or bigger numbers if you cannot use int64_t. Many real-life applications of size types work better with signed size types and signed index types, because int is the default type and almost everything else is signed. Even negative index or size values have their use in many APIs as special (error) values.

But we're writing C++ code, which heavily uses standard containers (or at least that's what we're supposed to do). And the standard containers use unsigned size_type. So we have to deal with those anyway, and the only way to it is to write static_cast<std::size_t>(span.size()):

std::string str;
gsl::span<char> span;
if (static_cast<std::size_t>(span.size()) > container.size()) ... // Compare sizes

std::string::size_type pos = str.find(...); // Can return npos witch is (size_type)-1
if (pos < static_cast<std::size_t>(span.size())) ... // Compare span size and string index

The code above is no good. It's not easy to read, it has long_ugly_casts, many braces, that static_cast<std::size_t> is just longer than the pos < span.size() expression. It doesn't look like a code that we want to enforce with introducing the C++ core guidelines.

What can we do?

It might not look good, but I think that we should add the usize() function, which would return unsigned size type.

class basic_span {
  // Use instead of static_cast<std::size_t>(span.size())
  std::make_unsigned_t<size_type> usize() const;
};

Yes it looks like a crutch and it is a crutch. That "u" in usize can stand for "ugly". But it would make the code more clean and readable. For a small price of learning "another size() function which should be used when you get a signed/unsigned warning".

[morrisonlevi]

You cannot address large numbers of items with a signed value.

If I am reading this GCC bug report correctly, GCC, glibc, and Bionic libc do not permit allocating more than half of the available address space in a single allocation. A significant reason seems to be that values in this range can cause various issues, some of which are security sensitive (just read the report; don't want to mis-summarize).

So it seems that this statement is, in practice, wrong anyway.

[adah1972]

I agree with @ascheglov . The decision to use std::ptrdiff_t as the return type of gsl::span makes the code uglier, not better. Practically, it forces me to static_cast all over the places!

STL may have made a mistake in using size_t, but you are making the reality worse!

[adah1972]

Anyway, it is unnecessary to argue anymore. I have just found out that GSL has changed size_type to size_t, in accordance with std::span of C++20.