[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

From: Paul Eggert
Subject: Re: [PATCH] Change module interface to no longer use GMP objects directly.
Date: Mon, 18 Nov 2019 13:21:53 -0800
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.2.0

Thanks. I like the idea of decoupling Emacs modules from GMP.

The proposed approach will typically require copying bignum data twice to do a conversion, even if modules use GMP themselves. Shouldn't there be a shortcut for modules using GMP, so that they can use mpz_limbs_write and mpz_limbs_finish and bypass one of the copies? We can do that even if the module interface itself doesn't include gmp.h or assume GMP. For example, the Emacs module interface could define a type emacs_limb_t that happens to be the same size and representation as mpz_limb_t when Emacs is implemented via GMP, and the example function could look like the attached handwavy file "bigt.c". (It might be possible to avoid even the remaining copy in some cases; I didn't investigate this.)

Without such a shortcut, then we are to some extent giving up on efficiency, and in that case shouldn't we just ask people to convert to text and back, thus simplifying the interface?

On 11/17/19 10:38 AM, Philipp Stephani wrote:

> +@deftypefn Function bool extract_big_integer (emacs_env *@var{env}, emacs_value @var{arg}, int *@var{sign}, ptrdiff_t *@var{count}, unsigned long *@var{magnitude})
> +This function, which is available since Emacs 27,

Should we be more systematic about saying which Emacs versions support which parts of the module API?

Uses of 'unsigned long' should be changed to a more-abstract integer type like 'emacs_limb_t', to give the implementation more leeway to change in future versions.

The extraction API looks a bit complicated. How about if we simplify it so that it (1) never signals an error and (2) it returns the limb count instead of always returning 'true' and storing the limb count via a pointer. Any too-small array can be reported merely by returning a limb count larger than the array size, without changing the array, as in the attached sample code.

> extracts the
> +integral value of @var{arg}.

Need to make it clearer that ARG can be any integer; it need not be a bignum.

> it stores the required array
> +size into @code{*count}

It should document that this size cannot exceed (min (PTRDIFF_MAX, SIZE_MAX) / sizeof (emacs_limb_t)). No matter what we do inside Emacs, it won't exceed that value for various reasons, and documenting this can simplify callers.

> + bool success = env->extract_big_integer (env, value, &sign, &count, NULL);

Shouldn't that 'value' be 'arg'?

> +  mpz_import (value, count, order, size, endian, nails, magnitude);

Shouldn't that 'value' be 'result'?

At this point I stopped detailed review because I wanted your thoughts on the bigger questions.

Attachment: bigt.c
Description: Text Data

reply via email to

[Prev in Thread] Current Thread [Next in Thread]