3

I've got an optional-like class (I can't use optional since it's in C++17). It contains a (possible) value along with a flag indicating if it's valid. I've got an explicit bool operator and a conversion operator to get the value out. The problem is, sometimes C++ will choose the bool operator in an explicitly bool context (an if statement), and other times it won't. Can anyone help me understand this behavior:

#include <algorithm>
#include <stdexcept>

template <typename T>
struct maybe {
    maybe()        : valid_(false) {}
    maybe(T value) : valid_(true)  { new (&value_) T(value); }

    operator T&&() {
        return std::move(value());
    }

    explicit operator bool() const {
        return valid_;
    }

    T& value() {
        if (!valid_) {
            throw std::runtime_error("boom");
        }
        return value_;
    }

private:
    union {
        T value_;
    };
    bool valid_;
};


int main() {
    // fine, uses operator bool()
    maybe<std::pair<int,int>> m0;
    if (m0) {
        std::pair<int,int> p = m0;
        (void)p;
    }

    // throws error, uses operator T&&()
    maybe<double> m1;
    if (m1) {
        double p = m1;
        (void)p;
    }
}   
gct
  • 14,100
  • 15
  • 68
  • 107
  • I am not sure if this is the case, but I suspect it being related to the fact, that any integral value can be used in the boolean context (where zero is consider to be false, and anything but zero, is considered to be false), hence, `double` can be used in such a context, due to being integral, while `pair` cannot, to not being integral value. – Algirdas Preidžius Aug 01 '18 at 16:13
  • double is considered an integral value? – gct Aug 01 '18 at 16:14
  • Amended: `if (static_cast(m1))` still converts to `double` live: https://godbolt.org/g/WFaZk8 This surprised me – Richard Critten Aug 01 '18 at 16:15
  • @RichardCritten `static_cast(const_cast&>(m1))`. – YSC Aug 01 '18 at 16:20
  • @SeanMcAllister As a side note: why do you have a union with only one member? Is there some deeper meaning to this? – freakish Aug 01 '18 at 16:32
  • @freakish A union like that will provide properly aligned and sized space for the types it holds, but won't initialize them. That lets me use placement new and not require default constructibility. – gct Aug 01 '18 at 17:39
  • @SeanMcAllister `if (0.0) { /*Something*/ }` compiled fine for me, on ideone, so I guess that it is, at least, valid in such a context. I am not certain if my wording is correct, or my thought process is correct, hence why I didn't write an answer.. – Algirdas Preidžius Aug 01 '18 at 18:21

3 Answers3

4

Whenever you write:

if (x)

That is equivalent to having written:

bool __flag(x);
if (__flag)

This is called a contextual conversion to bool (note that it's direct-initialization, so the explicit conversion function is a candidate).

When we do overload resolution on that construction for m0, there's only one valid candidate: explicit operator bool() const.

But when we do overload resolution on that construction for m1, there are two: explicit operator bool() const and operator double&&(), because double is convertible to bool. The latter is a better match because of the extra const qualification on the bool conversion function, even though we have to do an extra double conversion. Hence, it wins.

Would simply remove operator T&&() from your interface, as it doesn't make much sense for this type.

Barry
  • 286,269
  • 29
  • 621
  • 977
1

As soon as T is convertible to bool (double is, std::pair is not) your two operators will match and you'll get an ambiguous call, or one may be a better match for some reason.

You should only provide one of the two operators, not both.

YSC
  • 38,212
  • 9
  • 96
  • 149
  • I guess I didn't know double was convertible to bool. I guess bool is an integral type so it does double -> bool directly rather than double -> int -> bool which I guess I had in my head. I think you're right, I'll just rely on the .valid() function I've got. – gct Aug 01 '18 at 16:15
  • If I were you, I'd only keep the bool conversion operator. – YSC Aug 01 '18 at 16:15
  • @SeanMcAllister No, `double` is an arithmetic type. – YSC Aug 01 '18 at 16:16
  • I actually want to be able to do `type val = func_returning_maybe` in my programs and have it throw an exception and terminate if it's invalid, makes file I/O nice and clean. – gct Aug 01 '18 at 16:20
  • @SeanMcAllister I see your an Haskell fan, why won't you use another operator? What about `func_returning_maybe >> val`? – YSC Aug 01 '18 at 16:22
  • I'd have to declare the variable separately in that case, no? I don't know any other good unary operators to use, * seems like it'd be a little to clever... – gct Aug 01 '18 at 16:28
  • Doesn't shock me. I'd use it. – YSC Aug 01 '18 at 16:29
1

Your operator bool and conversion operator are ambiguous in this design.

  • In the first context, std::pair<int,int> does not cast to bool so the explicit bool conversion operator is used.

  • In the second context, double does cast to bool so the T conversion operator is used, which returns a double, which then casts to bool implicitly.

Note in the second context, you are calling std::move which puts value in a valid but undefined state, which leads to undefined behavior when you cast value to double a second time in the if block.

I'd use a named member function to indicate if it is valid, and modify the conversion operator:

#include <algorithm>
#include <stdexcept>

template <typename T>
struct maybe {
    maybe()        : valid_(false) {}
    maybe(T value) : valid_(true)  { new (&value_) T(value); }

    operator T&&() && { return std::move(value_); } // if rvalue
    operator T&() & { return value_; } // if lvalue
    operator const T&() const & { return value_; } // if const lvalue

    bool valid() const { return valid_; }

    T& value() {
        if (!valid_) {
            throw std::runtime_error("boom");
        }
        return value_;
    }

private:
    union {
        T value_;
    };
    bool valid_;
};

int main() {
    // fine, uses operator bool()
    maybe<std::pair<int,int>> m0;
    if (m0.valid()) {
        std::pair<int,int> p = m0;
        (void)p;
    }

    // throws error, uses operator T&&()
    maybe<double> m1;
    if (m1.valid()) {
        double p = m1;
        (void)p;
    }
}   

EDIT: The conversion operator should only move from member value_ if the maybe object is an rvalue reference. Using && after a functions signature specializes for this case -- please see Kerrek SB's answer for more information.

Taylor Nichols
  • 588
  • 2
  • 13
  • Can you elaborate on the trailing &s in the conversion operator definitions? I haven't seen that syntax before. – gct Aug 01 '18 at 16:22
  • 2
    See the section on **ref-qualified member functions** [here](https://en.cppreference.com/w/cpp/language/member_functions) – Useless Aug 01 '18 at 16:28
  • Or actually, [this question](https://stackoverflow.com/q/8610571/212858) might be more informative. – Useless Aug 01 '18 at 16:30
  • @Useless Ah thank you, lets you more precisely define the overload behavior, neat. – gct Aug 01 '18 at 16:34