Visitors can't return references, but they can with std::variant

[ligfx]

Example of the issue:

#define variant_CONFIG_SELECT_VARIANT 1
#include <variant>
#include "nonstd/variant.hpp"
#include <string>

struct tag_t {};

struct myvisitor {
  const std::string& operator()(const int&) const { throw std::exception(); }
  const std::string& operator()(const tag_t&) const { throw std::exception(); }
  const std::string& operator()(const std::string& s) const { return s; }
};

void test_std() {
   std::variant<int, tag_t, std::string> v("hello");
   std::visit(myvisitor(), v);
}

void test_nonstd() {
   nonstd::variant<int, tag_t, std::string> v("hello");
   nonstd::visit(myvisitor(), v);
}

int main() {
   test_std();
   test_nonstd();
}

The STL version of variant compiles and works. nonstd::variant throws a compile error:

./nonstd/variant.hpp:1925:16: error: reference to type 'const std::__1::basic_string<char>' requires an initializer
        return R();

[martinmoene]

Hi @ligfx , Thanks for your message.

Example on Godbolt.

I think a relevant remark related to C++11 and later is No perfect forwarding here in order to simplify code.

---

Used wrong selection of nonstd::variant:

Trying on Godbolt, using clang 7.0.0, -std=c++17 (the first that successfully compiles std::visit(myvisitor(), v);) also successfully compiles for nonstd::variant.

So it seems more specific information is needed to assess what it is you run into, like compiler version and compilation flags.

[ligfx]

Hi @martinmoene, I don't think perfect forwarding is related to this. There are two places in the code that explicitly try to default construct the return type, regardless of which version of C++: https://github.com/martinmoene/variant-lite/blob/a0a1be0d32066ca68cec3d351a71f337637d9ddf/include/nonstd/variant.hpp#L1922 and https://github.com/martinmoene/variant-lite/blob/a0a1be0d32066ca68cec3d351a71f337637d9ddf/include/nonstd/variant.hpp#L2109

When the return type is a const reference, it can't be default constructed.

Since a visitor is required to match all possible alternatives, that code should never be called, but the compiler doesn't like it anyways. What do you think about changing those lines to abort() or std::terminate? Something that the compiler knows won't return.

[martinmoene]

Thanks @ligfx , will look into this later.

[ligfx]

Great, thanks!