Skip to content

Commit

Permalink
GH-33911: [C++] Add missing std::forward to Result::ValueOrElse (#33912)
Browse files Browse the repository at this point in the history
It fixes #33911.

------

From #33911:

> https://github.com/apache/arrow/blob/4f1d255f3dc57457e5c78d98c4b76fc523cf9961/cpp/src/arrow/result.h#L369-L375
>
> The parameter `generate_alternative` is passed by univeral forwarding, but `std::forward` is missing to use it.
>
> This may result in incorrect overloading selection or compilation errors, such as a class type with the method `operator()() &&`.
* Closes: #33911

Lead-authored-by: PragmaTwice <twice.mliu@gmail.com>
Co-authored-by: Twice <twice@apache.org>
Signed-off-by: Antoine Pitrou <antoine@python.org>
  • Loading branch information
PragmaTwice committed Jan 31, 2023
1 parent dd80f85 commit 7a3c66a
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 1 deletion.
2 changes: 1 addition & 1 deletion cpp/src/arrow/result.h
Original file line number Diff line number Diff line change
Expand Up @@ -371,7 +371,7 @@ class [[nodiscard]] Result : public util::EqualityComparable<Result<T>> {
if (ok()) {
return MoveValueUnsafe();
}
return generate_alternative();
return std::forward<G>(generate_alternative)();
}

/// Apply a function to the internally stored value to produce a new result or propagate
Expand Down
10 changes: 10 additions & 0 deletions cpp/src/arrow/result_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
#include "arrow/testing/gtest_compat.h"
#include "arrow/testing/gtest_util.h"
#include "arrow/testing/matchers.h"
#include "arrow/util/functional.h"

namespace arrow {

Expand Down Expand Up @@ -795,5 +796,14 @@ TEST(ResultTest, MatcherExplanations) {
}
}

TEST(ResultTest, ValueOrGeneratedMoveOnlyGenerator) {
Result<MoveOnlyDataType> result = Status::Invalid("");
internal::FnOnce<MoveOnlyDataType()> alternative_generator = [] {
return MoveOnlyDataType{kIntElement};
};
auto out = std::move(result).ValueOrElse(std::move(alternative_generator));
EXPECT_EQ(*out.data, kIntElement);
}

} // namespace
} // namespace arrow

0 comments on commit 7a3c66a

Please sign in to comment.