[STLForwardCompat] Improve category handling in transformOptional #149539
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The old version would prefer the "const &" overload over the "&&" one unless the former was not allowed in the given situation. In particular, if the function passed was "[](auto &&)" the argument would be "const &" even if the value passed to transformOptional was an rvalue reference. This version improves the handling of expression categories, and the lambda argument category will reflect the argument category in the above scenario.
|
@llvm/pr-subscribers-llvm-adt Author: Krzysztof Parzyszek (kparzysz) ChangesThe old version would prefer the "const &" overload over the "&&" one unless the former was not allowed in the given situation. In particular, if the function passed was "[](auto &&)" the argument would be "const &" even if the value passed to transformOptional was an rvalue reference. This version improves the handling of expression categories, and the lambda argument category will reflect the argument category in the above scenario. Full diff: https://github.com/llvm/llvm-project/pull/149539.diff 2 Files Affected:
diff --git a/llvm/include/llvm/ADT/STLForwardCompat.h b/llvm/include/llvm/ADT/STLForwardCompat.h
index 7bd2c8705f393..c4bb5cd713139 100644
--- a/llvm/include/llvm/ADT/STLForwardCompat.h
+++ b/llvm/include/llvm/ADT/STLForwardCompat.h
@@ -55,21 +55,13 @@ using type_identity_t // NOLINT(readability-identifier-naming)
// TODO: Remove this in favor of std::optional<T>::transform once we switch to
// C++23.
-template <typename T, typename Function>
-auto transformOptional(const std::optional<T> &O, const Function &F)
- -> std::optional<decltype(F(*O))> {
- if (O)
- return F(*O);
- return std::nullopt;
-}
-
-// TODO: Remove this in favor of std::optional<T>::transform once we switch to
-// C++23.
-template <typename T, typename Function>
-auto transformOptional(std::optional<T> &&O, const Function &F)
- -> std::optional<decltype(F(*std::move(O)))> {
- if (O)
- return F(*std::move(O));
+template <typename Optional, typename Function,
+ typename Value = typename llvm::remove_cvref_t<Optional>::value_type>
+auto transformOptional(Optional &&O, Function &&F)
+ -> std::optional<std::invoke_result_t<Function, Value>> {
+ if (O) {
+ return F(*std::forward<Optional>(O));
+ }
return std::nullopt;
}
diff --git a/llvm/unittests/ADT/STLForwardCompatTest.cpp b/llvm/unittests/ADT/STLForwardCompatTest.cpp
index e3d500aa7b55a..b8b2ca1c85054 100644
--- a/llvm/unittests/ADT/STLForwardCompatTest.cpp
+++ b/llvm/unittests/ADT/STLForwardCompatTest.cpp
@@ -10,6 +10,11 @@
#include "CountCopyAndMove.h"
#include "gtest/gtest.h"
+#include <optional>
+#include <tuple>
+#include <type_traits>
+#include <utility>
+
namespace {
template <typename T>
@@ -142,6 +147,30 @@ TEST(TransformTest, MoveTransformLlvm) {
EXPECT_EQ(0, CountCopyAndMove::Destructions);
}
+template <typename T> constexpr bool IsRvalueReference(T &&arg) {
+ return std::is_rvalue_reference_v<decltype(arg)>;
+}
+
+TEST(TransformTest, TransformCategory) {
+ struct StructA {
+ int x;
+ };
+ struct StructB : StructA {
+ StructB(StructA &&A) : StructA(std::move(A)) {}
+ };
+
+ std::optional<StructA> A{StructA{}};
+ llvm::transformOptional(A, [](auto &&s) {
+ EXPECT_FALSE(std::is_rvalue_reference_v<decltype(s)>);
+ return StructB{std::move(s)};
+ });
+
+ llvm::transformOptional(std::move(A), [](auto &&s) {
+ EXPECT_TRUE(std::is_rvalue_reference_v<decltype(s)>);
+ return StructB{std::move(s)};
+ });
+}
+
TEST(TransformTest, ToUnderlying) {
enum E { A1 = 0, B1 = -1 };
static_assert(llvm::to_underlying(A1) == 0);
|
kuhar
approved these changes
Jul 18, 2025
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/27/builds/13293 Here is the relevant piece of the build log for the reference |
mdfazlay
pushed a commit
to mdfazlay/llvm-project
that referenced
this pull request
Jul 18, 2025
…vm#149539) The old version would prefer the "const &" overload over the "&&" one unless the former was not allowed in the given situation. In particular, if the function passed was "[](auto &&)" the argument would be "const &" even if the value passed to transformOptional was an rvalue reference. This version improves the handling of expression categories, and the lambda argument category will reflect the argument category in the above scenario.
This was referenced Jul 23, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The old version would prefer the "const &" overload over the "&&" one unless the former was not allowed in the given situation. In particular, if the function passed was "[](auto &&)" the argument would be "const &" even if the value passed to transformOptional was an rvalue reference.
This version improves the handling of expression categories, and the lambda argument category will reflect the argument category in the above scenario.