createcentury commented on PR #53314:
URL: https://github.com/apache/arrow/pull/53314#issuecomment-6128058509
The gcc/clang specialization looks good to me.
I checked the two things this expansion could have quietly broken, and both
come out clean:
- **Shadowing.** Moving `lhs` to the outside of the statement expression
changes name
lookup inside the initializer, which is what the `file_ipc.cc` rename is
about. I went
through the rest of `cpp/src` for the same pattern and that is the only
site, so nothing
else needs renaming.
- **Lifetime.** The intermediate `Result` now dies at the end of the
statement rather than
the enclosing scope. Nothing binds it by reference — there are no `auto&&`
or `auto&` left
hand sides — and the `std::string_view` ones view into storage owned
elsewhere, so they're
unaffected.
clang at `-O0` stays where it was, as your table shows, but killing the
growth in gcc `-O0`
and in both release builds covers most of the pain.
--
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]
