Boost variant of references and equality comparison

Question

The following program aborts:

#include <boost/variant.hpp>

using variant_type = boost::variant< int&, std::string& >;

int main () {
    int a, b;

    variant_type v (a), u (b);
    v == u;

    return 0;
}

with:

$ g++ -std=c++14 t.cpp && ./a.out 
a.out: /opt/boost/default/include/boost/variant/detail/forced_return.hpp:39: T boost::detail::variant::forced_return() [with T = const int&]: Assertion `false' failed.
Aborted (core dumped)

AFAICT you can't compare for equality between variants of non-const references because of a wrong overload of operator function-call being selected in class known_get. known_get is instantiated for const T in the comparer visitor instead of what seems should have been T (variant.hpp:905 in v1.59.0).

Am I missing something?

I can't see anything about variants of references in the boost documentation. so UB? — Richard Hodges, Oct 23 '15 at 17:35
I don't think so. While it's true that the docs and esp. the tutorial mentions nothing about storing references there seems to be no impediment to using variants of these. — Engineerist, Oct 23 '15 at 17:46

Barry · Accepted Answer · 2015-10-23T18:39:41.523

I think this is a Boost bug.

The type requirements here are:

CopyConstructible or MoveConstructible.

Destructor upholds the no-throw exception-safety guarantee.

Complete at the point of variant template instantiation. (See boost::recursive_wrapper<T> for a type wrapper that accepts incomplete types to enable recursive variant types.)

as well as:

EqualityComparable: variant is itself EqualityComparable if and only if every one of its bounded types meets the requirements of the concept.

A reference type is copy constructible, is no-throw destructible, complete, and equality comparable. So we're good on all points there. The issue is that the the visitor used in the implementation is:

template <typename T>
bool operator()(const T& rhs_content) const
{
    // Since the precondition ensures lhs and rhs types are same, get T...
    known_get<const T> getter;
    const T& lhs_content = lhs_.apply_visitor(getter);

    // ...and compare lhs and rhs contents:
    return Comp()(lhs_content, rhs_content);
}

That is, we're using const T as the known getter. This is fine for non-reference types, but incorrect for reference types, since known_get asserts if it gets the wrong type:

T& operator()(T& operand) const BOOST_NOEXCEPT
{
    return operand;
}

template <typename U>
T& operator()(U& ) const
{
    // logical error to be here: see precondition above
    BOOST_ASSERT(false);
    return ::boost::detail::variant::forced_return<T&>();
}

With int&, those overloads become:

const int& operator()(const int& ) const;
const int& operator()(int& ) const; [ U = int ]

The second overload is preferred since the type it refers to would be less const-qualified than the non-template overload. That's why you get the assert, and this behavior is clearly incorrect. We should be able to compare references!

The simpler solution would be to drop the consts from comparer and simply use:

template <typename T>
bool operator()(T& rhs_content) const
{
    known_get<T> getter;
    T& lhs_content = lhs_.apply_visitor(getter);

    return Comp()(lhs_content, rhs_content);
}

This would work for reference types as well as const types.

Yeah, my worry was that I was missing something obvious in there as to why known_get is instantiated for const T instead of T. I will try and file a bug with boost and then get back to voting. — Engineerist, Oct 23 '15 at 18:47

score 0 · Answer 2 · answered Oct 23 '15 at 17:51

ok, I had a think about this (and another look at the documentation).

The synopsis for boost::variant does not show an operator== being defined for a variant.

This leads me to suggest that the correct approach for comparison is via a binary visitor.

here is your (modified) code again, which compiles on my machine (apple clang) your code crashed my compiler too.

#include <string>
#include <boost/variant.hpp>

using variant_type = boost::variant< int&, std::string& >;

struct is_equal
{
    // compare int with int
    bool operator()(const int& l, const int& r) const {
        return l == r;
    }

    // compare string with string
    bool operator()(const std::string& l, const std::string& r) const {
        return l == r;
    }

    // anything else compared with anything else compares false.        
    template<class X, class Y>
    bool operator()(const X&, const Y&) const {
        return false;
    }
};

int main () {
    int a = 0, b = 0;

    variant_type v = a, u = b;

    bool ok = boost::apply_visitor(is_equal(), v, u);
//    bool ok = (v == u);

    return 0;
}

Richard, there is an operator== defined at line 2236 in variant.hpp header. Your solution is something I would have implemented, too, had that operator not been already there. For now I chalk this failure to a possible bug, because if you change the const T into a T in the template argument list of known_get at line 905 of variant.hpp, the code compiles and runs ok. — Engineerist, Oct 23 '15 at 18:34
agreed if the operator== is supposed to be part of the public interface then it's looking like a bug. It's surprising that it crashes both clang and gcc :) — Richard Hodges, Oct 23 '15 at 18:37

Boost variant of references and equality comparison

2 Answers2