5 ms·

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

by 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.
  - quietbritishjim 4y ago
    OK fair point
  - tsimionescu 4y ago
    I would say that std::optional's value_or() method at least is a foot gun here - as it creates a great opportunity for both an unnecessary temporary object and an unnecessary copy. That in itself merits a "use with care" flag. Also, the fact that std::optional::value() apparently returns a reference, and thus `return opt.value()` is always forced to be a copy (I'm writing this based on another comment, I can't claim to fully understand C++'s extraordinarily complicated value types and their respective limitations) seems like another problem that's somewhat specific to std::optional itself.
    - exitb 4y ago
      ::value() method can both return a reference (bare use) and give up the ownership of the wrapped value (used with std::move). You just have to state your intention correctly. It's not a problem with std::optional specifically, it's just the default C++ behavior. The entire thing works much better in Rust, where move is the default op and copies need to be explicit.
    - kllrnohj 4y ago
      `return opt.value()` is not forced to be a copy, no. It is, as you noted, a reference return. You can of course continue to propagate that as a reference if you want, assuming that makes sense. I don't think I'd agree with a characterization that `value_or` is a footgun? It behaves basically identically to similar features in every other language (like Java's Optional#orElse). If it somehow changed parameter evaluation such that it was deferred then that would be a footgun. But it's of course not doing that.
      - tsimionescu 4y ago
        I'm calling value_or a footgun because it calls both the initial constructor (which is to be expected, as in Java) but, if I understand correctly, a further copy constructor (perhaps ellided if the conditions are right). In Java/C#/Python/Ruby/Haskell/Rust etc, this copy would be guaranteed not to happen (at the cost of a heap allocation in the first 4, in most cases). Of course, even in Java this method is often inferior - it only really makes sense in languages like Haskell or if replaced by a macro of some kind, which would guarantee that the unnecessary value is neither constructed nor copied. Otherwise, an if/else or ? should generally be preferred, especially in a performance-focused language like C++.
- exitb 4y ago
  I agree that it's complex and overly nuanced. I posted this because the article pointed out a problem, said "be careful", but didn't actually offer any way to address it. Just z.value() gives you a reference, which, when returned will yield a copy. So no RVO in this case and the move is the next best thing.

by exitb 4y ago

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.
- quietbritishjim 4y ago
  OK fair point
- tsimionescu 4y ago
  I would say that std::optional's value_or() method at least is a foot gun here - as it creates a great opportunity for both an unnecessary temporary object and an unnecessary copy. That in itself merits a "use with care" flag. Also, the fact that std::optional::value() apparently returns a reference, and thus `return opt.value()` is always forced to be a copy (I'm writing this based on another comment, I can't claim to fully understand C++'s extraordinarily complicated value types and their respective limitations) seems like another problem that's somewhat specific to std::optional itself.
  - exitb 4y ago
    ::value() method can both return a reference (bare use) and give up the ownership of the wrapped value (used with std::move). You just have to state your intention correctly. It's not a problem with std::optional specifically, it's just the default C++ behavior. The entire thing works much better in Rust, where move is the default op and copies need to be explicit.
  - kllrnohj 4y ago
    `return opt.value()` is not forced to be a copy, no. It is, as you noted, a reference return. You can of course continue to propagate that as a reference if you want, assuming that makes sense. I don't think I'd agree with a characterization that `value_or` is a footgun? It behaves basically identically to similar features in every other language (like Java's Optional#orElse). If it somehow changed parameter evaluation such that it was deferred then that would be a footgun. But it's of course not doing that.
    - tsimionescu 4y ago
      I'm calling value_or a footgun because it calls both the initial constructor (which is to be expected, as in Java) but, if I understand correctly, a further copy constructor (perhaps ellided if the conditions are right). In Java/C#/Python/Ruby/Haskell/Rust etc, this copy would be guaranteed not to happen (at the cost of a heap allocation in the first 4, in most cases). Of course, even in Java this method is often inferior - it only really makes sense in languages like Haskell or if replaced by a macro of some kind, which would guarantee that the unnecessary value is neither constructed nor copied. Otherwise, an if/else or ? should generally be preferred, especially in a performance-focused language like C++.
exitb 4y ago
I agree that it's complex and overly nuanced. I posted this because the article pointed out a problem, said "be careful", but didn't actually offer any way to address it. Just z.value() gives you a reference, which, when returned will yield a copy. So no RVO in this case and the move is the next best thing.