Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

basic_string_view begins_with has an incorrect reference implementation #1870

Closed
cscrimge opened this issue Dec 1, 2017 · 5 comments
Closed

basic_string_view begins_with has an incorrect reference implementation #1870

cscrimge opened this issue Dec 1, 2017 · 5 comments
Labels
lwg Issue must be reviewed by LWG.

Comments

@cscrimge
Copy link

cscrimge commented Dec 1, 2017 

constexpr bool starts_with(basic_string_view x) const noexcept;
    Effects: Equivalent to: return compare(0, npos, x) == 0;

compare(0, npos, x) should be: compare(0, x.size(), x)

Otherwise "Hello World"sv.starts_with("Hello"sv) would return false.
@jwakely
Copy link
Member

jwakely commented Dec 1, 2017

This is not an editorial issue, so needs to be submitted as a defect report.

I'm pretty sure @mclow already noticed this problem, and might have opened an issue already.

@mclow
Copy link
Contributor

mclow commented Dec 1, 2017

I have written up the issue, but haven't added to the issues list. That will happen by Monday.

BTW, your proposed resolution is incorrect, because in the case "Hello"sv.starts_with("Hello World"sv) you would have undefined behavior.

@cscrimge
Copy link
Author

cscrimge commented Dec 1, 2017

You are right. I was assuming that substr(0, n) would clamp to size(), which is true for basic_string but not basic_string_view.

So the proposed resolution should be return (size() >= x.size()) && (compare(0, x.size(), x) == 0).

@jensmaurer jensmaurer added the lwg Issue must be reviewed by LWG. label Dec 1, 2017
@mclow
Copy link
Contributor

mclow commented Dec 1, 2017

Which is exactly the proposed resolution in my LWG issue, and in my patch for libc++: https://reviews.llvm.org/D40586

@mclow
Copy link
Contributor

mclow commented Dec 2, 2017

https://cplusplus.github.io/LWG/issue3040

@cpplearner cpplearner mentioned this issue May 26, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
lwg Issue must be reviewed by LWG.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@mclow @jwakely @cscrimge @jensmaurer