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

From: Pip Cet <[email-masked]>
Date: Mon, 20 Jan 2025 07:45:02 +0000

"Stefan Kangas" <[email-masked]> writes:

> Paul Eggert <[email-masked]> writes:
>
>> Yes, inserting 'default: emacs_abort ();' switches from static to
>> dynamic checking, which is less reliable. Although 'default: emacs_abort
>> ();' is sometimes the best one can do, it's better to avoid it when
>> possible.
>>
>> I use 'default: eassume (false);' more often than 'default: emacs_abort
>> ();', though for a completely different purpose: to tell GCC something
>> that it's not smart enough to figure out on its own, so that GCC can do
>> better static checking elsewhere.

My understanding is the only correct exhaustive enum switch right now
looks like this:

int switch_statement(enum ABC x)
{
  switch
    {
      case A:
      case B:
        return 0;
      case C:
        return 1;
    }
  eassume (0);
}

Using "break" instead of "return" is impossible:

* "default: eassume (0);"  means you lose compiler warnings if the enum is
expanded.  -Wswitch-enum generates the compiler warnings, but they're
lost in a sea of false positives because sometimes we cannot switch over
an enum exhaustively.

* "default: emacs_abort ();" will generate code to handle the "impossible"
case

* Omitting the default statement will make GCC treat the fall-through case
as reachable

This is because this code:

#include <stdio.h>

enum three { A, B, C };

int main (void)
{
  volatile enum three four = C + 1;
  volatile const char *bad = 0;
  switch (four) {
  case A:
  case B:
  case C:
    printf ("%s\n", "good");
    break;
  default:
    printf ("%s\n", bad);
  }
  return 0;
}

is treated by GCC as valid code and the "bad" printf cannot be
determined to be unreachable.

(BTW: The correct behavior of this program, on GNU systems, is to print
"(null)".  GCC miscompiles it so it segfaults instead.  This is a
long-standing and severe compiler bug, and I doubt it'll ever be fixed.)

-Wmaybe-uninitialized also does not behave as documented in code like
the above: if I access an uninitialized variable in just one of the
branches, there's no warning.

> Thanks, that's helpful.  Based on the above, I came up with the attached
> patch for pdumper.c.

Let's introduce macros for the different cases, and use the same macro
in the same case, until GCC offers a fix.

> But could you explain your last paragraph above?  In my attached patch,
> there is one removal of 'default: eassume(false)'; how can I know if
> it's worth keeping or not?

This is an impossible decision.  None of the options GCC provides are
good.  The best we can do is to distinguish them by using different
macros and waiting for GCC to be fixed.

Pip