Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
AndrewGaspar opened this issue Dec 2, 2020 · 8 comments
Open

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

AndrewGaspar opened this issue Dec 2, 2020 · 8 comments

Comments

@AndrewGaspar
Copy link

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
Copy link
Contributor

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
Copy link
Author

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

@AnthonyVH
Copy link
Contributor

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
Copy link
Author

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
Copy link

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
Copy link

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

@martinmoene
Copy link
Owner

@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
Copy link

Cheers

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants