Skip to content

Commit

Permalink
let Range members take values by const&
Browse files Browse the repository at this point in the history
Summary:
Various member functions, including `find` and `starts_with`, accept a `value_type` argument. But this interface is dangerous since it is possible for `value_type` to be non-copyable or to be expensively-copyable.

We change these members to take `value_type const&` but are careful not to let them take `const_reference` since the latter may not be the same as `value_type const&` or convertible from `value_type`.

Reviewed By: luciang, vitaut

Differential Revision: D60537917

fbshipit-source-id: f24dc9f41b3ef5ef4b3461e502612bb4e349af36
  • Loading branch information
yfeldblum authored and facebook-github-bot committed Aug 1, 2024
1 parent 906b45a commit be091f7
Showing 1 changed file with 18 additions and 13 deletions.
31 changes: 18 additions & 13 deletions folly/Range.h
Original file line number Diff line number Diff line change
Expand Up @@ -903,11 +903,13 @@ class Range {
return ret == npos ? ret : ret + pos;
}

size_type find(value_type c) const { return qfind(castToConst(), c); }
size_type find(const value_type& c) const { return qfind(castToConst(), c); }

size_type rfind(value_type c) const { return folly::rfind(castToConst(), c); }
size_type rfind(const value_type& c) const {
return folly::rfind(castToConst(), c);
}

size_type find(value_type c, size_t pos) const {
size_type find(const value_type& c, size_t pos) const {
if (pos > size()) {
return std::string::npos;
}
Expand Down Expand Up @@ -941,9 +943,9 @@ class Range {
return find_first_of(const_range_type(needles, n), pos);
}

size_type find_first_of(value_type c) const { return find(c); }
size_type find_first_of(const value_type& c) const { return find(c); }

size_type find_first_of(value_type c, size_t pos) const {
size_type find_first_of(const value_type& c, size_t pos) const {
return find(c, pos);
}

Expand Down Expand Up @@ -972,7 +974,9 @@ class Range {
return size() >= other.size() &&
castToConst().subpiece(0, other.size()) == other;
}
bool startsWith(value_type c) const { return !empty() && front() == c; }
bool startsWith(const value_type& c) const {
return !empty() && front() == c;
}

template <class Comp>
bool startsWith(const const_range_type& other, Comp&& eq) const {
Expand All @@ -987,7 +991,7 @@ class Range {
bool starts_with(const_range_type other) const noexcept {
return startsWith(other);
}
bool starts_with(value_type c) const noexcept { return startsWith(c); }
bool starts_with(const value_type& c) const noexcept { return startsWith(c); }
template <
typename...,
typename T = Iter,
Expand All @@ -1003,7 +1007,7 @@ class Range {
return size() >= other.size() &&
castToConst().subpiece(size() - other.size()) == other;
}
bool endsWith(value_type c) const { return !empty() && back() == c; }
bool endsWith(const value_type& c) const { return !empty() && back() == c; }

template <class Comp>
bool endsWith(const const_range_type& other, Comp&& eq) const {
Expand All @@ -1024,7 +1028,7 @@ class Range {
bool ends_with(const_range_type other) const noexcept {
return endsWith(other);
}
bool ends_with(value_type c) const noexcept { return endsWith(c); }
bool ends_with(const value_type& c) const noexcept { return endsWith(c); }
template <
typename...,
typename T = Iter,
Expand Down Expand Up @@ -1056,7 +1060,7 @@ class Range {
bool removePrefix(const const_range_type& prefix) {
return startsWith(prefix) && (b_ += prefix.size(), true);
}
bool removePrefix(value_type prefix) {
bool removePrefix(const value_type& prefix) {
return startsWith(prefix) && (++b_, true);
}

Expand All @@ -1067,7 +1071,7 @@ class Range {
bool removeSuffix(const const_range_type& suffix) {
return endsWith(suffix) && (e_ -= suffix.size(), true);
}
bool removeSuffix(value_type suffix) {
bool removeSuffix(const value_type& suffix) {
return endsWith(suffix) && (--e_, true);
}

Expand Down Expand Up @@ -1160,7 +1164,7 @@ class Range {
* }
*
*/
Range split_step(value_type delimiter) {
Range split_step(const value_type& delimiter) {
auto i = find(delimiter);
Range result(b_, i == std::string::npos ? size() : i);

Expand Down Expand Up @@ -1242,7 +1246,8 @@ class Range {
*
*/
template <typename TProcess, typename... Args>
auto split_step(value_type delimiter, TProcess&& process, Args&&... args)
auto split_step(
const value_type& delimiter, TProcess&& process, Args&&... args)
-> decltype(process(std::declval<Range>(), std::forward<Args>(args)...)) {
return process(split_step(delimiter), std::forward<Args>(args)...);
}
Expand Down

0 comments on commit be091f7

Please sign in to comment.