index_type ptr_diff

[stayprivates]

I glanced at p0122r6.pdf and could not find an answer. size() returns a type index_type which is a ptrdiff_t. ptrdiff_t is signed, so for code that does something similar to:

if ( std::vector::size() != std::span::size() )

I will get a compiler warning about comparing signed and unsigned. I do not understand why a span size could be negative.

What is the rational for using ptrdiff_t here? Given that some of span's constructor can accept size_t that means one could create a span with a size bigger than what span::size() could actually return ?

I understand this has nothing to do with your implementation but was wondering if you had any comment about this. I assume usage of ptrdiff_t is required by math is performed on pointers?

[martinmoene]

The historical choice of using an unsigned type for indices is regretted by some/many people working on/with C++. Changing to signed type is considered controversial by some/many working on/with C++.

Several references I could find:

• p0122, latest: span: bounds safe views for sequences of objects , page 7:

The type used for measuring and indexing into span is ptrdiff_t. Using a signed index type helps avoid common mistakes that come from implicit signed to unsigned integer conversions when users employ integer literals (which are nearly always signed). The use of ptrdiff_t is natural as it is the type used for pointer arithmetic and array indexing – two operations that span explicitly aims to replace but that an implementation of span would likely rely upon.

• p1089 - Sizes Should Only span Unsigned.

• M-GSL issue 339: span index is an int instead of unsigned int.

• M-GSL issue 322: size of span, signed versus unsigned.

• M-GSL issue 264: size() should be size_t.

• M-GSL issue 238: What is the rationnal for using signed integer as size ?

• M-GSL issue 171: size_type should be unsigned.

[grandseiken]

@martinmoene https://www.reddit.com/r/cpp/comments/9vwvbz/2018_san_diego_iso_c_committee_trip_report_ranges/ reports that size() will be size_t in C++20 (i.e. p1089r2 got accepted). Is that enough to make the change in this library?

[martinmoene]

@grandseiken Thanks!

The idea is to follow the (evolving) standard, so I'll revisit the code in light of what will/has become of C++20.

---

NTS: perhaps provide a configuration macro for backwards compatibility.

[martinmoene]

See also issue #27.

Plan:

• [x] Add typedef std::ptrdiff_t extent_t;
• [x] Add typedef extent_t extent_type; to class span
• [x] Change index_t Extent to extent_t Extent
• [x] Update locations for extent_t, extent_type
• [x] Add to_size() where necessary
• [x] Add configuration macro span_CONFIG_INDEX_TYPE with default std::ptrdiff_t
• [ ] Change configuration macro span_CONFIG_INDEX_TYPE default to std::size_t (later)
• [x] Update Readme
• [ ] ....

What to change subspan(offset, count) to with dynamic_extent = -1?

constexpr span<element_type, dynamic_extent>
subspan(index_type offset, index_type count = dynamic_extent) const;

constexpr span<element_type, dynamic_extent>
subspan(index_type offset, index_type count = static_cast<index_type>(dynamic_extent)) const;

[ululi1970]

I think this is related to this issue When compiling code containing nonstd::span with the CUDA compiler( nvcc), I get a 'Warning: integer conversion resulted in a change of sign" at lines 722, 737 and 754. The lines that trigger the warning are all the same

(Extent == dynamic_extent || Extent == N)

Here, Extent and dynamic_extent are of type std::ptrdiff_t, whereas N is site_t. Upcasting N to std::ptrdiff_t takes care of the warning, but I just wanted to make sure that it does not break the intended behavior...

[martinmoene]

Thanks @ululi1970 , I've added casts to size_t N: Extent == N is now Extent == static_cast<extent_t>(N).

[ululi1970]

Thanks to you for sharing this nice implementation of span.

On Fri, Feb 1, 2019 at 6:28 PM Martin Moene notifications@github.com wrote:

Thanks @ululi1970 https://github.com/ululi1970 , I've added casts to size_t N: Extent == N is now Extent == static_cast(N).

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/martinmoene/span-lite/issues/21#issuecomment-459903347, or mute the thread https://github.com/notifications/unsubscribe-auth/AIDOY0O1kGOwep0QIy4PhooWbQXg7EPRks5vJM2dgaJpZM4ULJHz .