Commits


Igor Anferov authored and GitHub committed 6decf1cca07
GH-44464: [C++] Added rvalue-reference-qualified overload for arrow::Result::status() returning value instead of reference (#44477) ### Rationale for this change In the current implementation, `arrow::Result::status()` always returns the internal `status_` field by a const lvalue reference, regardless of the value category of `Result`. This can lead to potential bugs. For example, consider the following code: ```c++ if (auto&& status = functionReturningArrowResult().status(); status.ok()) return 0; return -1; ``` In this case, the call to `status.ok()` results in undefined behavior because `status` is a dangling const lvalue reference that points to an object returned by `functionReturningArrowResult()`, which is destroyed after the semicolon. If `arrow::Result` had two overloads of the `status()` method for different reference qualifiers: ```c++ template <…> class Result { … auto status() const & -> const Status& { ... } auto status() && -> Status { ... } … }; ``` This would prevent such bugs and potentially allow for better optimization, as the `Status` could be moved from an expiring `Result` object. ### What changes are included in this PR? This PR adds the proposed overload for the `arrow::Result::status()` method and makes other rvalue-qualified `arrow::Result` methods preserve object ref-category during tail `status()` calls. Unfortunately, we can't move the `status_` field in the rvalue-qualified `status()` method, as the state of `status_` must be preserved until the destructor is called. This is because the `storage_` field is either destructed or considered empty based on the state of `status_`. ### Are these changes tested? Since this change is trivial (the new overload doesn't modify the `Result` object and returns `Status` by value), there's nothing significant to test, so no new tests were added. ### Are there any user-facing changes? No existing code will be broken by this change. In all cases where `status()` is called on an lvalue `Result`, the same reference-returning overload will be called. Meanwhile, code calling `status()` on an rvalue `Result` will invoke the new overload, returning `Status` by value instead. * GitHub Issue: #44464 Authored-by: igor-anferov <igor-anferov@mail.ru> Signed-off-by: mwish <maplewish117@gmail.com>