Re: master a99ba59aa02: ; * src/pdumper.c (dump_do_fixup): Pacify GCC.

[Pip Cet]

"Eli Zaretskii" <eliz@gnu.org> writes:

>> Date: Thu, 30 Jan 2025 10:51:34 +0000
>> From: Pip Cet <pipcet@protonmail.com>
>> Cc: emacs-devel@gnu.org, stefankangas@gmail.com, eggert@cs.ucla.edu
>>
>> "Eli Zaretskii" <eliz@gnu.org> writes:
>>
>> > compiler which evidently cannot figure out that dump_value is not
>> > initialized only in the cases where dump_write is false, which means
>> > dump_write is not called, and the value of dump_value is not used.
>>
>> THat's incorrect.  If type is 0x5a, for example, do_write will be true,
>> dump_value will be uninitialized, and dump_write will be called with a
>> pointer to the uninitialized dump_value.
>
> 0x5a is an impossible value.

If you mean because the compiler might remember that long ago, when the
cons cell containing the fixnum was created, the number that was encoded
corresponded to an enumeration value, I think you're expecting GCC to
remember too much about a data structure which has since been garbage
collected and mangled in many other ways by C code (not Lisp, at least).

Buf if we can agree the compiler cannot remember that XFIXNUM (...) is
already a value in the enumeration, and that it's just an integer which
the compiler knows very little about, casting it to any (non-bitfield)
enumeration type may yield 0x5a, whether or not that is a named value in
that enumeration type at compile time.

As the integer is safe to represent in any possible type the enumeration
is compatible with (the signed and unsigned integer types, plus char if
it's not in that list), the compiler assumes this case to be possible and
generates code and warnings for it.

This is perfectly valid C code:

enum ABC { A, B, C, };

void f (enum ABC v)
{
  volatile enum ABC v1 = v;

  switch (v1)
    {
    case A:
    case B:
    case C:
      return;
    }

  while (1);
}

int main (void)
{
  f ((enum ABC)'a');
}

and GCC must infloop in this case, because the C standard guarantees
that values in the intersection of all signed and unsigned integer types
and char can always be represented in any enum type, and ASCII 'a' is
such a value.

That's not how enums works in other languages, but it is how they work
in C, which requires you to explicitly add assumptions for the compiler
that only the enumerated cases are possible.  This used to be common:

enum ABCflags {
  A = 1,
  B = 2,
  C = 4,
};

enum ABCflags x = A | C;

such code remains valid and GCC doesn't even generate a warning about
it.

The behavior we usually want is achieved by:

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

This means:

1. Warnings if -Wswitch-enum is specified
2. No code is generated for out-of-enum cases
3. The fall-through case will be avoided, as will warnings such as the
one fixed by this UNINIT.
4. In debug builds, out-of-enum cases will abort

In some cases, we can omit the cast (if x has the right type already);
in some cases, we can move the eassume (0) after the switch statement
because all cases exit nonlocally.  It's always possible to write this
tediously as

bool matched = false;
switch (x)
{
  case A:
    matched = true;
    value = 'a';
    break;
  case B:
    matched = true;
    value = 'b';
    break;
}
eassume (matched);

But if those three defensive coding patterns aren't options, the choices
are:

1. -Wswitch-enum, extra warnings we don't want
2. -Wswitch, no warnings if there's a default: label
3. making the fallthrough case incorrectly reachable, generating
additional warnings
4. hack GCC to generate fallthrough code but mark it as "probably not
reached so I won't warn about it".

The C situation is regrettable but not something that's about to be
fixed.

Pip