5 ms·

Unfortunately there are a lot of overly-strong claims in this blog post; the choices are not as clear-cut as the author makes them seem. I'll list a few example

Unfortunately there are a lot of overly-strong claims in this blog post; the choices are not as clear-cut as the author makes them seem. I'll list a few examples to illustrate (these are not exhaustive): > What we mean to write is: auto plus(const std::string& s, const std::string& t) { return s + t; } To a first-order approximation, sure. But arguably it's better to pass 's' by value and 't' by const reference, then do s += t and return s. That lets you reuse any existing buffer if you receive an rvalue, and lets the caller preallocate the buffer if it so desires. This can help you avoid a heap allocation. > This function is wrong: auto plus(const std::string s, const std::string& t). The programmer meant to type const std::string& s. Not necessarily. The const tells the programmer that s will never be modified. That's an excellent constraint that can make a nontrivial function easy to reason about. (Obviously that doesn't imply you should do this everywhere. I'm just pointing out it's not "wrong".) P.S. I hear optimizers can sometimes optimize const objects, though I don't have proof handy for that example, so someone else can fact-check that part. > or maybe they meant const std::shared_ptr<Connection>& for efficiency (but in that case why aren’t they simply passing const Connection& from the caller?) That "why" actually has an answer: "because the function might only copy the shared_ptr conditionally, and desire to remain efficient in the no-copy case". > Const data members are never a good idea. This one is correct!

nemetroid 5y ago
> That "why" actually has an answer: "because the function might only copy the shared_ptr conditionally, and desire to remain efficient in the no-copy case". Yeah. This blog post has a few well argued guidelines about passing smart pointers to functions, which covers this case: https://herbsutter.com/2013/06/05/gotw-91-solution-smart-pointer-parameters/ https://herbsutter.com/2013/06/05/gotw-91-solution-smart-poi... > Use a const shared_ptr& as a parameter only if you’re not sure whether or not you’ll take a copy and share ownership; otherwise use widget* instead (or if not nullable, a widget&).
tsimionescu 5y ago
> Obviously that doesn't imply you should do this everywhere. I'm just pointing out it's not "wrong". The article makes a point about contracts. In their code base, it's always wrong, since function parameters, by convention, are only allowed to be (a) non-const values, (b) const references, or (c) non-const pointers. Anything else, in their codebase indicates an error. Their point is that these three are all you need, so that getting in the habit of only using them helps maintenance, since during code review or something you know for sure that any other parameters is a mistake someone made, not an intentional choice (because of the coding standard). > P.S. I hear optimizers can sometimes optimize const objects, though I don't have proof handy for that example, so someone else can fact-check that part. That can happen, if a reference to your local variable is passed to a function. For example, tried this in Godbolt [0] void some_func(const int& x); void some_other_func(int x); int bar() { int x = 123; some_func(x); volatile int y = x; } The final line directly writes 123 to y if x is const (or if calling some_other_func()), but it reads again from memory as it is. This happens because some_func could do this: void some_func(const int& x) { const_cast<int&>(x) = 7; } If x was `const int x`, this is UB, but if x is `int x`, the program MUST write 7 to y. Note however that this optimization only happens (with clang) IF the value of x is a compile-time constant. If instead the value is only known at runtime, there is 0 difference - the value of x is still read from the stack, not kept in a register for example. [0] https://godbolt.org/z/YKf4ndq1v https://godbolt.org/z/YKf4ndq1v
omegalulw 5y ago
> Obviously that doesn't imply you should do this everywhere. I'm just pointing out it's not "wrong".) I have to disagree here. const-int everything except stuff truly needs to be optimized significantly helps with readability, and when someone is updating their code they can other be warned of invariants if they inadvertently try to modify something that is const. Yes, it's a little verbose but worth it IMO. It also helps the compiler :)
- dataflow 5y ago
  I held the same position as you until quite recently. Unfortunately I've changed my mind on this somewhat. I've found it introduces too much friction in practice to be generous with const everywhere. A few reasons for this that I can remember off the top of my head are the following (there are probably more that I'm forgetting): (1) Constifying variables hampers your ability to move from them (including RVO), which you still frequently want to do even in cases where you don't otherwise mutate the object. Which is obviously bad for performance. Now your solution might be "ok, so don't put const if you plan to move", and sure, you can do that, but then your variables and parameters start becoming an inconsistent mixture of const- and non-const types. Not the end of the world, but it's kind of jarring for a reader (read: increased cognitive load, confusion, etc. during maintenance) when they wonder why X is const and Y is mutable despite the fact that the problem should treat both of them as const. Now, if this was the sole downside, I would say might still be worth it on the balance, but it doesn't help the situation when there are other downsides. (2) Conceptually, constness of object types in a function signature is an implementation detail callers should be unconcerned with, but language-wise, it's also encodable in the prototype. The practical implication of this is that the prototype ends up going out of sync with your implementation, and you'll want to fix them up so they match. (Not just for aesthetics, but because it also helps with tools, say grep.) The compiler doesn't necessarily complain for object parameters, but if you don't fix them then you end up with inconsistent signatures that are further confusing at best. Not only does this get quite annoying, but fixing it also forces the recompilation of all callers, which is yet another thing that slows down build times unnecessarily. (3) It's practically impossible to get other developers on board with this even if you ignore all the other issues. Too many people just don't care enough for the advantages to change their habits on this issue. If anything, you'll be forced to remove "excessive" const for consistency with the existing codebase. Now there are obvious advantages, and I could honestly argue for either side, but the friction starts getting old. It's just not a clear-cut case in practice, as much as in theory I want to agree with you (and have in the past).
jcranberry 5y ago
Passing by 'const value' is not a real thing. In the implementation you can remove the const and the compiler will not complain. You can put them in the definition and the compiler will prevent you from mutating that argument in the body of the function however. I remove these in declarations if I see them usually. I see it as noisy and a little distracting, the caller can already see that there's going to be no side effects to that variable, whether or not the function changes its copy of it is an implementation detail.

nemetroid 5y ago

> That "why" actually has an answer: "because the function might only copy the shared_ptr conditionally, and desire to remain efficient in the no-copy case". Yeah. This blog post has a few well argued guidelines about passing smart pointers to functions, which covers this case: https://herbsutter.com/2013/06/05/gotw-91-solution-smart-pointer-parameters/ https://herbsutter.com/2013/06/05/gotw-91-solution-smart-poi... > Use a const shared_ptr& as a parameter only if you’re not sure whether or not you’ll take a copy and share ownership; otherwise use widget* instead (or if not nullable, a widget&).

tsimionescu 5y ago

> Obviously that doesn't imply you should do this everywhere. I'm just pointing out it's not "wrong". The article makes a point about contracts. In their code base, it's always wrong, since function parameters, by convention, are only allowed to be (a) non-const values, (b) const references, or (c) non-const pointers. Anything else, in their codebase indicates an error. Their point is that these three are all you need, so that getting in the habit of only using them helps maintenance, since during code review or something you know for sure that any other parameters is a mistake someone made, not an intentional choice (because of the coding standard). > P.S. I hear optimizers can sometimes optimize const objects, though I don't have proof handy for that example, so someone else can fact-check that part. That can happen, if a reference to your local variable is passed to a function. For example, tried this in Godbolt [0] void some_func(const int& x); void some_other_func(int x); int bar() { int x = 123; some_func(x); volatile int y = x; } The final line directly writes 123 to y if x is const (or if calling some_other_func()), but it reads again from memory as it is. This happens because some_func could do this: void some_func(const int& x) { const_cast<int&>(x) = 7; } If x was `const int x`, this is UB, but if x is `int x`, the program MUST write 7 to y. Note however that this optimization only happens (with clang) IF the value of x is a compile-time constant. If instead the value is only known at runtime, there is 0 difference - the value of x is still read from the stack, not kept in a register for example. [0] https://godbolt.org/z/YKf4ndq1v https://godbolt.org/z/YKf4ndq1v

omegalulw 5y ago

> Obviously that doesn't imply you should do this everywhere. I'm just pointing out it's not "wrong".) I have to disagree here. const-int everything except stuff truly needs to be optimized significantly helps with readability, and when someone is updating their code they can other be warned of invariants if they inadvertently try to modify something that is const. Yes, it's a little verbose but worth it IMO. It also helps the compiler :)

dataflow 5y ago
I held the same position as you until quite recently. Unfortunately I've changed my mind on this somewhat. I've found it introduces too much friction in practice to be generous with const everywhere. A few reasons for this that I can remember off the top of my head are the following (there are probably more that I'm forgetting): (1) Constifying variables hampers your ability to move from them (including RVO), which you still frequently want to do even in cases where you don't otherwise mutate the object. Which is obviously bad for performance. Now your solution might be "ok, so don't put const if you plan to move", and sure, you can do that, but then your variables and parameters start becoming an inconsistent mixture of const- and non-const types. Not the end of the world, but it's kind of jarring for a reader (read: increased cognitive load, confusion, etc. during maintenance) when they wonder why X is const and Y is mutable despite the fact that the problem should treat both of them as const. Now, if this was the sole downside, I would say might still be worth it on the balance, but it doesn't help the situation when there are other downsides. (2) Conceptually, constness of object types in a function signature is an implementation detail callers should be unconcerned with, but language-wise, it's also encodable in the prototype. The practical implication of this is that the prototype ends up going out of sync with your implementation, and you'll want to fix them up so they match. (Not just for aesthetics, but because it also helps with tools, say grep.) The compiler doesn't necessarily complain for object parameters, but if you don't fix them then you end up with inconsistent signatures that are further confusing at best. Not only does this get quite annoying, but fixing it also forces the recompilation of all callers, which is yet another thing that slows down build times unnecessarily. (3) It's practically impossible to get other developers on board with this even if you ignore all the other issues. Too many people just don't care enough for the advantages to change their habits on this issue. If anything, you'll be forced to remove "excessive" const for consistency with the existing codebase. Now there are obvious advantages, and I could honestly argue for either side, but the friction starts getting old. It's just not a clear-cut case in practice, as much as in theory I want to agree with you (and have in the past).

jcranberry 5y ago

Passing by 'const value' is not a real thing. In the implementation you can remove the const and the compiler will not complain. You can put them in the definition and the compiler will prevent you from mutating that argument in the body of the function however. I remove these in declarations if I see them usually. I see it as noisy and a little distracting, the caller can already see that there's going to be no side effects to that variable, whether or not the function changes its copy of it is an implementation detail.