GMP and C++11 move constructors

[Hans Åberg]

> On 20 May 2018, at 23:52, Marco Bodrato <bodrato at mail.dm.unipi.it> wrote:
> 
> Il Dom, 20 Maggio 2018 2:57 pm, Hans Åberg ha scritto:
>>> On 20 May 2018, at 13:37, Marco Bodrato <bodrato at mail.dm.unipi.it>
> 
>> It is just curious, because there is a value that guarantees deallocators
>> to do nothing, and that is (void*)0.
> 
> This is NOT required for a custom deallocator that someone may want to use
> with GMP. So that a custom deallocator can avoid to check ;-)

Indeed, but even if it does, the GMP API does not guarantee that. So it is not usable, even if it 'free'.

>> The moved from object, or r-value, then must be in a legal state before
>> the destructor is applied. As it is a allocated heap pointer, the simplest
> 
> The destructor is applied, but the object may also be reused, right? You
> seem to assume that, after move, only the destructor can be called on that
> object. I guess this is not what the standard says.

The C++ standard says that after the move, the original object is left in a legal, but unspecified state. So it is not usable for anything else but destruction from the point if view of the language. If you change it by std::move it is up to you to make sure nothing bad happens.

>> The only reason is to use mpz_move is to create an API: If GMP changes the
>> underlying mpz_t, this code might break.
> 
> If the code uses documented features, it will not be broken so easily.

The fields of the mpz_t are undocumented, so they cannot formally be used.

>> Now, instead of
>> integer(integer&& x) {mpz_move(value_, x.value_); mpz_init(x.value_);}
>> one might try
>> integer(integer&& x) {mpz_move(value_, x.value_); mpz_null(x.value_);}
>> using
>> inline void mpz_null(mpz_t x) {
>>   x[0]._mp_alloc = 0;
>>   x[0]._mp_size = 0;
>>   x[0]._mp_d = NULL;    // Setting allocation pointer to null.
>> }
> 
> The only real difference I see, for current development code, is that you
> can inline the function mpz_null.

The inline is only because it is in a header, so that the compiler can remove it, causing no overhead.

>> This works in the current implementation, as it just applies 'free', I
>> think.
>> 
>> But in the new implementation, it will break, since you use another value
>> than NULL.
> 
> I do not know what do you mean with "the current" and "the new"...

The one in the repository does not allocate, but sets it not a non-zero dummy value.

> For sure, if the code uses internals such as _mp_alloc, _mp_size and _mp_d
> instead of documented functions, it is exposed, and it may be broken as
> soon as the next version is out.

Yes, not only may, it will, since it has already been changed.

>> So, returning to your question above, as the GMP API does not explicitly
>> specify that NULL is allowed,
> 
> It does even more, the documentation explicitly _forbids_ NULL.
> 
> For GMP-6.1.2, see https://gmplib.org/manual/Integer-Internals.html
> "there’s always at least one limb allocated, so for instance mpz_set_ui
> never needs to reallocate, and mpz_get_ui can fetch _mp_d[0]
> unconditionally"

Yes, but that does not matters here, as only a destructor mpz_clear will be applied, and it only calls the deallocator, which is 'free' or whatever it is set to.

Also, I have used the same method with a dummy default value, as your new code, but it turned out that it was hard to ensure some code didn't change it. 

> For the current development code,
> see https://gmplib.org/repo/gmp/rev/110bcd4c29f4#l2.20
> "there's always at least one readable limb, so for instance mpz_get_ui can
> fetch _mp_d[0] unconditionally"
> 
> In either cases _mp_d[0] must at least be readable, NULL[0] is not.

That will not happen in this case, as only mpz_clear will be applied.