Re: emacs-module.c, eassert, and nonnull args

[Philipp Stephani <p.stephani2@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1961 bytes --]

Philipp Stephani <p.stephani2@gmail.com> schrieb am So., 11. Juni 2017 um
22:34 Uhr:

> Paul Eggert <eggert@cs.ucla.edu> schrieb am So., 11. Juni 2017 um
> 19:45 Uhr:
>
>> Philipp Stephani wrote:
>> > Here's a new patch. It makes this change, and also changes the
>> allocation
>> > slightly so that objects have unique addresses (useful for checking).
>>
>> Thanks for working on this. Some comments.
>>
>> Have you looked into the runtime overhead of this approach, assuming
>> -module-assertions is not specified? Currently lisp_to_value and
>> value_to_lisp
>> typically  are optimized away (i.e., zero machine instructions), and that
>> wouldn't be true under the proposed patch even when -module-assertions is
>> not
>> used. Granted, module calls need not be super-fast, but it might be nice
>> to
>> avoid the overhead in the typical case if that is easy.
>>
>
> I've run a simple benchmark (5000000 iterations of mod-test-sum). Result
> for the base case (module assertions not compiled) is 4.602 seconds, with
> the new code it's 4.605 seconds. That's probably good enough.
>
>
>>
>> What happens if -module-assertions is enabled for Emacs, Emacs dumps
>> itself, and
>> -module-assertions is not enabled when the dumped Emacs is run?
>
>
> Good question. I've now just made -module-assertions during dumping an
> error.
>
>
>> Or vice versa.
>>
>
> That's the normal case, which should work just fine.
>
>
>>
>> Are the FOR_EACH_TAIL_SAFE macro calls needed? This macro is needed only
>> for
>> lists that might be circular (e.g., the lists are available to Lisp code,
>> which
>> can call setcdr on their components). If the calls are not needed I
>> suggest just
>> an ordinary for loop.
>>
>
> Done.
>
>
>>
>> A small point: the FOR_EACH... loops should be indented like ordinary
>> for-loops.
>>
>
> Done. (Would be great if CC-Mode knew about this.)
>

I've now pushed this commit and also a commit that uses
__attribute__((nonnull)) to master.

[-- Attachment #2: Type: text/html, Size: 3488 bytes --]