================ @@ -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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits