mp_ptr and mp_srcptr

[Emmanuel Thomé]

On Thu, Feb 03, 2011 at 09:23:51PM +0100, Torbjorn Granlund wrote:
> bodrato at mail.dm.unipi.it writes:
> 
>   On Thu, January 27, 2011 8:06 am, Paul Zimmermann wrote:
>   > on December 17, Vincent Lef?vre asked a question about mp_ptr and
>   > mp_srcptr
>   
>   I do not have a firm opinion on the subject: should mp_ptr and mp_srcptr
>   be documented or not?
[...]
>   
> I too don't have strong opinions.  I suppose I agree with Marco.
[...]


Well, I may have a relatively strong opinion, depending on what we are
exactly talking about.

I don't think it's necessary to export mp_ptr and mp_srcptr. Put
otherwise, I don't have a very strong opinion on this either.  The base
type of the mpn layer is documented as mp_limb_t *, functions prototypes
are documented as taking mp_limb_t * and const mp_limb_t *. Up to the
user to abbreviate these. But as long as mp_limb_t is still a plain old
datatype, it's not much of an issue.

IMO, it's an entirely different story for mpz's (and other gmp types). I
personally have been using the mpz_ptr and mpz_srcptr for a long time,
without ever noticing they weren't documented. They are exported, though.
The prototypes which appear in the documentation of gmp make no mention
of the constness properties of the exported functions, which I regret.
This makes the two inconsistent. The documentation says:
        void mpz_add(mpz_t, mpz_t, mpz_t);
while in reality (mangling ignored), gmp.h exports:
        void mpz_add(mpz_ptr, mpz_srcptr, mpz_srcptr);
Points in favor of documenting these would be:

 - I believe it would be much cleaner if the documented prototypes
   matched the actually exported ones. Think about Joe User willing to
   have a function pointer value like:
        void (*add_or_sub)(mpz_t, mpz_t, mpz_t);
   defined so because Joe blindly read the documentation. To his dismay,
   Joe encounters a warning error on
        add_or_sub = mpz_add;

 - I do want to be able to qualify the mpz arguments to my functions as
   being const. Okay, I can define foo(const mpz_t a). However I think
   it's good practice to be cautious about doing so for a data type which
   is not an immediate datatype. const-ness and arrays aren't always a
   very happy match in plain C.

 - Another minor point to favor mpz_srcptr against const mpz_t :
   eventually, the former is exposed to the user anyway through the
   warning messages.  Not documenting it, and silently mandating the
   const mpz_t syntax instead has the impact of exposing __mpz_struct in
   warning messages, which looks ugly:
        expected ‘mpz_ptr’ but argument is of type ‘const struct __mpz_struct *’
   I much prefer:
        expected ‘mpz_ptr’ but argument is of type ‘mpz_srcptr’
   which is what I get when my buggy function is
        void bar(mpz_ptr u, mpz_srcptr a) { mpz-add(a,u,a); }

> Slightly related is whether mp_srcptr is useful at all.  OK, it might
> prevent an accidental write to a read-only parameter, but it does not
> AFAIK help compilers at all.

true. But semantically, it does help to prevent coding mistakes.

And (although I know this is not really a compelling argument ;-) for C++
it's really important, because there you want const-correctness as deep as
you can, in order to be able to avoid extraneous copies.

And indeed, mpz prototypes _are_ exported correctly const-qualified. It
would make sense to document it.

> A major problem with C's semantics is that a write through a pointer of
> type T may modify any object of type T, whether declared const or not.
> The const declaration should not be interpreted as a declaration for the
> target object, it just serves to say that it may not be modified through
> *this* pointer.

Sure. And ? I don't see why it would be a reason for not documenting
const-ness of interfaces.

> We might instead in most places in GMP use a stronger statement; the c99
> restrict declarator might be what we need.  (I am not suggesting we
> should replace const by restrict; the latter would be used for both read
> and write pointers in certain functions.)

Yes this could be something to consider, for the interfaces which forbid
overlaps.

Best,

E.