On 30/03/2017 11:59, Evgeny Fraimovitch wrote:
Although the code is suboptimal, I do not agree it is bad. This lack of pass by reference in this case should not be affecting correctness, since we are not going to modify the object. Also, it might not be std::string. Shared points are often passed by value and they exhibit the same issue. They can be passed as const reference too, as an optimization, however whether this justifies additional clutter in the parameter list is a matter of personal taste.
Perhaps this is a domain-specific concern, but where I work you get severely beaten* if you pass strings *or* shared_ptrs by value. (Or in general anything with non-trivial copy semantics.) (* or at least mocked at the water cooler and code reviews) Neither memory allocations nor atomic refcount adjustments are free, and the function call boundary is rarely the correct place for them to occur. But yes, ideally this would produce correct output despite being suboptimal. It's one of the unfortunate side effects of perfect forwarding. Having said that, with the existence of perfect forwarding it means you're using C++11, which means you could use lambdas instead of using bind. They don't suffer the same issue because they don't use perfect forwarding: auto triple = [](std::string x) { f(x, x, x); }; auto triple = [](std::string const& x) { f(x, x, x); }; auto triple = [](std::string && x) { f(x, x, x); }; Substituting any of these produces correct output; you would only provoke the incorrect output if you explicitly std::move(x) in the last version, which is an obvious programmer error. It can be most problematic for large codebases with existing pre-C++11 bind code, of course, that is later updated to C++11. Hopefully you have unit tests to catch these sorts of things. :) There's some other corner cases with bind that are a bit weird, for example, given: void g(std::string&& x) { std::string y(std::move(x)); std::cout << y << std::endl; } (which is even worse than pass-by-value in this case, of course.) This compiles and works as expected: auto fn = boost::bind(g, _1); fn(std::string("some string")); As does this: std::string x("some string"); auto fn = boost::bind(g, _1); fn(std::move(x)); But this fails to compile: std::string x("some string"); auto fn = boost::bind(g, std::move(x)); fn(); Even though logically they ought to be the same. (The reason this happens AFAIK is because fn could be called more than once, but wouldn't be in a valid state after the first time; I think someone decided this should be a compile error rather than throwing an exception if you do happen to call it twice. It does compile if g doesn't have an rvalue-reference parameter.) Ironically this is actually one of the scenarios where bind was still superior to lambdas in C++11. Given void g(std::string const& x), you can compile that same last block of code just fine (even with the move), but you couldn't write an equivalent lambda (without contortions) since they lacked move-capturing. This has been fixed in C++14 though, albeit a bit wordy.