martinmoene / variant-lite

variant lite - A C++17-like variant, a type-safe union for C++98, C++11 and later in a single-file header-only library
Boost Software License 1.0
239 stars 25 forks source link

Argument to nonstd::visit is always a const reference? #42

Open AndrewGaspar opened 3 years ago

AndrewGaspar commented 3 years ago

Try as I might, I cannot use nonstd::visit on C++14 to mutate the item currently stored in a variant. If I change to -std=c++17 (and therefore use std::variant), it works.

Very simple example: https://godbolt.org/z/nsYqxs

AnthonyVH commented 3 years ago

Yes, the overview page explicitly lists all arguments to visit to be const lvalue references. That's indeed different from std::visit which takes its arguments as universal references.

The reason your tests work for C++17 is because when using C++17 nonstd::variant is a typedef for std::variant. You can disable that by defining -Dvariant_CONFIG_SELECT_VARIANT=variant_VARIANT_NONSTD during compilation.

AndrewGaspar commented 3 years ago

What limitation in pre-C++17 stops visit from taking the arguments as non-const lvalue references?

AnthonyVH commented 3 years ago

I'm not really qualified to answer this, since I didn't take a good look at how visit is implemented here.

But my guess is that the code currently implements visit using const lvalue ref, since that makes it compatible with C++98. With a couple of extra ifdefs to select between C++98 or newer, I think implementing visit using universal references for C++11 and up should be possible. But I might be mistaken of course.

AndrewGaspar commented 3 years ago

Oh, I suppose even with C++11, using just plain lvalue references would prevent you from using visit with temporaries, since temporaries can't be passed as a non-const reference... I suppose you have to use universal references to get it to work right.

pfeatherstone commented 3 years ago

I imagine I'm having the same problem, but the following doesn't work for C++ < 17:

#include <cstdlib>
#include <vector>
#include <queue>
#include <nonstd/variant.hpp>

using namespace std;

struct helper_size
{
    template<typename T>
    size_t operator()(const T& item)
    {
        return item.size();
    }
};

/*
 * 
 */
int main(int argc, char** argv) 
{
    using buffer_t = nonstd::variant<std::vector<char>,
                                     std::queue<char>>;

    buffer_t buf;
    size_t size = nonstd::visit(helper_size{}, buf);
    return 0;
}

For C++ >= 17, it's fine.

@martinmoene Is this the desired behaviour?

pfeatherstone commented 3 years ago

If someone wants to write a PR at some point, abseil has an implementation too for reference.

martinmoene commented 3 years ago

@pfeatherstone, Making operator()(const T& item) a const method lets it compile.

At the moment I'm quite removed from the internal workings of nonstd::visit and in how far the limitations are more or less easily overcome (or to split implementation for C++98, and C++11 and later).

struct helper_size
{
    template<typename T>
    size_t operator()(const T& item) const
    {
        return item.size();
    }
};
pfeatherstone commented 3 years ago

Cheers