mpq_rational asserts when used in variant

[András Kucsma <r0maikx02b@…>]

The following code fails with an assertion:

#include <boost/variant.hpp>
#include <boost/multiprecision/gmp.hpp>
 
struct Foo {
        Foo() {}
        Foo(const Foo&) {}
};
 
int main() {
        typedef boost::multiprecision::mpq_rational Num;
        typedef boost::variant<Foo, Num> Variant;
 
        Variant x = Num(2);
        Variant y = std::move(x);
 
        x = Foo();
}

Assert text:

test: /usr/local/include/boost/multiprecision/gmp.hpp:1947: __mpq_struct (& boost::multiprecision::backends::gmp_rational::data())[1]: Assertion `m_data[0]._mp_num._mp_d' failed.
Aborted

Some notes on the code:

• The explicit copy constructor for Foo is necessary
• Replacing mpq_rational with mpz_int or cpp_rational doesn't cause assertion
• Using boost::move without the -std=c++11 flag doesn't assert. (but it does assert with -std=c++11)

Tested on Linux with:

• g++-4.8.1, g++-4.8.2, clang 3.5 (trunk 197340)
• boost-1.55.0 and trunk revision: 86799
• libgmp 5.1.0

Compiled with the command: (clan)g++ -std=c++11 main.cpp /usr/local/lib/libgmp.a -O0 -g

The source of the problem could very well be in how variant handles move semantics in C++11 mode.

Note: you can also find this file here: ​https://gist.github.com/r0mai/7978397

[John Maddock]

I'm unable to reproduce here - but I do notice that gmp_rational, unlike the other classes, does not allow a move-copy from an already moved-from source object. I've committed a tentative fix (see ​https://github.com/boostorg/multiprecision/commit/f552968b21b2fd37c7ce69cb283d12d2c2a765bc), can you please try that out and report back?

Many thanks, John.

[András Kucsma <r0maikx02b@…>]

Everything looks good with the patch. It doesn't assert anymore and behaves as expected.

Thank you!

[John Maddock]

That's a relief then!

Closing down.

[John Maddock]

See also ​https://github.com/boostorg/multiprecision/commit/787cd1101e4bc1446206f6e345d023ca18ded09a

I'm reopening and reassigning this as I don't really see why Variant would be move-copying from an already moved-from object. It might not be an issue, but it feels like a bug...

[Antony Polukhin]

It's not a bug, boost::variant tries to satisfy the never-empty guarantee:

• boost::multiprecision::mpq_rational has no noexcept default constructor because default constructor of gmp_rational is not marked with noexcept
• Foo has no noexcept default constructor
• because of previous two points variant can not use boost::multiprecision::mpq_rational or Foo as fallback types
• copy constructor of Foo is not noexcept, there is no fallback type in variant, so it attempts to copy/move content of variant to backup storage before doing an assignment of Foo.
• ...

Looks like gmp_rational calls only pure C methods. Can the default constructor of gmp_rational be marked with noexcept?

[John Maddock]

Looks like gmp_rational calls only pure C methods. Can the default constructor of gmp_rational be marked with noexcept?

Not really, as those methods perform memory allocation, and the allocator used can be a user-supplied custom one (which throws). In any case, if you're happy copying an already-moved-from-object and there's really no alternative, then that's fine. Closing down.

[Antony Polukhin]

Replying to johnmaddock:

In any case, if you're happy copying an already-moved-from-object and there's really no alternative, then that's fine. Closing down.

I'm not 100% happy with that, but the only solution I see is to totally disable move assignments for cases when there is no fallback type. This can break user's code where variant is used with move-only types and there's no fallback. I'm in doubt.