How to make a safer C++ variant visitor, similar to switch statements?
Asked Answered
P

3

37

The pattern that a lot of people use with C++17 / boost variants looks very similar to switch statements. For example: (snippet from cppreference.com)

std::variant<int, long, double, std::string> v = ...;

std::visit(overloaded {
    [](auto arg) { std::cout << arg << ' '; },
    [](double arg) { std::cout << std::fixed << arg << ' '; },
    [](const std::string& arg) { std::cout << std::quoted(arg) << ' '; },
}, v);

The problem is when you put the wrong type in the visitor or change the variant signature, but forget to change the visitor. Instead of getting a compile error, you will have the wrong lambda called, usually the default one, or you might get an implicit conversion that you didn't plan. For example:

v = 2.2;
std::visit(overloaded {
    [](auto arg) { std::cout << arg << ' '; },
    [](float arg) { std::cout << std::fixed << arg << ' '; } // oops, this won't be called
}, v);

Switch statements on enum classes are way more secure, because you can't write a case statement using a value that isn't part of the enum. Similarly, I think it would be very useful if a variant visitor was limited to a subset of the types held in the variant, plus a default handler. Is it possible to implement something like that?

EDIT: s/implicit cast/implicit conversion/

EDIT2: I would like to have a meaningful catch-all [](auto) handler. I know that removing it will cause compile errors if you don't handle every type in the variant, but that also removes functionality from the visitor pattern.

Perithecium answered 16/8, 2017 at 7:45 Comment(14)
Doesn't the snippet just above this one on en.cppreference do exactly what you want?Reenareenforce
Yesterday I learned that there is no thing called "implicit cast", because all casts are explicit. The phrase you are looking for is "implicit conversion" :) https://mcmap.net/q/426860/-c-how-can-i-access-to-an-inner-enum-classDevoe
@Holt, the one with constexpr if? I think it has the same pitfall.Stanford
@kim366 thank you. In either case this feature in C++ is an abomination. :)Stanford
@MichałBrzozowski Try it, and try adding any type to the variant, you'll see that your code won't compile anymore, even if the type is implicitly convertible to another one (e.g. try adding char).Reenareenforce
If you provide default in your switch, you have the same issue. (which is the equivalent of auto arg here).Retributive
Sorry that the question is confusing. What I mean is that if you put float instead of double in the variant signature, the visitor can still have a double handler, and any float value will trigger the [](auto) handler. My aim is to get a compiler error instead. @Holt: I tried the same with the if constexpr version and the same thing happens, you end up in the last else statement.Stanford
@MichałBrzozowski And the last else statement is a compile time error... I don't see what you want to do that this version does not?Reenareenforce
@Holt: in this case yes, but you will often have a catch-all statement there, just like the default: in a switch.Stanford
@MichałBrzozowski So you want a compiler error when you change your code basically, this is not possible... Compiler do not have memory of previous compilation... How do you want the compiler to know that you want a compile-time error in this case but not in the other one? What would be the difference between adding float and adding std::vector<float> for the compiler?Reenareenforce
@Holt: declare an enum class, then write a non-exhaustive switch on it with a default handler. Then change the name of an element in the enum that you have a case statement for. It causes a compile error, I want the same thing here.Stanford
BTW, I'm not sure in real scenario, it happens often to add an new type implicitly convertible in the variant.Retributive
@MichałBrzozowski This is not the same as adding, this is changing something, in which case this makes more sense. Simply add a static_assert at the beginning of the lambda to check for only possible types.Reenareenforce
This might be helpful: youtube.com/watch?v=mqei4JJRQ7s&t=397sDevoe
R
31

If you want to only allow a subset of types, then you can use a static_assert at the beginning of the lambda, e.g.:

template <typename T, typename... Args>
struct is_one_of: 
    std::disjunction<std::is_same<std::decay_t<T>, Args>...> {};

std::visit([](auto&& arg) {
    static_assert(is_one_of<decltype(arg), 
                            int, long, double, std::string>{}, "Non matching type.");
    using T = std::decay_t<decltype(arg)>;
    if constexpr (std::is_same_v<T, int>)
        std::cout << "int with value " << arg << '\n';
    else if constexpr (std::is_same_v<T, double>)
        std::cout << "double with value " << arg << '\n';
    else 
        std::cout << "default with value " << arg << '\n';
}, v);

This will fails if you add or change a type in the variant, or add one, because T needs to be exactly one of the given types.

You can also play with your variant of std::visit, e.g. with a "default" visitor like:

template <typename... Args>
struct visit_only_for {
    // delete templated call operator
    template <typename T>
    std::enable_if_t<!is_one_of<T, Args...>{}> operator()(T&&) const = delete;
};

// then
std::visit(overloaded {
    visit_only_for<int, long, double, std::string>{}, // here
    [](auto arg) { std::cout << arg << ' '; },
    [](double arg) { std::cout << std::fixed << arg << ' '; },
    [](const std::string& arg) { std::cout << std::quoted(arg) << ' '; },
}, v);

If you add a type that is not one of int, long, double or std::string, then the visit_only_for call operator will be matching and you will have an ambiguous call (between this one and the default one).

This should also works without default because the visit_only_for call operator will be match, but since it is deleted, you'll get a compile-time error.

Reenareenforce answered 16/8, 2017 at 8:13 Comment(4)
visit_only_for might have its operator deleted. (it would avoid to introduce dummy ret_t).Retributive
@Retributive true, I'm not used to delete such kind of functions, but that looks nicer ;) Answer updated.Reenareenforce
template <typename... Args> visit_only_for<Args...> make_visit_only_for(const std::variant<Args...>&) { return {}; } might also be a good addition.Retributive
@Retributive This would break the goal of the operation - If I use make_visit_only_for(w), I'll get a visit_only_for<int, long, float, std::string>, and thus the float visitor would not raise a compile-time error (it would fall back to the template operator). My goal here is to force programmer to explicitly write the variant types in both places so that a change in one place always result in a change in the other.Reenareenforce
R
1

You may add an extra layer to add those extra check, for example something like:

template <typename Ret, typename ... Ts> struct IVisitorHelper;

template <typename Ret> struct IVisitorHelper<Ret> {};

template <typename Ret, typename T>
struct IVisitorHelper<Ret, T>
{
    virtual ~IVisitorHelper() = default;
    virtual Ret operator()(T) const = 0;
};

template <typename Ret, typename T, typename T2, typename ... Ts>
struct IVisitorHelper<Ret, T, T2, Ts...> : IVisitorHelper<Ret, T2, Ts...>
{
    using IVisitorHelper<Ret, T2, Ts...>::operator();
    virtual Ret operator()(T) const = 0;
};

template <typename Ret, typename V> struct IVarianVisitor;

template <typename Ret, typename ... Ts>
struct IVarianVisitor<Ret, std::variant<Ts...>> : IVisitorHelper<Ret, Ts...>
{
};

template <typename Ret, typename V>
Ret my_visit(const IVarianVisitor<Ret, std::decay_t<V>>& v, V&& var)
{
    return std::visit(v, var);
}

With usage:

struct Visitor : IVarianVisitor<void, std::variant<double, std::string>>
{
    void operator() (double) const override { std::cout << "double\n"; }
    void operator() (std::string) const override { std::cout << "string\n"; }
};


std::variant<double, std::string> v = //...;
my_visit(Visitor{}, v);
Retributive answered 16/8, 2017 at 8:54 Comment(0)
B
1

Somewhat based on the visit_only_for example by Holt, I'm currently trying something like this to have a drop-in "tag" to my std::visit calls that prevents forgetting explicit handlers/operators:

//! struct visit_all_types_explicitly
//!
//! If this callable is used in the overload set for std::visit
//! its templated call operator will be bound to any type
//! that is not explicitly handled by a better match.
//! Since the instantiation of operator()<T> will trigger
//! a static_assert below, using this in std::visit forces
//! the user to handle all type cases.
//! Specifically, since the templated call operator is a
//! better match than call operators found via implicit argument
//! conversion, one is forced to implement all types even if
//! they are implicitly convertible without warning.
struct visit_all_types_explicitly {
    template<class> static inline constexpr bool always_false_v = false;

    // Note: Uses (T const&) instead of (T&&) because the const-ref version
    //       is a better "match" than the universal-ref version, thereby
    //       preventing the use of this in a context where another
    //       templated call operator is supplied.
    template<typename T>
    void operator()(T const& arg) const {
        static_assert(always_false_v<T>, "There are unbound type cases! [visit_all_types_explicitly]");
    }
};

using MyVariant = std::variant<int, double>;

void test_visit() {
    const MyVariant val1 = 42;

    // This compiles:
    std::visit(
        overloaded{
            kse::visit_all_types_explicitly(),
            [](double arg) {},
            [](int arg) {},
        },
        val1
        );

    // does not compile because missing int-operator causes
    // visit_all_types_explicitly::operator()<int> to be instantiated
    std::visit(
        overloaded{
            visit_all_types_explicitly(),
            [](double arg) {},
            // [](int arg) {  },
        },
        val1
        );

    // does also not compile: (with static assert from visit_all_types_explicitly)
    std::visit(
        overloaded{
            visit_all_types_explicitly(),
            [](double arg) {},
            // [](int arg) {  },
            [](auto&& arg) {}
        },
        val1
    );

    // does also not compile: (with std::visit not being able to match the overloads)
    std::visit(
        overloaded{
            visit_all_types_explicitly(),
            [](double arg) {},
            // [](int arg) {  },
            [](auto const& arg) {}
        },
        val1
    );
}

For now, this seems to do what I want, and what the OP asked for:

Instead of getting a compile error, you will have the wrong lambda called, usually the default one, or you might get an implicit conversion that you didn't plan.

You intentionally cannot combine this with an "default" / auto handler.

Bobsledding answered 10/6, 2021 at 21:34 Comment(0)

© 2022 - 2025 — McMap. All rights reserved.