1

boost::optional can store references, so suppose I want to write a function like this

T frob(const boost::optional<const T&> x) {
    return x.value_or(T{42});
}

This fails on a static_assert that the argument passed to value_or for reference holding optionals should not be an rvalue reference. I see that authors of boost library decided to safeguard me against something like

auto& l = x.value_or(T{42});

where I could get a dangling reference to a temporary.

But if "I know what I'm doing" I could circumvent this with something like

T frob(const boost::optional<const T&> x) {
    return x.value_or(to_lvalue(T{42}));
}

where to_lvalue could be defined like

template<class T>
T& to_lvalue(T&& r) {
    return r;
}

Can an argument be made that this is a good "I know what I'm doing" usecase for such a function or am I missing something bigger here?

unkulunkulu
  • 11,576
  • 2
  • 31
  • 49
  • I am not confident about `boost::optional` but there is something wrong about `boost::optional`. Moreover, it is inherently meaningless to pass `optional` as input parameter. Just pass `const T*` and check if it is a `nullptr`. `optional` is designed to be output of a function not input. – ALX23z Dec 19 '19 at 22:26
  • 5
    Personally I'd use `if (x) return x.value(); return T{42};` – NathanOliver Dec 19 '19 at 22:27
  • I'm leaning towards using a pointer in the actual project, but the question is just inspired by code in the codebase that I stumbled upon when upgrading boost. What I'm probably looking for is a kind of "can of worms demonstration" argument against this pattern. – unkulunkulu Dec 19 '19 at 22:28
  • @NathanOliver-ReinstateMonica, good idea, but then I should probably just refactor this to passing a pointer. – unkulunkulu Dec 19 '19 at 22:28
  • 2
    @ALX23z: Raw pointers might have several meaning (mostly caused of legacy)... C-array or single element + unclear ownership, output parameter style, ... `optional` has clearer meaning IMO. – Jarod42 Dec 19 '19 at 22:31
  • @unkulunkulu Nothing wrong with non-owning raw pointers. My reason to not like `to_lvalue` is someone may see it, not really understand why you are using it and think it really does convert a temporary into an lvalue and start misusing it. The if statement gets rid of that possibility. – NathanOliver Dec 19 '19 at 22:32
  • 2
    Personally I think that authors of boost overdid this a bit, because I could just as easily pass a reference to a local and return it, in my experience those kinds of checks get in the way a little bit without really increasing safety of code that much. – unkulunkulu Dec 19 '19 at 22:38
  • You might still do `T defaultValue{42}; return x.value_or(defaultValue);` to keep original syntax. – Jarod42 Dec 19 '19 at 22:38
  • @Jarod42, that's what was done in a pull request in our project and caught my attention as being quite weird, then I dug deeper. – unkulunkulu Dec 19 '19 at 22:39
  • I kind of feel all this is ugly, but maybe someone could provide a scary enough scenario that would completely discourage me from even thinking about that :) – unkulunkulu Dec 19 '19 at 22:40
  • @Jarod42 if you write internal code (or open source that client compiles themselves) then arrays you pass as vectors - never via a "pointer". If you write external code (you provide shared library or dll) then you cannot pass the data as a vector not you can use optional to pass data. Your only choise is to pass data as raw pointers and provide explanation to it. Just decide who this interface is for. – ALX23z Dec 19 '19 at 22:42
  • 1
    @unkulunkulu: Rvalue-ness is commonly misused as a proxy for an argument having a shorter lifetime than the return value. – Davis Herring Dec 20 '19 at 03:28

0 Answers0