Conversation
e98f1b7 to
a98849c
Compare
pablomartinezbernardo
left a comment
There was a problem hiding this comment.
Small comment, LGTM
| bool starts_with(StringView subject, StringView prefix) { | ||
| if (prefix.size() > subject.size()) { | ||
| return false; | ||
| auto s = subject.data(); | ||
| auto p = prefix.data(); | ||
| const auto prefix_end = p + prefix.size(); | ||
| while (*s == *p) { | ||
| ++s; | ||
| ++p; | ||
| } | ||
|
|
||
| return std::mismatch(subject.begin(), subject.end(), prefix.begin()).second == | ||
| prefix.end(); | ||
| return p == prefix_end; | ||
| } |
There was a problem hiding this comment.
This will cause problems when subject is equal to prefix, right? Not that important, but should also not be difficult to fix
There was a problem hiding this comment.
There's nothing stopping s and p from going over the end of the string_view, so it would be undefined behavior, but even if nothing breaks the condition p == prefix_end will be false. Did a small example to reproduce it
To avoid that, the function could be written like this for example
bool starts_with(StringView subject, StringView prefix) {
auto s = subject.data();
auto p = prefix.data();
const auto prefix_end = p + prefix.size();
while (p < prefix_end) {
if (*s++ != *p++) {
return false;
}
}
return true;
}There was a problem hiding this comment.
There's nothing stopping s and p from going over the end of the string_view, so it would be undefined behavior
Indeed, thanks to Github awful diff I thought if (prefix.size() > subject.size()) { ... } was part of the implementation, which reduce the area of the UB. However, as you mentioned, if prefix.size() == subject.size() then it's undefined behaviour and plain wrong. Thank you.
- Fix implementation + add unit tests.
- refactor: the codebase to use `substr` instead of `range`. - refactor: CMake targets. - refactor: add bazelrc to build using either abseil or std::string STD. - ci: build and run on windows using CMake and bazel. - update: add platform support section and update building instructions.
a98849c to
d7bb00e
Compare
Description
This PR add support for windows. It is a prerequisite for #85.
Contains:
substrinstead ofrange.