Skip to content

Added comment about narrow contract for Span(T* begin, T* end) ctor#22291

Closed
ViralTaco wants to merge 1 commit intobitcoin:masterfrom
ViralTaco:master
Closed

Added comment about narrow contract for Span(T* begin, T* end) ctor#22291
ViralTaco wants to merge 1 commit intobitcoin:masterfrom
ViralTaco:master

Conversation

@ViralTaco
Copy link
Copy Markdown

Hi,
Following issue: #22289
It's completely overkill, even though it's just a comment. But it's what it'll take for me to sleep tonight.
I've only added a comment to clarrify that this constructor comes with a narrow contract.

@fanquake fanquake added the Docs label Jun 21, 2021
@laanwj
Copy link
Copy Markdown
Member

laanwj commented Sep 16, 2021

~0 ACK 39cb974
This comment is correct, however I don't know what else one would expect when passing an invalid range into this function.

@ViralTaco
Copy link
Copy Markdown
Author

I don't know what else one would expect when passing an invalid range into this function.

Defined behavior?

@maflcko
Copy link
Copy Markdown
Member

maflcko commented Oct 20, 2021

The docs say that this is UB, so why would anyone assume the opposite?

https://en.cppreference.com/w/cpp/container/span/span

The behavior is undefined if [first, first + count) is not a valid range, if It does not actually model contiguous_iterator, or if extent != std::dynamic_extent && count != extent.

@ViralTaco
Copy link
Copy Markdown
Author

The docs say that this is UB, so why would anyone assume the opposite?

https://en.cppreference.com/w/cpp/container/span/span

The behavior is undefined if [first, first + count) is not a valid range, if It does not actually model contiguous_iterator, or if extent != std::dynamic_extent && count != extent.

Following issue: #22289

@fanquake
Copy link
Copy Markdown
Member

Going to close this for now, given #22289 is also closed.

@fanquake fanquake closed this Mar 24, 2022
@bitcoin bitcoin locked and limited conversation to collaborators Mar 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants