The reason is, as lelanthran said, that when I see func(param), I don't expect param to be modified.
The time I lost because of non-const reference parameters adds up to days, maybe weeks. In fact, bugs caused by stealthy references are often way harder to find than null pointer crashes. Null pointer crashes are among the bugs that concern me the least, sure crashes are bad, but the core dump often tells you exactly what's wrong, and they rarely lead to non-DoS exploits. See: offensive programming.
Now that I think of it, maybe IDEs should highlight non-const references in function calls.
I think it is unfortunate because I like the extra guarantees that references offer over pointers but the lack of explicit (de)referencing is enough for me to avoid.
Well, we have wildly different experiences, I can't remember a single instance where this was an issue in the projects I've been involved with.
- Personal experience: it is not a construct I use, and most of the code I work with don't use it either, so when it happens, it is a surprise. People who come from a pointerless language like Java may expect it.
- I work with a lot of messy code: large code bases I barely know, written by many people from different companies, without a style guide. So one function may use references and the next one may use pointers, adding to the confusion.
- Of course, documentation is misleading (to the point that I make a conscious effort not to read it), and so are function names. Things like getChild(n) that increments n when pretty much every getChild(n) function everywhere passes n by value. Terrible code, that's why there is a bug in the first place, bugs are usually not in the best written parts.
- I actually like jumping in other people terrible code and fix bugs, it is an interesting challenge and I became rather good at it where I work. As a result, I am often given these kinds of job. A few days is not that much compared to my entire experience of debugging terrible code, but enough to be significant.
- Of course I spend a lot more time fixing memory corruption (and the problem with stealthy reference is that it can look like memory corruption). But in my experience, references over pointers don't help that much with it. It helps against null pointer dereferencing, but as I said before, these are usually easy bugs to fix.
CLion highlights this with an off-color "&:" in front of the parameter.
https://godbolt.org/z/c5YeP6feG
edit: std::reference_wrapper is of course the standard blessed way to do this, but still not idiomatic.
This is a great idea. When I type `ls` in a shell, I get important information from the colors of the output: one color for normal files, another for executables, another for directories, etc.
This would work well for parameter types, even if it were as simple as "white means it cannot be modified (pass by value, const reference, etc.) and yellow means it can (non-const reference, pointer, etc.)"
I wonder if this exists?
If a function returns a value, have it return that value, instead of using an out-parameter. Passing in-parameters by const reference rather than by value is a nice win because the semantics of the function remain clear. Input parameters still look like input parameters, you're just preventing unnecessary copies where it's easy to do so. Returning a value as an out-parameter rather than just returning it is a performance hack that prevents a copy, but at the expense of muddying the semantics of the function.
As the prophet says: we should forget about small efficiencies, say about 97% of the time. Write code that clearly expresses what it does. Return your return values. If you run a profiler over your code and you notice that some returns are causing a performance problem - sure, at that point, add an overload that takes an out-parameter and use it at the frequenty-executed hot spot.
SomeType result;
if (!do_something(&result))
return;
process(result);
std::tuple<bool, SomeType> ret = do_something();
if (!std::get<0>(ret))
return;
process(std::get<1>(ret));
const std::optional<SomeType> result = do_something();
if (!result.has_value()) {
return;
}
process(result.value());
C++ doesn't have great support for sum types, so this is all always going to be pretty non-ergonomic, and the compiler doesn't always warn if you forget to do things. I might even say that the main advantage of using sum types is gone in C++ - you actually can forget to check has_value and call operator* on an empty optional.
It would be so easy for someone to make a mistake and write:
process(*do_something());
This whole thing is a mess, I hate C++, thinking about UB all the time...
I like the std::optional<> version and use that pattern some of the time. But if the object being passed/returned to/from do_something() is non-trivial, i don't want the copy overhead.
Can you go into detail? What are the benefits that can't be replicated?
If anything, the std::optional version seems better, since we can avoid an invocation of the SomeType constructor. Otherwise, everything is the same - everything's allocated on the stack, there is no dynamic allocation.
> If an optional<T> contains a value, the value is guaranteed to be allocated as part of the optional object footprint, i.e. no dynamic memory allocation ever takes place. Thus, an optional object models an object, not a pointer, even though operator*() and operator->() are defined.
That's from here: https://en.cppreference.com/w/cpp/utility/optional
Since C++17, RVO is required by the standard too:
> Return value optimization is mandatory and no longer considered as copy elision; see above.
From here: https://en.cppreference.com/w/cpp/language/copy_elision
I admit I'm not really a C++ expert, so maybe I'm missing something.
Unless your compiler is truly ancient odds are good it RVOs just fine no?
Probably many C++ aficionados will run to godbolt.org to prove this in a simple testcase, and they will probably be right. But then maybe it’s because you’ve only tested this for primitive or POD types. Maybe this might not be guaranteed for non-POD types with custom constructors? Or maybe this might be okay because of RVO or something? Hmm, let us read the specs again…
The problem is, C++ is a language that is very unintuitive about how your code is going to get mapped to actual hardware (assembly code). For many systems developers, it isn’t enough that the compiler might optimize this smartly; we instead want predictable compiler behavior. So if you want to make sure, and you’re not confident about the gnarly details of the C++ specification, it will be better for you just use out parameters instead of std::optional, if you are in the situation where you really need to squeeze out performance (which happens a lot for low-level systems programming).
The implementation of std::optional is not permitted by the standard to allocate memory.
It’s more specific to the example code the original commenter posted (and about general out-parameter usage) than the flaws of std::optional itself. Maybe this didn’t get communicated properly from my comment.
do_something().and_then(process);
auto [error_code, result] = do_something();
if(error_code == 0) {
process(result);
} // Oops, backwards!
auto [result, error_code] = do_something();
if(error_code) {
process(result);
}It's a very well established pattern to have (error, result) I'd immediately see something off with (result, error)
And clang-tidy can catch this: https://clang.llvm.org/extra/clang-tidy/checks/readability-i...
auto [error, value] = do_something()
if(!error)
process(value)
Those have their problems too, though, and I'd never claim std::variant is "nice" to use.
However, if you are compiling at -O0 without any optimizations this may not happen.
There are probably exceptions and loopholes just like with literally everything in C++, but there's this line:
> Return value optimization is mandatory and no longer considered as copy elision; see above.
Thanks for the clarification. Clang does do NRVO in that case but I guess it's obviously not guaranteed by the standard.
I guess it makes sense that an optimization that GCC is not able to do at all should not be part of the standard ; I'm pretty sure it should be possible to write cases that clang wouldn't be able to optimize either.
void foo<T>(T by_value, ref T by_ref, in T by_immutable_ref, out T out_only);
`out` parameters pass no value in and must pass a value out. Isn't that just a return value, semantically? Yes, but we didn't used to have value tuples and destructuring so we had to make do. It can still be nicer for some things:
bool TryParse(string input, out int value)
if (int.TryParse("123", out int value))
// Use valueThus (anonymous) tuples were introduce so we can do it Go/Rust-style:
async Task<(bool, T)> Foo<T>(T value) {
/* do something async like a web request or file read etc. */
var aThing = await GetAThing<T>(value);
return (aThing != null, aThing);
}The other side of the coin is that it hurts readability if you are passing non-const references. When a function takes a non-const reference, the caller looks like this:
foo (bar); // Have to read the header to know if bar is modified.
foo (&bar); // Immediately clear that we cannot gloss over this line in code-review.
A better (but not best) solution might be to have the language recognise and enforce nullability on arguments. Still not perfect (pointer may be non-null but invalid), but better.
One other benefit pointer parameters have over reference parameters is that the API contract is more flexible. For example, I have a function that accepts a network connection,
mynet_accept(int listener_fd, char **remote_ip, uint16_t *remote_port);
If that was changed to use references, the caller will have to declare variables just to satisfy the parameter list, even if they never intend to use them.
This has honestly never ever been an issue for me. I don't remember the last time where I passed an argument to a C++ function and actually had to wonder if the argument was modified or not. Like, what if they are being modified if that's the called function has to do anyways ? And even if it's const, the implementation can just remove the const which while not a very nice thing to do, is legal in many cases.
$ cd
$ rg "const_cast" -g '*.cpp' | wc -l
2004
> mynet_accept(int listener_fd, char *remote_ip, uint16_t *remote_port);
For this exact case my go-to solution nowadays is
struct connection_info {
int fd;
std::string ip;
uint16_t port;
};
mynet_accept({ .fd = the_socket, .port = 4556 });
That's not what I was complaining about. I am saying that, as a reader of the code, it is easier to reason about what happens in this situation:
foo (&bar);
...
if (bar) { do something }
foo (bar);
...
if (bar) { do something }
That's not functionally the same as the example I posted. In any case, I was using the example to point out that using pointers provides a more flexible API contract.
When using pointers, the contract can state that any parameters that are NULL are ignored, whether they are out parameters or not.
The idea is that you have some code like this:
x = some_func();
do_something(x);
if x == 0 {
do_something_else();
}
Frequently it can't be declared constant, because it is being changed in that scope as it runs.
Then I put a breakpoint at that line and hit f5 and don't try to guess what may or may not happen because in the end that's the only way to know what actually happens
There's no need to guess if the parameter is passed by value only. When pointers are used visual inspection alone is enough to ensure that the reader knows what he value of the argument is after the callee returns.
I said that it's easier to read when pointers are used, and you propose stepping through the code as an alternative. Surely you see my point now?
not really a fair count though, since those casts may also be adding const
Almost never, in my experience. During code review, when I see this:
foo(&bar);
When I see this:
foo(bar);
In a place where the convention is to use pointers instead of mutable references I never have to ask the question - the code review of the implementation of `foo` would have changed the mutable reference parameter to a pointer parameter.
I read code much more often than I write it. References hurt readability.
void foo(const T & in, const T * optional_in, T & out, T * optional_out);Agreed. Also what did you mean by introducing child classes? If you move const fields from your class to a field which is an inner class, either it has an assignment operator (so you can overwrite the entire class by mistake) or not (so the outer class can't be assigned over). If you move all fields from your class to a field, it becomes substantially more tedious to access them. And I suppose you could move immutable fields to be private in a base class supplying a getter, but this feels like a hacky use of inheritance (you lose aggregate initialization, you expose an implementation detail in the header, etc).
Personally I want C++'s const fields to allow moving and swapping to replace them, but not regular methods. Is it possible for the compiler to distinguish moves/swaps and regular methods, or provide an escape hatch to allow `operator=() = default` or manually defining moves/swaps? And would the standards committee accept that type of change?
There's one exception, though. I'd support const fields being non-const in the constructor and destructor. But then that should be coupled with the constructor being able to run code before constructing members (in a non-hacky way). The constructor's task is to establish class invariants, so while the constructor is running, it's reasonable to assume all invariants haven't been established, yet, including the immutability of some fields.
This isn't nearly as clear-cut for move construction/assignment, which would need to modify a different object's const members, which at the point of invocation is not at any special stage of its lifetime.
You need no such NULL check. Specify as part of the function's contract that a pointer be non-NULL: if a caller breaks the contract, behavior is undefined. An unexpected NULL is just like any other violation of the function's contract: a programming error.
Spell out the contract in the function's documentation. A contract is more than the signature. There is absolutely no need for a runtime check (certain special cases aside) to enforce function contracts.
In some cases, assert()ing that a parameter that is expected to be non-NULL is in fact non-NULL can be useful --- but you don't need such checks for correctness of the program, and use of assertions in general ought to be a matter of taste and discretion.
Knee-jerk, automatic checking by random internal functions of a module of pointer parameters for NULL in every single place suggests that the author doesn't fully comprehend the concept of interface contracts generally.
Maybe so. Reasonable people differ on this subject.
My point is different: it's just not true that if a parameter is a pointer, then as a general requirement you have to check at runtime whether that pointer is non-NULL. It drives me nuts when I see these checks sprinkled throughout a codebase.
My point is that if you only ever use pointer arguments to indicate that the argument is optional, then the checks are required.
More nuanced, it's more important to have these null checks on a library's public interfaces than a library's private interfaces. Private interfaces can do with asserts for null check.
No it isn't.
The sibling subthread is all about people not wanting to read the function's documentation.
Until then I avoid adding undefined behavior to my code for good reasons.
While I generally agree that non-const references make more sense for out-parameters (although I actually prefer return values), don't forget that it's possible for a reference to become "null" too. It's quite unlikely to happen, but if it happens it's much worse to debug than a null pointer.
Oversimplified example:
const int& bla = *((int*)nullptr);
OP says this like this is an industry rule of sorts but I've pretty much always seen non-const references for this. Using pointers is terrible because now you have to add a check for null inside your function, as its API contract says "I can accept null values", while references make it clear that it does not make sense to call if you don't have a correct object to pass it.
> That’s right: the point of making a class with private members is to preserve invariants among those members.
How would OP make a class, which loads content on construction which must absolutely not being changed after construction then, even within its implementation ? "Being careful" is really not sufficient in my experience, and introducing child classes just to enforce the invariant that "const" provides is actively harmful to readability.