felipecrv commented on code in PR #33912:
URL: https://github.com/apache/arrow/pull/33912#discussion_r1091261046
##########
cpp/src/arrow/result.h:
##########
@@ -371,7 +371,7 @@ class [[nodiscard]] Result : public
util::EqualityComparable<Result<T>> {
if (ok()) {
Review Comment:
> > Additionally, I don't think this should be `G&&` as the intention here
is to let the compiler not even materialize `G` (e.g. by allocating an object
on the stack and calling some constructor for `G`), but instead just splice the
code from the lambda into the inlined code for this function.
>
> I'm not sure what you mean here. `G&&` is a [forwarding
reference](https://en.cppreference.com/w/cpp/language/reference#Forwarding_references)
according to the C++ reference.
Yes, but why do we want to forward `G` when we want the compiler to
completely inline the code it represents?
>
> > This is how predicates are passed to functions like `find_if` in the
standard library.
>
> Is this the actual declaration in a standard library implementation, or
the abstract signature as shown in the reference?
Yes. This is a common way of passing lambdas that you want to be completely
inlined if they are inline at the callsite.
See
https://github.com/llvm/llvm-project/blob/main/libcxx/include/__algorithm/find_if.h#L23
> (also, since `find_if` typically calls the predicate several times, it can
probably not `std::forward` it)
Again, I don't understand why we should forward it. We only call it, don't
store/pass it anywhere.
Another example from the STL is how `std::sort` inlines `Compare __c`
everywhere. If you pass a lambda it will inline the lambda, if you pass a
function pointer or an `std::function` it will invoke it by following the
pointer and so on.
https://github.com/llvm/llvm-project/blob/main/libcxx/include/__algorithm/sort.h#L266
--
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]
