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

Invalid copy constructor selected in some cases #122

Closed
ukreator opened this issue Oct 5, 2016 · 4 comments
Closed

Invalid copy constructor selected in some cases #122

ukreator opened this issue Oct 5, 2016 · 4 comments
Assignees
Milestone

Comments

@ukreator
Copy link

ukreator commented Oct 5, 2016

Simple code snippet to reproduce the issue:

struct X
{
    template <typename ValueType>
    X(const ValueType&)  {}
};

int main()
{
    mapbox::util::variant<X, int> a{123}; // init with int
    decltype(a) b = a;
    assert(a.which() == b.which());
}

boost::variant works fine in this case. This issue was originally spotted by random failures after replacing boost::variant with map box::variant. One of the types in the list was boost::any which has a templated constructor like in struct X above.

@lightmare
Copy link
Contributor

lightmare commented Oct 5, 2016

This is caused by X being constructible from variant<Types...> and

template<typename T, ...> variant(T&&)

being a better match than

variant(variant<Types...> const& old)

when the actual argument is a non-const reference.

Possible solutions:

  • add non-const copy overload variant(variant<Types...> & old)
  • add && !is_same<variant<Types...>, typename Traits::value_type>::value to enable_if in the universal constructor template

@springmeyer springmeyer added this to the 1.1.3 milestone Oct 5, 2016
@springmeyer
Copy link
Contributor

Thanks for reporting @ukreator and for the thoughts on solutions @lightmare. @artemp is out on vacation this week but I know he's planning a v1.1.x release soon after returning. I've assigned this to 1.1.3 to consider fixing before the next release.

@artemp artemp self-assigned this Oct 21, 2016
@artemp
Copy link
Contributor

artemp commented Oct 24, 2016

@lightmare - thanks for digging! Adding an extra check in universal ctor and explicitly disabling passing variants in there is the winner:

&& !std::is_same<variant<Types...>, typename Traits::value_type>::value

@artemp
Copy link
Contributor

artemp commented Oct 24, 2016

@ukreator - Thanks for reporting and providing test-case !

@artemp artemp closed this as completed in a5a79a5 Oct 24, 2016
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