3 ms·

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

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++.
    - kllrnohj 4y ago
      https://en.cppreference.com/w/cpp/utility/optional/value_or https://en.cppreference.com/w/cpp/utility/optional/value_or value_or's parameter type is U&&, so no there's no additional copy there. There's only the explicitly visible default constructor call. But you did get me to take a second look at the post, and yeah the author's statement of "Finally, we copy construct an instance of A when the call to value_or terminates." is just incorrect. Try it out yourself: https://godbolt.org/z/hWqnrfGKE https://godbolt.org/z/hWqnrfGKE If you replace the `.value_or(A())` with `.value()` there's no reduction in copies, only a single reduction in the constructed count which of course makes sense since the `A()` parameter isn't there. The way you would optimize this to eliminate copies would be to do things like construct the optional in-place and take the parameter by const& instead, such as: A f(const std::optional<A>& z) { return z.value_or(A()); } A g() { std::optional<A> z(std::in_place, "message"); return f(z); } `std::in_place` being definitely a classic C++ "quirk" here and usually unlikely to be worth it but hey if you want to play "minimize the copy count" golf there you go. The value_or isn't actually adding any copies, though.

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++.
  - kllrnohj 4y ago
    https://en.cppreference.com/w/cpp/utility/optional/value_or https://en.cppreference.com/w/cpp/utility/optional/value_or value_or's parameter type is U&&, so no there's no additional copy there. There's only the explicitly visible default constructor call. But you did get me to take a second look at the post, and yeah the author's statement of "Finally, we copy construct an instance of A when the call to value_or terminates." is just incorrect. Try it out yourself: https://godbolt.org/z/hWqnrfGKE https://godbolt.org/z/hWqnrfGKE If you replace the `.value_or(A())` with `.value()` there's no reduction in copies, only a single reduction in the constructed count which of course makes sense since the `A()` parameter isn't there. The way you would optimize this to eliminate copies would be to do things like construct the optional in-place and take the parameter by const& instead, such as: A f(const std::optional<A>& z) { return z.value_or(A()); } A g() { std::optional<A> z(std::in_place, "message"); return f(z); } `std::in_place` being definitely a classic C++ "quirk" here and usually unlikely to be worth it but hey if you want to play "minimize the copy count" golf there you go. The value_or isn't actually adding any copies, though.