[clang-tools-extra] [libc++] Prevent calling the projection more than three times (PR #66315)
Louis Dionne via cfe-commits
cfe-commits at lists.llvm.org
Fri Sep 15 11:06:03 PDT 2023
================
@@ -37,9 +37,10 @@ struct __fn {
_LIBCPP_ASSERT_UNCATEGORIZED(!bool(std::invoke(__comp, std::invoke(__proj, __high), std::invoke(__proj, __low))),
"Bad bounds passed to std::ranges::clamp");
- if (std::invoke(__comp, std::invoke(__proj, __value), std::invoke(__proj, __low)))
+ auto&& __projected = std::invoke(__proj, __value);
+ if (std::invoke(__comp, std::forward<decltype(__projected)>(__projected), std::invoke(__proj, __low)))
return __low;
- else if (std::invoke(__comp, std::invoke(__proj, __high), std::invoke(__proj, __value)))
+ else if (std::invoke(__comp, std::invoke(__proj, __high), std::forward<decltype(__projected)>(__projected))
----------------
ldionne wrote:
There is actually a problem here. We can potentially do a double-move. Imagine we have the following comparator:
```
struct Foo {
std::string s;
};
// taking by value is important here
auto comparator = [](std::string a, std::string b) {
return std::atoi(a.c_str()) < std::atoi(b.c_str());
};
auto projection = [](Foo const& foo) {
return foo.s;
};
Foo foo{"12"};
Foo high{"10"};
Foo low{"1"};
std::ranges::clamp(foo, low, high, comparator, projection);
```
Here, we end up calling `std::invoke(comp, forward<...>(projected), low)`, it returns false and then we call `std::invoke(comp, forward<...>(projected), high)`. We end up moving `projected` twice into the `invoke` calls, so the second time we get an empty string.
Is that right? If so, this would need a test too.
https://github.com/llvm/llvm-project/pull/66315
More information about the cfe-commits
mailing list