taepper commented on PR #53314: URL: https://github.com/apache/arrow/pull/53314#issuecomment-6141482687
> In the first code snippet, the string_view references a temporary value created via extended syntax, resulting in a dangling reference. I think you are correct that our Warnings do not currently cover this case explicitly: ``` /// WARNING: ARROW_ASSIGN_OR_RAISE `std::move`s its right operand. If you have /// an lvalue Result which you *don't* want to move out of cast appropriately. /// /// WARNING: ARROW_ASSIGN_OR_RAISE is not a single expression; it will not /// maintain lifetimes of all temporaries in `rexpr` (e.g. /// `ARROW_ASSIGN_OR_RAISE(auto x, MakeTemp().GetResultRef());` /// will most likely segfault)! ``` If I understand correctly, the issue arises in your example, because the `std::move(rexpr)` cannot move the `Result<std::string>` and will therefore copy the string to the temporary which will not outlive the statement expression. IMO this is implicitly covered by this statement > ARROW_ASSIGN_OR_RAISE is not a single expression; it will not maintain lifetimes of all temporaries in `rexpr` Because the caller provides a `const Result` it is not unexpected that the evaluation will perform a copy of `rexpr` to a temporary. And the doc already warns regarding this temporary's lifetime -- 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]
