[fpc-devel] [Patch/RFC] Warnings for (in/over)complete case statements

[Martok]

Hi,

don't get me wrong, I like having something of mine included, but I'm genuinely
surprised you used it in the way I designed it.

As you said, the warning and code that is actually generated must agree,
otherwise they're useless. And there is a huge gap in the definition of the
Freepascal interpretation of what "all values of the type" even means, with the
code-as-written sometimes using one interpretation and sometimes the other.

On the topic of enums with assigned ordinals:

> I simply forgot to take them into account. I'm tempted to disable the 
> warning altogether for them, because
Please don't. Having sanity checks for "easy" types (where you don't really need
them) and not having them for "complicated" types (where they would likely catch
programmer errors) is not super helpful, and I would consider this as *worse*
than the status quo of not having warnings at all, because it would give a false
sense of safety ("My code compiles with -Sew? I must have done everything right!").
I'll never understand why Borland took that short cut for enumeration typeinfo
and got away with it.

> 1) Semantically, it would make sense to give a warning only if not all 
> explicitly defined enum values are covered
Agreed.

> 2) However, you still can't give an "unreachable code" warning if 
> there's an "else" in that case, because implicit values can also be 
> stored in the enum
Which takes us aaaaaall the way back to the original problem.

In my (and Borlands, and Microsofts) opinion, this program should compile
without raising the two new warnings (all named values are used AND else for
unnamed values 3 and 4), rangecheck as correct (3 in [2..5]), and be fully
defined at runtime. DFA should show that the else branch is always taken, which
may then mark the case labels as dead code.

program Project1;
{$mode objfpc}
type
  TMyEnum = (one = 2, two = 5);
var
  A: TMyEnum;
begin
  A := TMyEnum(3);
  case A of
    one: Exit;
    two: Exit;
  else
    Writeln('Other');
  end;
end.

With R := Default(TMyRange), which assigns 0 [side note: why is the default
value an invalid value? That makes no sense at all], I would expect the same
results except for a range check warning at compile time (Range check error
while evaluating constants (0 must be between 2 and 5)).

I would *wish for* (and Borland documented this) the code to still be
runtime-safe even in $R- in this case, but I'm well aware that you and Florian
have already declined that. For completeness: subrange types silently upgrade to
their storage types if required, which to me implies also in comparisons, which
implies the "none of the above" label must always mean the entire storage type.


I'm still in favour of making an effort to formally define at least the core
elements of the Freepascal languages (syntax, statements, types, overloads,
resolution order, typeconv order, ...) with as few implementation quirks as
possible and then align the compiler to it (not the other way around). Having
less corner cases to juggle in one's mind may make things easier, and a formal
spec is also a good reference for future extensions to see if ideas are
orthogonal or not. But that's a whole different topic.


-- 
Regards,
Martok