igor-anferov opened a new pull request, #44477:
URL: https://github.com/apache/arrow/pull/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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
