documentation on internals not up to date

[Niels Möller]

"Marco Bodrato" <bodrato at mail.dm.unipi.it> writes:

> from gmp-bugs.
>
> Il Ven, 27 Aprile 2018 7:21 am, Niels Möller ha scritto:
>> "Marco Bodrato" <bodrato at mail.dm.unipi.it> writes:
>
>>> Currently there's always at least one readable limb, so for instance
>>> @code{mpz_get_ui} can fetch @code{_mp_d[0]} unconditionally (though its
>>> value is then only wanted if @code{_mp_size} is non-zero).
>>
>> Maybe change "is only wanted if" to "is valid only if" or "is usable
>> only if". At some point, I suffered the misunderstanding that _mp_size
>> == 0 implies _mp_d[0] == 0, which isn't right.
>
> Do you mean that we should underline that also with _mp_size == 0, any
> limb above the size of the number is undefined?

This is a minor point, but.... The thing is, that when we say that
_mp_d[0] is readable but "not wanted", that leaves it somewhat unclear
what the value might be, maybe it's always a redundant zero? I think we
should be more explicit, making it clear from the docs that when we read
_mp_d[0] and _mp_size == 0, the value read must be considered garbage
and not used in any way.

>> _mp_alloc == 0 and _mp_size != 0 is a read-only value, _mp_d is neither
>> written, reallocated or freed by mpz functions. It must not be passed as
>> destination argument to any mpz function. Should also link to docs for
>> mpz_roinit_n and MPZ_ROINIT_N.
>
> Currently, if an mpz_t is initialised with _roinit, it can be passed to
> _clear or _clears with no errors. Should we document this? I think we
> should.
>
> Moreover, the various mpz_set_ functions should work smoothly too.

I'd prefer that we not document any way to pass _roinit values to any
mpz functions taking a non-const mpz_t, even if it happens to work in
the current implementation. Maybe as a later extension, *if* we find
some use cases where it provides a significant advantage.

I'd also consider backing that up by having the realloc function abort
in case _mp_alloc == 0 but _mp_size != 0.

> And I'm
> quite sure that any mpz function is able to "overwrite" an mpz_t that was
> initialised with _roinit. I mean, the following code actually works:
>
> int main(void)
> {
>   mpz_t f0, f1, fn;
>   int n = 10;
>   mp_limb_t dummy = 1;
>
>   mpz_roinit_n (f0, &dummy, 1);
>   mpz_roinit_n (f1, &dummy, 1);
>   mpz_roinit_n (fn, &dummy, 0);
>
>   for(;--n;) {
>     mpz_add (fn, f0, f1);
>     mpz_swap (f0, f1);
>     mpz_swap (f1, fn);
>   }
>   gmp_printf ("%Zd\n", f1);
>   mpz_clears (f0, f1, fn, NULL);
> }

Even if it works, I don't think we should document or encourage such
use. I'd recommend this way to add two roinit values is

    mpz_t f0, f1, fn;
    mpz_init(fn);
    mpz_add (fn, mpz_roinit_n (f0, ...), mpz_roinit_n (f1, ...));

And switching arguments above, say

    mpz_add (mpz_roinit_n (f0, ...), fn, mpz_roinit_n (f1, ...));

would give a compile time warning because the roinit return value is
const. Which is a good think, imo.

Regards,
/Niels

-- 
Niels Möller. PGP-encrypted email is preferred. Keyid 368C6677.
Internet email is subject to wholesale government surveillance.