Hacker News new | ask | show | jobs
by e____g 891 days ago
I saw std::function and std::string (e.g. TotW 127, https://abseil.io/tips/117) being passed by value a lot in newer google3 code. Both are larger than 16 bytes.
5 comments

This is done so you can use std::move to take ownership of the allocated memory in these objects rather than do a new allocation. Passing by value rather than rvalue reference let's your function be more flexible at the call site. You can pass an rvalue/move or just make a copy at the call site which means the caller (who actually knows if a copy or a move is more appropriate) gets to control how the memory gets allocated.

An unnecessary memory allocation is much more of a performance hit than suboptimal calling convention.

In that case, the optimal interface should take std::string&& no? But it's awkward.
The vanilla interface often allows what's right depending on the context (e.g., if an unnamed return value is passed in, it can be move constructed, otherwise it can be copied). I saw some code bases provide an override for a const ref and a normal type.
Wouldn't this be very annoying to work with, because now you have to explicitly move or copy the string whenever you want to construct one of these objects?
It's kinda what Rust forces you to do, except that std::move is implied. Anything taken by value is equivalent to taking by && unless the type is explicitly marked as Copy (i.e. it can be trivially copied and the copies are implicit).

But yeah, in a c++ codebase, good modern practices are often verbose and clunky.

The function could accept an universal reference instead of an rvalue reference, this avoids the dance the caller has to do to pass a copy.

IMO it's hard to beat pass by value considering both performance and cognitive load.

Yeah, making it accept a universal reference would fix it...

...but that requires the argument to be a type from a template D: so you'd have to write:

    template<typename String = std::string>
    void dosomething(String &&str)
...and that's not quite right either, since you'd want it to be either an rvalue reference or a const lvalue reference
Explicity having to copy or move is a desired coding style IMO.
Passing std::function by value is almost definitely wrong these days with absl::AnyInvocable (if you need to store the type) and absl::FunctionRef (if you don't). Rough analogues in the standard are std::move_only_function (C++23) and std::function_ref (C++26).

std::string in the case you cited is only really relevant if you std::move() into it and you would otherwise incur a string copy. Yes, it's bigger than 16 bytes (24 bytes), but that pales in comparison to the alternative.

(Taking std::string&& would eliminate the possibility of misuse / accidental copies, but that pattern is generally discouraged by Google's style guide for various reasons.)

Also, just because you see a certain pattern an awful lot even at Google doesn't mean that it's best practice -- there are plenty of instances of protobufs being passed by value...

Please prove me wrong on these points. My current belief and understanding is that:

1. `foo(T)` indicates polymorphic over both `T::T(const T&)` and `T::T(T&&)`. This gives you benefits of both pass-by-move (using `std::move` as needed), or copy.

2. Usage of `foo(T&&)` signals code-smell or an anti-pattern as `foo(T)` should be used instead unless it is perfecting forwarding / universal reference `template <typename T> foo(T&&)`.

Google builds non-PIE, non-PIC, static, profile-guided, link-time-optimized, and post-link-optimized binaries and probably DGAF about calling conventions.
I have seen the assembly output of Google code, and I will say that my previous comment still stands.
not trivially copyable types are never passed in registers regardless of size