Re: [PATCH] Change module interface to no longer use GMP objects directly.

[Philipp Stephani]

Am Di., 19. Nov. 2019 um 22:54 Uhr schrieb Paul Eggert <address@hidden>:
>
> On 11/19/19 1:12 PM, Philipp Stephani wrote:
>
> > I'm not against a typedef to provide clearer semantics, but then we'd
> > also have to define EMACS_LIMB_MAX etc., and I see relatively little
> > benefit in that (since we can't realistically change it anyway later).
>
> I can see a benefit, partly for the cleaner semantics, partly as it lets
> us arrange for Emacs limbs to be the same size as GMP limbs. If we can't
> realistically change this later, let's do it the better way the first
> time. It's easy enough to do; for example, we can define EMACS_LIMB_MAX
> to be the equivalent of ((emacs_limb_t) -1).

GMP seems to make a very complex series of choices when selecting the
limb datatype, which we can't and shouldn't replicate. I guess the
best we can do is to use unsigned long long if unsigned long is 4
bytes, and unsigned long otherwise.

>
> > The current "unsigned long" type is identical to mpz_limb_t on most
> > platforms
>
> unsigned long differs from mpz_limb_t on significant-enough platforms
> (MinGW, Cygwin, ...) that it's worth creating an emacs_limb_t to support
> those platforms better.

I really don't want to pollute the emacs-module header with lots of
platform-specific cruft just to support a very specific
micro-optimization.

>
> > I'm not feeling strongly about the function in isolation, but the
> > current interface is modeled after the existing copy_string_contents
>
> Why does copy_string_contents always return true when it returns? If
> there's no good reason, let's have the new function use a cleaner API. A
> flaw in an old function's API does not require us to have a similar flaw
> in the new one.

It's not a flaw, just a different design choice. copy_string_contents
returns false if and only if it has signaled an error, which is a
quite useful invariant.

>
> >> It should document that this size cannot exceed (min (PTRDIFF_MAX,
> >> SIZE_MAX) / sizeof (emacs_limb_t))....
> >
> > It's true that this allows
> > callers to use malloc without checking for overflow, and it seems this
> > already follows from the C standard (can an array with total size
> > larger than SIZE_MAX exist?),
> It does not follow from the C standard, as there's no requirement
> anywhere that Emacs internally must represent each integer as a
> contiguous array.
>
> I found the guarantee useful in the first function I wrote that
> attempted to use the proposed API, so that's good evidence that it's
> worth documenting.

I don't think so, it's a very narrow guarantee that we also don't give
for other functions. Each guarantee, however minor, comes with a cost:
we need to document it and stick to it forever.