35 comments

[ 4.3 ms ] story [ 53.6 ms ] thread
Note that taking a 'const' by-value parameter is very sensible in some cases, so it is not something that could be detected as a typo by the C++ compiler in general.
clang-tidy can often detect these. If the body of the function doesn't modify the value, for example.

But it needs to be conservative of course, in general you can't do this.

Yes. For example, if an argument fits into the size of a register, it's better to pass by value to avoid the extra indirection.
> if an argument fits into the size of a register, it's better to pass by value to avoid the extra indirection.

Whether an argument is passed in a register or not is unfortunately much more nuanced than this: it depends on the ABI calling conventions (which vary depending on OS as well as CPU architecture). There are some examples where the argument will not be passed in a register despite being "small enough", and some examples where the argument may be split across two or more registers.

For instance, in the x86-64 ELF ABI spec [0], the type needs to be <= 16 bytes (despite registers only being 8 bytes), and it must not have any nontrivial copy / move constructors. And, of course, only some registers are used in this way, and if those are used up, your value params will be passed on the stack regardless.

[0] Section 3.2.3 of https://gitlab.com/x86-psABIs/x86-64-ABI

Right. Copying is very fast on modern CPUs, at least up to the size of a cache line. Especially if the data being copied was just created and is in the L1 cache.

If something is const, whether to pass it by reference or value is a decision the compiler should make. There's a size threshold, and it varies with the target hardware. It might be 2 bytes on an Arduino and 16 bytes on a machine with 128-bit arithmetic. Or even as big as a cache line. That optimization is reportedly made by the Rust compiler. It's an old optimization, first seen in Modula 1, which had strict enough semantics to make it work.

Rust can do this because the strict affine type model prohibits aliasing. So the program can't tell if it got the original or a copy for types that are Copy. C++ does not have strong enough assurances to make that a safe optimization. "-fstrict-aliasing" enables such optimizations, but the language does not actually validate that there is no aliasing.

If you are worried about this, you have either used a profiler to determine that there is a performance problem in a very heavily used inner loop, or you are wasting your time.

> There are plenty of linters and tools to detect issues like this (ex: clang-tidy can scan for unnecessary value params)

Exactly, this is not an issue in any reasonable setup because static analysis catches (and fixes!) this reliably.

> but evidently these issues go unnoticed until a customer complains about it or someone actually bothers to profile the code.

No

I think your estimate of how many C++ devs use linters is too high.
This is my gripe with C++ - I have to have a CI pipeline that runs a job with clang-tidy (which is slow), jobs with asan, memsan and tsan, each running the entire test-suite, and ideally also one job for clang and one for gcc to catch all compiler warnings, then finally a job that produces optimized binaries.

With Rust I have one job that runs tests and another that runs cargo build --release and I'm done...

That's a pretty heavy setup. Clang tidy is usually enough. And not slow when running locally on newly typed code in resharper for example.
Rust's behavior of moving without leaving a moved-out shell behind also simplifies the implementation of the type itself, because its dtor doesn't have to handle the special case of a moved-out shell, and the type doesn't even need to be able to represent a moved-out shell.

For example, a moved-out-from tree in C++ could represent this by having its inner root pointer be nullptr, and then its dtor would have to check for the root being nullptr, and all its member fns would have the danger of UB (nullptr dereference) if the caller called them on a moved-out shell. But the Rust version could use a non-nullable pointer type (Box), and its dtor and member fns would be guaranteed to act on a valid pointer.

In practice, move operations typically just leave an empty object behind. The destructor already has to deal with that. And of course you can't call certain methods on an empty object. So in practice you don't need special logic except for the move operations themselves.
> For example, a moved-out-from tree in C++ could represent this by having its inner root pointer be nullptr, and then its dtor would have to check for the root being nullptr,

delete null is fine in C++ [1], so, assuming root either is a C++ object or a C type without members that point to data that also must be freed, its destructor can do delete root. And those assumptions would hold in ‘normal’ C++ code.

[1] https://en.cppreference.com/w/cpp/language/delete.html: “If ptr is a null pointer value, no destructors are called, and the deallocation function may or may not be called (it's unspecified), but the default deallocation functions are guaranteed to do nothing when passed a null pointer.”

> I was specifically inspired by a performance bug due to a typo. This mistake is the “value param” vs “reference param” where your function copies a value instead of passing it by reference because an ampersand (&) was missing ... This simple typo is easy to miss

the difference between `const Data& d` and `const Data d` isn't accurately characterized as "a typo" -- it's a semantically significant difference in intent, core to the language, critical to behavior and outcome

even if the author "forgot" to add the `&` due to a typo, that mistake should absolutely have been caught by linting, tests, CI, or code review, well before it entered the code base

so not feelin' it, sorry

It's const so you're not changing it, and you're not sneaking a pointer either. So what's the difference in intent?
If the implications of a one char diff are this egregious that they’re considered obvious, maybe it should take less cognitive effort to spot this? CI and tooling are great, but would be far less necessary if it was more difficult to make this mistake in the first place.
if you are programming in C++ then you have opted-in to a set of syntax and semantic properties that are ancient and well-defined and core to the language. those properties include at a very basic level exactly the sigil under discussion here.

it is not productive or interesting to characterize this absolutely core property of the language as "a one char diff" that takes any kind of special cognitive effort to spot

Disclaimer: I didn't have any production experience, only side projects in both C++ & Rust.

I think the problem with `T &d` and `T d` is that these 2 declarations yield a "name" `d` that you can operate on very similarly. It's not necessarily about reference declaration `T& d` is 1 char diff away compared to value declaration `T d`.

While there is a significant semantic difference between declaring things as a value and as a reference (&), non-static member function invocation syntax is the same on both `&d` and `d`. You can't tell the difference without reading the original declaration, and the compiler will happily accept it.

Contrast this to `T *d` or `T d`. Raw pointers require different operations on `d` (deref, -> operator, etc). You're forced to update the code if you change the declaration because the compiler will loudly complain about it.

It shares the same problem with a type system with nullable-by-default reference type vs an explicit container of [0..1] element Option<T>. Migrating existing code to Option<>-type will cause the compiler to throw a ton of explicit errors, and it will become a breaking change if it was a public API declaration. On the other hand, you're never able to feel safe in nullable-by-default; a public API might claim it never return `null` in the documentation, but you will never know if it's true or not only from the type signature.

Whether it's good or bad, I guess it depends on the language designer's decision. It is certainly more of a hassle to break & fix everything when updating the declaration, but it also can be a silent footgun as well.

The fact that implicit copies are a feature doesn't mean they were a good design choice to begin with. In new code I've started making the copy constructor explicit whenever I can, for instance, just to avoid this kind of shenanigans
Problem is it doesn't affect outcome at all unless you do mutation, and as such testing is irrelevant, but still can significantly impacts perf, and performance problems can take a while to surface; like, it may slowly grow from 0.1% of runtime to like 2%, low enough to not get get noticed at all at first, and still be too low to have significant thought put into it afterwards (but still way too high from a single missing character).

And, as you said, this is a meaningful difference in intent, so linting can't just blanket complain on every single instance of a non-&-ed argument.

And the difference in writing down intent is the wrong direction - doing a full nested object clone should require adding code in any sane language, whereas, in C++, making code clone takes.. negative one characters.

Whereas in Rust, the only thing that's ever implicit is a bitwise copy on objects with constant size; everything else requires either adding &-s or .clone()s, or your code won't compile.

Great article. It think it raises a good point. An important aspect of modern programming languages should be to simplify the syntax, to help developers avoid mistakes.

This reminds me of arguing more than once with JS developers about the dangers of loose typing (especially in the case of JS) and getting the inevitable reply ”I just keep track of my type casting.”.

I would never have this typo as I usually delete the copy constructor in heavy structures.
I like Rust's approach to this. It's even more important when comparing with languages that hide value/reference semantics at the call site.

I've been writing some Swift code in recent years. The most frequent source of bugs has been making incorrect assumptions on whether a parameter is a class or a struct (reference or value type). C# has the same issue.

It's just a terrible idea to make the value/reference distinction at the type level.

while doing math... would you call a missing sign a typo rather than a mistake? if so, anything can be a typo...
The difference between a typo and an error is what the author had in mind to write. A typo is a subtype of mistake.
As someone who programs both C++ and Rust, without even reading the article, my own experience with typos in those languages is:

Rust: Typo? Now it just doesn't compile anymore. Worst case is that the compiler does a bad job at explaining the error and you don't find it immediately.

C++: Typo? Good luck. Things may now be broken in so subtle and hard to figure out ways it may haunt you till the rest of your days.

But that of course depends on the nature of the typo. Now I should go and read the article.

This might be an unpopular opinion - I think const by-value parameters in C++ shouldn’t exist. Const reference and mutable values are enough for 99% cases, and the other 1% is r-value refs.

Regarding const by-value parameters, they should never appear in function declarations (without definition) since that doesn’t enforce anything. In function definitions, you can use const refs (which have lifetime extension) to achieve the same const-correctness, and const refs are better for large types.

Admittedly this further proves the point that c++ is needlessly complicated for users, and I agree with that.

Absolutely correct. Basically, C++ has value semantics — you pass arguments of type X like `void f(X x)`, and you return them like `X f()`, and that's good enough for a first approximation. (This is the only thing C lets you do.)

The second refinement is that you can use `const X&` as an optimization of `X`. (Perfectly safe for parameters; somewhat treacherous for return values.) Passing by `X&` without the const, or by `const X` without the ampersand, are both typos, and you should regularly use tooling to find and fix that kind of typo.

https://quuxplusone.github.io/blog/2019/01/03/const-is-a-con...

And that's it, for business-logic code. If you're writing your own resource-management type, you'll need to know about `X(X&&)` and `X& operator=(X&&)`, but ordinary business-logic code never does.

"What about `X&` for out-parameters?" Pass out-parameters by pointer. It's important and helpful to indicate their out-parameter-ness at the call-site, which is exactly what passing by pointer does. (And the pointer value itself will be passed by value, just like in C.)

"What about return by const value, like Scott Meyers recommended 20–30 years ago?" No, don't do that. It disables the ability to move-assign or move-construct from the return value, which means it's a pessimization. Scott found this out, retracted that advice in 2009, and correctly issued the opposite advice in his 2014 book.

https://quuxplusone.github.io/blog/2019/01/03/const-is-a-con...

At work I use a Clang patched with "-Wqual-class-return-type" to report return-by-const-value typos — since, again, `const X getter()` is almost always a typo for `const X& getter()`.

You can use that compiler too: https://godbolt.org/z/7177MTfb8

With Rust executing a function for either case deploys the “optimal” version (reference or move) by default, moreover, the compiler (not the linter) will point out the any improper “use after moves”.

    struct Data {
      // Vec cannot implement "Copy" type
      data: Vec<i32>,
    }

    // Equivalent to "passing by const-ref" in C++
    fn BusinessLogic(d :&Data) {
      d.DoThing();
    }

    // Equivalent to "move" in C++
    fn FactoryFunction(d: Data) -> Owner {
      owner = Owner{data: d};
      // ...
      return owner
    }

Is this really true?

I believe in Rust, when you move a non-Copy type, like in this case, it is up to the compiler if it passes a reference or makes a physical copy.

In my (admittedly limited) understanding of Rust semantics calling

     FactoryFunction(d: Data) 
could physically copy d despite it being non-Copy. Is this correct?

EDIT:

Thinking about it, the example is probably watertight because d is essentially a Vec (as Ygg2 pointed out).

My point is that if you see

     FactoryFunction(d: Data) 
and all you know is that d is non-Copy you should not assume it is not physically copied on function call. At least that is my believe.
This isn't a C++ vs. Rust thing.

If you care about performance, you measure it. If you don't measure performance, you don't care about it.

This is why the D programming language uses the keyword `ref` rather than the ampersand. Too many overlooked misteaks with the latter.

It extends it a bit, too, with `out` meaning that the referenced argument is initialized by the function, not read.

> Granted, these repercussions of these defaults also result in (in my opinion) verbose language constructs like iter, into_iter, iter_mut ↩

Note that assuming the into_iter comes from IntoIterator that’s what the for loop invokes to get an iterator from an iterable. So

    for lr in LoadRequests.into_iter() {
Is completely unnecessary verbosity,

    for lr in LoadRequests {
Will do the exact same thing. And the stdlib will generally implement the trait with the relevant semantics on shared and unique references so

    for lr in LoadRequests.iter_mut()
Can generally be written

    for lr in &mut LoadRequests
So you rarely need to invoke these methods outside of functional pipelines if you dislike them (some prefer them for clarity / readability).
(comment deleted)
The real issue is that C++ does implicit _deep_ copies by default on assignment and that you can't retrofit the language to change that. One quick, fast solution to avoid such shenanigans is to follow the one parameter `explicit` constructor rule religiously and always mark copy constructors explicit unless you know as a fact the type is trivially memcpy-able. This fixes most of the issues.

Another problem with C++ references is that they aren't really reference types, they are aliases, so they have wonky semantics and crazy nonsensical features like `const T&` doing lifetime extension

I guess he prefers the magic action at a distance pattern over functional and concurrency safeties. Then he should also mention it at least.

All good linters complain about const buffer data missing the ampersand btw