What's the best way to implement this "on error, throw" callback?

Question

Be warned: There's a lot of background info here before we get to the real question.

I have a fairly wide C++ class hierarchy (representing something like expressions of different types):

class BaseValue { virtual ~BaseValue(); };
class IntValue final : public BaseValue { int get() const; };
class DoubleValue final : public BaseValue { double get() const; };
class StringValue final : public BaseValue { std::string get() const; };

And on the other side, I have a way to coerce the user's input to an expected type:

class UserInput { template<class T> get_as() const; };

So one way to write a matcher — "does the user's input equal this BaseValue's value?" — would be like this:

class BaseValue { virtual bool is_equal(UserInput) const; };
class IntValue : public BaseValue {
    int get() const;
    bool is_equal(UserInput u) const override {
        return u.get_as<int>() == get();
    }
};
// and so on, with overrides for each child class...
bool does_equal(BaseValue *bp, UserInput u) {
    return bp->is_equal(u);
}

However, this doesn't scale, either in the "width of the hierarchy" direction, or in the "number of operations" direction. For example, if I want to add bool does_be_greater(BaseValue*, UserInput), that would require a whole nother virtual method with N implementations scattered across the hierarchy. So I decided to go this route instead:

bool does_equal(BaseValue *bp, UserInput u) {
    if (typeid(*bp) == typeid(IntValue)) {
        return static_cast<IntValue*>(bp)->get() == u.get_as<int>();
    } else if (typeid(*bp) == typeid(DoubleValue)) {
        return static_cast<DoubleValue*>(bp)->get() == u.get_as<double>();
    ...
    } else {
        throw Oops();
    }
}

In fact, I can do some metaprogramming and collapse that down into a single function visit taking a generic lambda:

bool does_equal(BaseValue *bp, UserInput u) {
    my::visit<IntValue, DoubleValue, StringValue>(*bp, [&](const auto& dp){
        using T = std::decay_t<decltype(dp.get())>;
        return dp.get() == u.get_as<T>();
    });
}

my::visit is implemented as a "recursive" function template: my::visit<A,B,C> simply tests typeid against A, calls the lambda if so, and calls my::visit<B,C> if not. At the bottom of the call stack, my::visit<C> tests typeid against C, calls the lambda if so, and throws Oops() if not.

Okay, now for my actual question!

The problem with my::visit is that the on-error behavior "throw Oops()" is hard-coded. I'd really prefer to have the error behavior be user-specified, like this:

bool does_be_greater(BaseValue *bp, UserInput u) {
    my::visit<IntValue, DoubleValue, StringValue>(*bp, [&](const auto& dp){
        using T = std::decay_t<decltype(dp.get())>;
        return dp.get() > u.get_as<T>();
    }, [](){
        throw Oops();
    });
}

The problem I'm having is, when I do that, I can't figure out how to implement the base class in such a way that the compiler will shut up about either mismatched return types or falling off the end of a function! Here's the version without an on_error callback:

template<class Base, class F>
struct visit_impl {
    template<class DerivedClass>
    static auto call(Base&& base, const F& f) {
        if (typeid(base) == typeid(DerivedClass)) {
            using Derived = match_cvref_t<Base, DerivedClass>;
            return f(std::forward<Derived>(static_cast<Derived&&>(base)));
        } else {
            throw Oops();
        }
    }

    template<class DerivedClass, class R, class... Est>
    static auto call(Base&& base, const F& f) {
    [...snip...]
};

template<class... Ds, class B, class F>
auto visit(B&& base, const F& f) {
    return visit_impl<B, F>::template call<Ds...>( std::forward<B>(base), f);
}

And here's what I'd really like to have:

template<class Base, class F, class E>
struct visit_impl {
    template<class DerivedClass>
    static auto call(Base&& base, const F& f, const E& on_error) {
        if (typeid(base) == typeid(DerivedClass)) {
            using Derived = match_cvref_t<Base, DerivedClass>;
            return f(std::forward<Derived>(static_cast<Derived&&>(base)));
        } else {
            return on_error();
        }
    }

    template<class DerivedClass, class R, class... Est>
    static auto call(Base&& base, const F& f, const E& on_error) {
    [...snip...]
};

template<class... Ds, class B, class F, class E>
auto visit(B&& base, const F& f, const E& on_error) {
    return visit_impl<B, F>::template call<Ds...>( std::forward<B>(base), f, on_error);
}

That is, I want to be able to handle both of these cases:

template<class... Ds, class B, class F>
auto visit_or_throw(B&& base, const F& f) {
    return visit<Ds...>(std::forward<B>(base), f, []{
        throw std::bad_cast();
    });
}

template<class... Ds, class B>
auto is_any_of(B&& base) {
    return visit<Ds...>(std::forward<B>(base),
        []{ return true; }, []{ return false; });
}

So I guess one way to do it would be write several specializations of the base case:

when is_void_v<decltype(on_error())>, use {on_error(); throw Dummy();} to silence the compiler warning
when is_same_v<decltype(on_error()), decltype(f(Derived{}))>, use {return on_error();}
otherwise, static-assert

But I feel like I'm missing some simpler approach. Can anyone see it?

A [MCVE] reproducing the compiler warnings that you want to silence would be nice. — Vittorio Romeo, Jun 02 '17 at 19:19
The comma operator is always ready to be abused: http://coliru.stacked-crooked.com/a/78f96318349b604b — Johannes Schaub - litb, Jun 03 '17 at 17:35

score 2 · Answer 1 · answered Jun 02 '17 at 19:25

I guess one way to do it would be write several specializations of the base case

Instead of doing that, you could isolate your "compile-time branches" to a function that deals exclusively with calling on_error, and call that new function instead of on_error inside visit_impl::call.

template<class DerivedClass>
static auto call(Base&& base, const F& f, const E& on_error) {
    if (typeid(base) == typeid(DerivedClass)) {
        using Derived = match_cvref_t<Base, DerivedClass>;
        return f(std::forward<Derived>(static_cast<Derived&&>(base)));
    } else {
        return error_dispatch<F, Derived>(on_error);
//             ^^^^^^^^^^^^^^^^^^^^^^^^^
    }
}

template <typename F, typename Derived, typename E>
auto error_dispatch(const E& on_error) 
    -> std::enable_if_t<is_void_v<decltype(on_error())>>
{
    on_error(); 
    throw Dummy();
}

template <typename F, typename Derived, typename E>
auto error_dispatch(const E& on_error) 
    -> std::enable_if_t<
        is_same_v<decltype(on_error()), 
                  decltype(std::declval<const F&>()(Derived{}))>
    >
{
    return on_error();
}

1. what if `f` returns `void`? 2. Why can't `on_error` return something convertible to the result type? — T.C., Jun 02 '17 at 20:22
This is essentially what I ended up doing, except that I left the SFINAE condition off of the `is_same_v` branch so that it would work if `on_error` returned something convertible to the result type, and so that it would hard-error in the other case. ...But T.C. raises a good point about "what if `f` returns void?", so I will now have to go change that. :) — Quuxplusone, Jun 02 '17 at 21:08

score 1 · Answer 2 · answered Jun 02 '17 at 19:35

1

How about using variant (std C++17, or boost one)? (and use static visitor)

using BaseValue = std::variant<int, double, std::string>;

struct bin_op
{
    void operator() (int, double) const { std::cout << "int double\n"; }
    void operator() (const std::string&, const std::string&) const
    { std::cout << "strings\n"; }

    template <typename T1, typename T2>
    void operator() (const T1&, const T2&) const { std::cout << "other\n"; /* Or throw */ }
};


int main(){
    BaseValue vi{42};
    BaseValue vd{42.5};
    BaseValue vs{std::string("Hello")};

    std::cout << (vi == vd) << std::endl;

    std::visit(bin_op{}, vi, vd);
    std::visit(bin_op{}, vs, vs);
    std::visit(bin_op{}, vi, vs);
}

Demo

answered Jun 02 '17 at 19:35

Jarod42

203,559
14
181
302

Nope, replacing the polymorphic hierarchy with a non-polymorphic `variant` is right out. – Quuxplusone Jun 02 '17 at 21:07
Is the hierarchy fixed ? if yes you can still apply [visitor pattern](https://stackoverflow.com/documentation/design-patterns/4579/visitor-pattern/15127/visitor-pattern-example-in-c#t=201706022117345811126). (and then possibly [multiple dispatch](https://stackoverflow.com/a/29345504/2684539)). – Jarod42 Jun 02 '17 at 21:17
I believe your question is answered by this sentence in my OP: "For example, if I want to add `bool does_be_greater(BaseValue*, UserInput)`, that would require a whole nother virtual method with N implementations scattered across the hierarchy." I'll study https://stackoverflow.com/questions/29286381/multiple-dispatch-solution-with-full-maintainability/29345504#29345504 and see if it's applicable, though. – Quuxplusone Jun 02 '17 at 21:23
With visitor pattern, you would just have a new `struct does_be_greater` with one overload by type if you want to add a new function. but if you want to add `ComplexValue`, it would required to change all existing visitors. – Jarod42 Jun 02 '17 at 21:26
That makes sense; I think I see how it works technically. But stylistically in terms of lines-of-code, isn't that a massive amount of cut-and-paste? `struct does_be_greater { bool go(IntValue& p, UserInput u) { return ...; } bool go(DoubleValue& p, UserInput u) { return ...; } ... };` — Is there a way to eliminate all that boilerplate (other than macros of course)? – Quuxplusone Jun 02 '17 at 21:57
You may still use template if they share implementation. – Jarod42 Jun 03 '17 at 14:53

What's the best way to implement this "on error, throw" callback?

2 Answers2