Subject: bug#75451: scratch/igc: Enable CHECK_STRUCTS

From: Paul Eggert <[email-masked]>
Date: Sun, 19 Jan 2025 21:03:33 -0800

On 2025-01-19 18:01, Stefan Kangas wrote:
> In my attached patch,
> there is one removal of 'default: eassume(false)'; how can I know if
> it's worth keeping or not?

There are a few reasons to have such a line. First, to pacify a GCC 
false alarm. Second, to make the code clearer to a human reader. Third 
(less likely), to generate more efficient code. If no reason applies I 
would omit such lines.

>    int type = XFIXNUM (dump_pop (&lreloc));
>    reloc.emacs_offset = dump_off_from_lisp (dump_pop (&lreloc));
>    dump_check_emacs_off (reloc.emacs_offset);
> -  switch (type)
> +  switch ((enum emacs_reloc_type) type)

When it's easy, let's avoid casts like that as they're too powerful: you 
can cast pointers by mistake. Here it's easy to omit the cast, and 
declare the local variable 'type' to be of type 'enum emacs_reloc_type' 
rather than of type 'int'. (I assume you know for other reasons that the 
value is in range for the enum - otherwise the cast wouldn't be valid 
either.) In other words, just change the first line to:

    enum emacs_reloc_type type = XFIXNUM (dump_pop (&lreloc));

This way, the last line can remain unchanged and there is no need for a 
cast.