7 ms·

Care is needed to use C++ std:optional with non-trivial objects

21 points by dilawar 4y ago

kllrnohj 4y ago
The "care" seems to just be "thing that looks like a value type is a value type." I'm not sure why the author would have ever expected anything different?
- CoastalCoder 4y ago
  > I'm not sure why the author would have ever expected anything different? Perhaps the message is aimed at less experienced C++ programmers?
- omnicognate 4y ago
  Yeah, that's the whole point of std::optional. Pointers (including smart ones) have the possibility of being empty/pointing-to-nothing built in. std::optional adds it to value types. To avoid tons of unnecessary copies use move semantics, which std::optional of course supports.
- jay-barronville 4y ago
  Fair point, but I didn’t gather that the author expected anything different; I think he simply wanted to highlight a behavior that may not be that obvious to programmers who aren’t expert C++ programmers (i.e., most programmers).
  - kllrnohj 4y ago
    But why focus on optional then instead of just passing anything by-value? And why not actually dive into when/where this could be of a concern and how to recognize it? It seems like an article aimed neither at beginners nor experts.
- flohofwoe 4y ago
  The (typical C++ stdlib) problem here is that things that look deceptively simple on the surface may result in a surprising amount of activity under the hood (e.g. you need to know how std::string is implemented internally, and that a copy operation may or may not be expensive, maybe depending on the length of the payload, compiler and stdlib implementation, etc etc...). And while the fabled 'zero cost abstraction' often is able to clean up the mess (at the cost of high build times), a small and innocent looking change in the surface code may throw things out of whack again and suddenly you have an unexpected performance regression.
  - bluGill 4y ago
    Most languages don't let you reason about such things. You just copy things around with no clue or care if it is fast or slow. Most problems don't need that level of reasoning, but when it matters C++ gives you a lot of power to ensure you can control when it happens.
    - jay-barronville 4y ago
      I mean, 9x out of 10, the very reason to use a low-level language like C++ is precisely because you want or need to reason about such things, no?
      - bluGill 4y ago
        Hopefully. There are other low level tasks (device drivers) that you might not care, but need C++'s ability to get close to the hardware for, but in general C++ is for when you need both abstractions and the ability to control performance. (Rust, C, ada, and other such languages also play in this space as well, which is best is not something I can comment on)
  - kllrnohj 4y ago
    What innocent looking change are you thinking of that becomes an issue as a result of this? If your function is currently taking a parameter by-value, and you replace it with std::optional<T>, you're still taking it by value and doing the exact same amount of copies. No (new) issues from optional. You only run into trouble if you do something like change `void foo(const T&)` to 'void foo(std::optional<T>)`. But in such a change, it's "clear" (insofar as C++ is ever clear about anything) you're replacing a reference parameter with a value parameter. I don't think this is surprising std::optional behavior? > The (typical C++ stdlib) problem here is that things that look deceptively simple on the surface may result in a surprising amount of activity under the hood It's literally exactly what it looks like. The C++ stdlib didn't complicate or obfuscate anything.
deleted 4y ago
[deleted]
assbuttbuttass 4y ago
This doesn't have anything to do with std::optional. Passing anything by copy works this way in C++
exitb 4y ago
Most of these problems can be solved with moves, if that's the intention. This code does not copy anything and avoids the unnecessary default constructed object: http://okturing.com/src/14823/body http://okturing.com/src/14823/body
- quietbritishjim 4y ago
  Might as well just include that tiny snippet in your comment: A f(std::optional<A> z) { if (z) { return std::move(z.value()); } else { return A(); } } A a("message"); auto z = std::optional<A>(std::move(a)); f(std::move(z)); I don't think you're saying anything fundamentally different from the article. It says you need to exercise care, and you've shown an exmaple that does. "Most of these problems can be solved with moves" - It may be that the problem of copies is solved by using moves (i.e. this is a solved problem at the compiler level). But the problem of needing to be careful is not solved by the advice to use moves (i.e. it's trickier at the human level). It's so easy to forget to use move and you would get no warning about it. It doesn't help that the rules are very complicated. For example, if you were returning z rather than z.value() from your function (with correspondingly different return type), the usual advice is to return z rather than return std::move(z) because it's already implicitly an rvalue in that situation and using move() prevents NRVO. But... is that still true for z.value()? I don't think so but it's not obvious (IMO). Or, if you return std::move(x) where x is a const type, it will copy, which is fair enough as that's all it can do but it ought to warn you (or not compile IMO). It's not always obvious is something is const e.g. if you have a template function, x is of deduced type T (passed by copy) is const part of the deduced type? I'm not sure.
  - kllrnohj 4y ago
    If you're going to say "you need to be careful about copies when passing-by-value and moves are complicated" then I 100% agree. BUT if you're going to say "you need to be careful about std::optional" then I completely disagree. It's a thoroughly unsurprising class with unsurprising behavior. Whether or not the moves do anything or not is entirely dependent on the type T here, not optional, and is therefore orthogonal to whether or not you use optional. Ie, optional did not introduce any new concerns, and therefore doesn't warrant any particular scrutiny or concern.
secondcoming 4y ago
There is a push in C++ towards Value Semantics. The compilers don't seem good enough for it yet. It's a problem with overuse of `auto` too, copies everywhere and I'd wager a lot of them are unintended copies. But hey, your code compiled so it must be right!
matthews2 4y ago
Maybe the title could be shortened to "Care is needed to use C++" :-). While it's certainly possible to create high quality software with C++, there are a lot of traps (like this).
s9w 4y ago
[dead]
kadoban 4y ago
The article's example of using std::optional doesn't seem very realistic, so its counts of copies don't feel compelling at all. Their code is causing almost all of them. They also didn't confirm with a compiler which will actually be copies or what can be elided (either automatically or manually/by better use of types), making it even less compelling.
shadowgovt 4y ago
Care is needed to do anything non-trivial in C++.
hugs_vs_toph 4y ago
Clueless article upvoted by cpp haters. That code is working as intended. That ain't C#. Want to avoid copies? Use std::move.
moloch-hai 4y ago
Care is needed when writing code.
thom 4y ago
I am not sure this is particular to std::optional. There is an awful lot to grasp about values in C++ before you can intuit the right thing to do in every situation: https://www.youtube.com/watch?v=tH0Z2OvHAd8 https://www.youtube.com/watch?v=tH0Z2OvHAd8
throway2kl4223 4y ago
It's an immediate red flag to see `T::T(T)` used anywhere for non-trivial objects or PODs or something not word-size. Typically C++ requires a lot of plumbing to use value semantics with `std::move` everywhere and `std::forward`. I don't have patience for it anymore, so I just use Rust which is move by default.
- kllrnohj 4y ago
  A "move" of a POD value type is still a copy and std::move/std::forward don't do anything (same in Rust for Copy traits). Rather the Rust feature that's missing here is the compiler automatically converting pass-by-(move)value to a pass-by-reference under the covers.
olliej 4y ago
I honestly assumed this would be yet another safety issue, but it turns out it's just another example of my nemesis: the ease by which a reference becomes a value and triggers a copy. It's super annoying because the the messaging is "use references to be a pointer that cannot be null", which fails on multiple levels - this one where it becomes a value copy, but also that references can happily be null (many years ago I filed a feature request on clang asking for a non-null attribute that was rejected because references exist).

21 points by dilawar 4y ago

The "care" seems to just be "thing that looks like a value type is a value type." I'm not sure why the author would have ever expected anything different?

CoastalCoder 4y ago
> I'm not sure why the author would have ever expected anything different? Perhaps the message is aimed at less experienced C++ programmers?
omnicognate 4y ago
Yeah, that's the whole point of std::optional. Pointers (including smart ones) have the possibility of being empty/pointing-to-nothing built in. std::optional adds it to value types. To avoid tons of unnecessary copies use move semantics, which std::optional of course supports.
jay-barronville 4y ago
Fair point, but I didn’t gather that the author expected anything different; I think he simply wanted to highlight a behavior that may not be that obvious to programmers who aren’t expert C++ programmers (i.e., most programmers).
- kllrnohj 4y ago
  But why focus on optional then instead of just passing anything by-value? And why not actually dive into when/where this could be of a concern and how to recognize it? It seems like an article aimed neither at beginners nor experts.
flohofwoe 4y ago
The (typical C++ stdlib) problem here is that things that look deceptively simple on the surface may result in a surprising amount of activity under the hood (e.g. you need to know how std::string is implemented internally, and that a copy operation may or may not be expensive, maybe depending on the length of the payload, compiler and stdlib implementation, etc etc...). And while the fabled 'zero cost abstraction' often is able to clean up the mess (at the cost of high build times), a small and innocent looking change in the surface code may throw things out of whack again and suddenly you have an unexpected performance regression.
- bluGill 4y ago
  Most languages don't let you reason about such things. You just copy things around with no clue or care if it is fast or slow. Most problems don't need that level of reasoning, but when it matters C++ gives you a lot of power to ensure you can control when it happens.
  - jay-barronville 4y ago
    I mean, 9x out of 10, the very reason to use a low-level language like C++ is precisely because you want or need to reason about such things, no?
    - bluGill 4y ago
      Hopefully. There are other low level tasks (device drivers) that you might not care, but need C++'s ability to get close to the hardware for, but in general C++ is for when you need both abstractions and the ability to control performance. (Rust, C, ada, and other such languages also play in this space as well, which is best is not something I can comment on)
- kllrnohj 4y ago
  What innocent looking change are you thinking of that becomes an issue as a result of this? If your function is currently taking a parameter by-value, and you replace it with std::optional<T>, you're still taking it by value and doing the exact same amount of copies. No (new) issues from optional. You only run into trouble if you do something like change `void foo(const T&)` to 'void foo(std::optional<T>)`. But in such a change, it's "clear" (insofar as C++ is ever clear about anything) you're replacing a reference parameter with a value parameter. I don't think this is surprising std::optional behavior? > The (typical C++ stdlib) problem here is that things that look deceptively simple on the surface may result in a surprising amount of activity under the hood It's literally exactly what it looks like. The C++ stdlib didn't complicate or obfuscate anything.

deleted 4y ago

[deleted]

assbuttbuttass 4y ago

This doesn't have anything to do with std::optional. Passing anything by copy works this way in C++

exitb 4y ago

Most of these problems can be solved with moves, if that's the intention. This code does not copy anything and avoids the unnecessary default constructed object: http://okturing.com/src/14823/body http://okturing.com/src/14823/body

quietbritishjim 4y ago
Might as well just include that tiny snippet in your comment: A f(std::optional<A> z) { if (z) { return std::move(z.value()); } else { return A(); } } A a("message"); auto z = std::optional<A>(std::move(a)); f(std::move(z)); I don't think you're saying anything fundamentally different from the article. It says you need to exercise care, and you've shown an exmaple that does. "Most of these problems can be solved with moves" - It may be that the problem of copies is solved by using moves (i.e. this is a solved problem at the compiler level). But the problem of needing to be careful is not solved by the advice to use moves (i.e. it's trickier at the human level). It's so easy to forget to use move and you would get no warning about it. It doesn't help that the rules are very complicated. For example, if you were returning z rather than z.value() from your function (with correspondingly different return type), the usual advice is to return z rather than return std::move(z) because it's already implicitly an rvalue in that situation and using move() prevents NRVO. But... is that still true for z.value()? I don't think so but it's not obvious (IMO). Or, if you return std::move(x) where x is a const type, it will copy, which is fair enough as that's all it can do but it ought to warn you (or not compile IMO). It's not always obvious is something is const e.g. if you have a template function, x is of deduced type T (passed by copy) is const part of the deduced type? I'm not sure.
- kllrnohj 4y ago
  If you're going to say "you need to be careful about copies when passing-by-value and moves are complicated" then I 100% agree. BUT if you're going to say "you need to be careful about std::optional" then I completely disagree. It's a thoroughly unsurprising class with unsurprising behavior. Whether or not the moves do anything or not is entirely dependent on the type T here, not optional, and is therefore orthogonal to whether or not you use optional. Ie, optional did not introduce any new concerns, and therefore doesn't warrant any particular scrutiny or concern.

secondcoming 4y ago

There is a push in C++ towards Value Semantics. The compilers don't seem good enough for it yet. It's a problem with overuse of `auto` too, copies everywhere and I'd wager a lot of them are unintended copies. But hey, your code compiled so it must be right!

matthews2 4y ago

Maybe the title could be shortened to "Care is needed to use C++" :-). While it's certainly possible to create high quality software with C++, there are a lot of traps (like this).

s9w 4y ago

[dead]

kadoban 4y ago

The article's example of using std::optional doesn't seem very realistic, so its counts of copies don't feel compelling at all. Their code is causing almost all of them. They also didn't confirm with a compiler which will actually be copies or what can be elided (either automatically or manually/by better use of types), making it even less compelling.

shadowgovt 4y ago

Care is needed to do anything non-trivial in C++.

hugs_vs_toph 4y ago

Clueless article upvoted by cpp haters. That code is working as intended. That ain't C#. Want to avoid copies? Use std::move.

moloch-hai 4y ago

Care is needed when writing code.

thom 4y ago

I am not sure this is particular to std::optional. There is an awful lot to grasp about values in C++ before you can intuit the right thing to do in every situation: https://www.youtube.com/watch?v=tH0Z2OvHAd8 https://www.youtube.com/watch?v=tH0Z2OvHAd8

throway2kl4223 4y ago

It's an immediate red flag to see `T::T(T)` used anywhere for non-trivial objects or PODs or something not word-size. Typically C++ requires a lot of plumbing to use value semantics with `std::move` everywhere and `std::forward`. I don't have patience for it anymore, so I just use Rust which is move by default.

kllrnohj 4y ago
A "move" of a POD value type is still a copy and std::move/std::forward don't do anything (same in Rust for Copy traits). Rather the Rust feature that's missing here is the compiler automatically converting pass-by-(move)value to a pass-by-reference under the covers.

olliej 4y ago

I honestly assumed this would be yet another safety issue, but it turns out it's just another example of my nemesis: the ease by which a reference becomes a value and triggers a copy. It's super annoying because the the messaging is "use references to be a pointer that cannot be null", which fails on multiple levels - this one where it becomes a value copy, but also that references can happily be null (many years ago I filed a feature request on clang asking for a non-null attribute that was rejected because references exist).