8 ms·

The repercussions of missing an Ampersand in C++ and Rust

80 points by nablags 1y ago

deleted 1y ago
[deleted]
darig 1y ago
[dead]
bubblebeard 1y ago
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.”.
- lionkor 1y ago
  I don't think the syntax has to be simple, it just needs to be expressive
machina_ex_deus 1y ago
I would never have this typo as I usually delete the copy constructor in heavy structures.
- lionkor 1y ago
  this is the defensive and correct C++ approach, anyways.
  - Ygg2 1y ago
    Isn't that just same old "skill issue", "No True C(++) programmer" refrain? If people could keep entirety of J.2 appendix in their mind at all time we would not have these issues. And if they had entirety of J appendix in mind all C code would be portable. Or if people just always ran -Wall -Wpedantic -Wall_for_real_this_time -fsanitize=thread,memory,address,leaks,prayers,hopes,dreams,eldritch_beings,elder_gods -fno-omit-frame-pointer I mean if this was all it took then C and C++ programs would be as safe as Rust. Which is not what we see in practice. And it's not like C programmers are an average web dev. It's a relatively niche and well versed community.
    - lionkor 1y ago
      Yes, it is the old "skill issue" argument. When your language is that unsafe and difficult to hold correctly, you have to make sure that you at least try your very best.
- colonwqbang 1y ago
  Do you ever use the C++ standard library? Most types have a copy ctor defined, also the really "heavy" ones.
fauigerzigerk 1y ago
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.
b0gb 1y ago
while doing math... would you call a missing sign a typo rather than a mistake? if so, anything can be a typo...
- 1718627440 1y ago
  The difference between a typo and an error is what the author had in mind to write. A typo is a subtype of mistake.
atoav 1y ago
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.
- estebank 1y ago
  > Worst case is that the compiler does a bad job at explaining the error and you don't find it immediately. By the way, the project considers this a bug and accepts reports for that. In many occasions they are easy to fix. In others large refactors are needed. But being aware of the case is the necessary first step to making them better.
squirrellous 1y ago
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.
- quuxplusone 1y ago
  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-contract/#grep-your-codebase-today 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-contract/#be-aware-that-scott-meyers-effec 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 https://godbolt.org/z/7177MTfb8
weinzierl 1y ago
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.
- Ygg2 1y ago
  Can't run Godbolt on my phone for some reason, but in this case I expect compiler to ignore wrapper types and just pass Vec around. If you have Vec<i32> // newtype struct struct Data{ data: Vec<i32> } // newtype enum in rust // Possibly but not 100% sure // enum OneVar { Data(Vec<i32>) } From my experiments with newtype pattern, operations implemented on data and newtype struct yielded same assembly. To be fair in my case it wasn't a Vec but a [u8; 64] and a u32.
  - tialaramex 1y ago
    The compiler isn't ignoring your new types, as you'll see if you try to pass a OneVar when the function takes a Vec but yes, Rust really likes new types whose representation is identical yet their type is different. My favourite as a Unix person is Option<OwnedFd>. In a way Option<OwnedFd> is the same as the classic C int file descriptor. It has the exact same representation, 32 bits of aligned integer. But Rust's type system means we know None isn't a file descriptor, whereas it's too easy for the C programmer to forget that -1 isn't a valid file descriptor. Likewise the Rust programmer can't mistakenly do arithmetic on file descriptors, if we intend to count up some file descriptors but instead sum them in C that compiles and isn't what you wanted, in Rust it won't compile.
jmull 1y ago
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.
- tialaramex 1y ago
  That's a fair observation about performance, but I think this goes to correctness too. For some types copying them affects the program correctness, and so in C++ you're more likely to write an incorrect program as a result of this choice.
- Ygg2 1y ago
  Problem is there is a huge number of pitfalls when measuring performance. You have to do it correct or you might be just measuring: when your system is pulling updates, how big is your username, the performance of the least critical thing in your app. And at worst you can speed up your least performing function only to yield a major slowdown to overall performance.
WalterBright 1y ago
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.
masklinn 1y ago
> 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).
- Waterluvian 1y ago
  This is where I think linters can shine as educational tools. Underline either as an error and you’ve taught someone something that’s actually quite tricky to discover on your own. Similar to all the times I defensively str(something) in Python to find that “oh that has __str__ called on it anyways.”
  - bayesnet 1y ago
    When I was starting out in rust, replacing my IDE’s `cargo check` invocation with pedantic clippy (which has a lint for this use of `into_iter` [0]) was very useful in learning these parts of the language. [0]: https://rust-lang.github.io/rust-clippy/master/index.html#explicit_into_iter_loop https://rust-lang.github.io/rust-clippy/master/index.html#ex...
SuperV1234 1y ago
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.
- Rubberducky1324 1y ago
  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.
- spacechild1 1y ago
  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.
  - vitus 1y ago
    > 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 https://gitlab.com/x86-psABIs/x86-64-ABI
- Animats 1y ago
  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.
on_the_train 1y ago
> 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
- Dylan16807 1y ago
  I think your estimate of how many C++ devs use linters is too high.
- dvratil 1y ago
  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...
  - on_the_train 1y ago
    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.
Arnavion 1y ago
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.
- spacechild1 1y ago
  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.
  - Dylan16807 1y ago
    > The destructor already has to deal with that. That's partly true, partly circular. Because moves work this way, it's harder to make a class that doesn't have empty states, so I don't design my class to avoid empty states, so the destructor has to handle them.
    - spacechild1 1y ago
      Please give me an example for a class that needs to handle empty state in the destructor only because of move operations. These exist, but IME they are very rare. As soon as you have a default constructor, the destructor needs to handle the case of empty state.
      - tialaramex 1y ago
        This means C++ is riddled with types that have unrelated "I'm empty" state inside them rather than this being relegated to a separate wrapper type. It's Tony's Billion Dollar Mistake but smeared across an entire ecosystem. The smart pointer std::unique_ptr<T> is an example of this, sometimes people will say it's basically a boxed T, so analogous to Rust's Box<T> but it isn't quite, it's actually equivalent to Option<Box<T>>. And if we don't want to allow None? Too bad, you can't express that in C++ But you're right that C++ people soldier on, there aren't many C++ types where this nonsense unavoidably gets in your face. std::variant's magic valueless_by_exception is such an example and it's not at all uncommon for C++ people to just pretend it can't happen rather than take it square on.
qalmakka 1y ago
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
kiitos 1y ago
> 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
- Dylan16807 1y ago
  It's const so you're not changing it, and you're not sneaking a pointer either. So what's the difference in intent?
- eptcyka 1y ago
  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.
  - Disposal8433 1y ago
    What do you suggest? Some kind of std::const_reference<Type>? Clang-tidy is enough in addition to the reviews.
    - Mesopropithecus 1y ago
      I'm seeing this way too often in production code, despite linters and reviews. So we have to keep plastering over.
    - eptcyka 1y ago
      The person is arguing that it is a massive difference, not a typo. I am saying that if that is the case, then maybe the hamming distance between correct and buggy code that both compile should be greater than 1, regardless if more tooling can help solve the problem or not. I specifically take issue with this framing of it is not an issue for we have the tools to help with this, especially where the tools are not part of a standard distribution of a toolchain and require more than minimal effort. C++ has had many a warts for many decades, and the response has always been *you are just holding it wrong* and not running a well covering integration test suite with sanitizers on every commit, you just need to run one more tool in the CI, just a comprehensive benchmarking suite, have more eyes looking for a single char difference in reviews.
rurban 1y ago
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

80 points by nablags 1y ago

deleted 1y ago

[deleted]

darig 1y ago

[dead]

bubblebeard 1y ago

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.”.

lionkor 1y ago
I don't think the syntax has to be simple, it just needs to be expressive

machina_ex_deus 1y ago

I would never have this typo as I usually delete the copy constructor in heavy structures.

lionkor 1y ago
this is the defensive and correct C++ approach, anyways.
- Ygg2 1y ago
  Isn't that just same old "skill issue", "No True C(++) programmer" refrain? If people could keep entirety of J.2 appendix in their mind at all time we would not have these issues. And if they had entirety of J appendix in mind all C code would be portable. Or if people just always ran -Wall -Wpedantic -Wall_for_real_this_time -fsanitize=thread,memory,address,leaks,prayers,hopes,dreams,eldritch_beings,elder_gods -fno-omit-frame-pointer I mean if this was all it took then C and C++ programs would be as safe as Rust. Which is not what we see in practice. And it's not like C programmers are an average web dev. It's a relatively niche and well versed community.
  - lionkor 1y ago
    Yes, it is the old "skill issue" argument. When your language is that unsafe and difficult to hold correctly, you have to make sure that you at least try your very best.
colonwqbang 1y ago
Do you ever use the C++ standard library? Most types have a copy ctor defined, also the really "heavy" ones.

fauigerzigerk 1y ago

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.

b0gb 1y ago

while doing math... would you call a missing sign a typo rather than a mistake? if so, anything can be a typo...

1718627440 1y ago
The difference between a typo and an error is what the author had in mind to write. A typo is a subtype of mistake.

atoav 1y ago

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.

estebank 1y ago
> Worst case is that the compiler does a bad job at explaining the error and you don't find it immediately. By the way, the project considers this a bug and accepts reports for that. In many occasions they are easy to fix. In others large refactors are needed. But being aware of the case is the necessary first step to making them better.

squirrellous 1y ago

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.

quuxplusone 1y ago
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-contract/#grep-your-codebase-today 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-contract/#be-aware-that-scott-meyers-effec 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 https://godbolt.org/z/7177MTfb8

weinzierl 1y ago

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.

Ygg2 1y ago
Can't run Godbolt on my phone for some reason, but in this case I expect compiler to ignore wrapper types and just pass Vec around. If you have Vec<i32> // newtype struct struct Data{ data: Vec<i32> } // newtype enum in rust // Possibly but not 100% sure // enum OneVar { Data(Vec<i32>) } From my experiments with newtype pattern, operations implemented on data and newtype struct yielded same assembly. To be fair in my case it wasn't a Vec but a [u8; 64] and a u32.
- tialaramex 1y ago
  The compiler isn't ignoring your new types, as you'll see if you try to pass a OneVar when the function takes a Vec but yes, Rust really likes new types whose representation is identical yet their type is different. My favourite as a Unix person is Option<OwnedFd>. In a way Option<OwnedFd> is the same as the classic C int file descriptor. It has the exact same representation, 32 bits of aligned integer. But Rust's type system means we know None isn't a file descriptor, whereas it's too easy for the C programmer to forget that -1 isn't a valid file descriptor. Likewise the Rust programmer can't mistakenly do arithmetic on file descriptors, if we intend to count up some file descriptors but instead sum them in C that compiles and isn't what you wanted, in Rust it won't compile.

jmull 1y ago

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.

tialaramex 1y ago
That's a fair observation about performance, but I think this goes to correctness too. For some types copying them affects the program correctness, and so in C++ you're more likely to write an incorrect program as a result of this choice.
Ygg2 1y ago
Problem is there is a huge number of pitfalls when measuring performance. You have to do it correct or you might be just measuring: when your system is pulling updates, how big is your username, the performance of the least critical thing in your app. And at worst you can speed up your least performing function only to yield a major slowdown to overall performance.

WalterBright 1y ago

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.

masklinn 1y ago

> 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).

Waterluvian 1y ago
This is where I think linters can shine as educational tools. Underline either as an error and you’ve taught someone something that’s actually quite tricky to discover on your own. Similar to all the times I defensively str(something) in Python to find that “oh that has __str__ called on it anyways.”
- bayesnet 1y ago
  When I was starting out in rust, replacing my IDE’s `cargo check` invocation with pedantic clippy (which has a lint for this use of `into_iter` [0]) was very useful in learning these parts of the language. [0]: https://rust-lang.github.io/rust-clippy/master/index.html#explicit_into_iter_loop https://rust-lang.github.io/rust-clippy/master/index.html#ex...

SuperV1234 1y ago

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.

Rubberducky1324 1y ago
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.
spacechild1 1y ago
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.
- vitus 1y ago
  > 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 https://gitlab.com/x86-psABIs/x86-64-ABI
Animats 1y ago
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.

on_the_train 1y ago

> 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

Dylan16807 1y ago
I think your estimate of how many C++ devs use linters is too high.
dvratil 1y ago
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...
- on_the_train 1y ago
  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.

Arnavion 1y ago

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.

spacechild1 1y ago
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.
- Dylan16807 1y ago
  > The destructor already has to deal with that. That's partly true, partly circular. Because moves work this way, it's harder to make a class that doesn't have empty states, so I don't design my class to avoid empty states, so the destructor has to handle them.
  - spacechild1 1y ago
    Please give me an example for a class that needs to handle empty state in the destructor only because of move operations. These exist, but IME they are very rare. As soon as you have a default constructor, the destructor needs to handle the case of empty state.
    - tialaramex 1y ago
      This means C++ is riddled with types that have unrelated "I'm empty" state inside them rather than this being relegated to a separate wrapper type. It's Tony's Billion Dollar Mistake but smeared across an entire ecosystem. The smart pointer std::unique_ptr<T> is an example of this, sometimes people will say it's basically a boxed T, so analogous to Rust's Box<T> but it isn't quite, it's actually equivalent to Option<Box<T>>. And if we don't want to allow None? Too bad, you can't express that in C++ But you're right that C++ people soldier on, there aren't many C++ types where this nonsense unavoidably gets in your face. std::variant's magic valueless_by_exception is such an example and it's not at all uncommon for C++ people to just pretend it can't happen rather than take it square on.

qalmakka 1y ago

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

kiitos 1y ago

> 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

Dylan16807 1y ago
It's const so you're not changing it, and you're not sneaking a pointer either. So what's the difference in intent?
eptcyka 1y ago
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.
- Disposal8433 1y ago
  What do you suggest? Some kind of std::const_reference<Type>? Clang-tidy is enough in addition to the reviews.
  - Mesopropithecus 1y ago
    I'm seeing this way too often in production code, despite linters and reviews. So we have to keep plastering over.
  - eptcyka 1y ago
    The person is arguing that it is a massive difference, not a typo. I am saying that if that is the case, then maybe the hamming distance between correct and buggy code that both compile should be greater than 1, regardless if more tooling can help solve the problem or not. I specifically take issue with this framing of it is not an issue for we have the tools to help with this, especially where the tools are not part of a standard distribution of a toolchain and require more than minimal effort. C++ has had many a warts for many decades, and the response has always been *you are just holding it wrong* and not running a well covering integration test suite with sanitizers on every commit, you just need to run one more tool in the CI, just a comprehensive benchmarking suite, have more eyes looking for a single char difference in reviews.

rurban 1y ago

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