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

[Ondrej Pokorny]

On 12.05.2019 17:45, Jonas Maebe wrote:
> On 12/05/2019 17:14, Ondrej Pokorny wrote:
>> On 12.05.2019 16:36, Jonas Maebe wrote:
>>> Thanks. I have added these warnings to the compiler in r42047, and 
>>> also the static/dynamic errors for Standard resp. Extended ISO Pascal.
>>
>> Very nice.
>>
>> One question about "C-style enumeration types" 
>> https://www.freepascal.org/docs-html/ref/refse12.html#QQ2-26-31
>>
>> Do the holes between undefined enum values belong to the enumeration 
>> or not? Because:
>
> It's because Pred and Succ do not make sense with enumerations with 
> holes in our opinion, and could be confusing. E.g. in your example, 
> one could expect at first sight that succ(one) = two.

It depends if the holes are valid enumeration values or not. If they 
are, Pred and Succ would make perfect sense - Delphi documentation 
explicitly states that Pred/Succ can and should be used to navigate to 
the holes:

"type Size = (Small = 5, Medium = 10, Large = Small + Medium);
Only three of these values have names, but the others are accessible 
through typecasts and through routines such as Pred, Succ, Inc, and 
Dec." - see 
http://docwiki.embarcadero.com/RADStudio/Rio/en/Simple_Types_(Delphi)#Enumerated_Types

But I am completely fine about your decision to disable Pred/Succ for 
C-style enumeration types (you did so only for non-delphi modes).

Yet, it still does not answer my question if the holes belong to the 
valid enumeration values from the FPC point-of-view or not.


> I simply forgot to take them into account. I'm tempted to disable the 
> warning altogether for them, because
> 1) Semantically, it would make sense to give a warning only if not all 
> explicitly defined enum values are covered
> 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
>
> That would make the warnings inconsistent.

Again, it all depends if the holes are valid enumeration values or not.

Your (1) suggests that they are not, whereas your (2) suggests they are.

Btw. subrange types have the same problem with implicit values - the 
same with (2):

program TestRanges;
{$mode objfpc}
type
   TMyRange = 2..5;
var
   R: TMyRange;
begin
   R := Default(TMyRange); // assign implicit value
   case R of
     2..5: Exit;
   else // compiler warning: unreachable code
     Writeln('1: ', R);// unreachable code is still executed despite the 
warning!
   end;
   Writeln('2: ', R);
   ReadLn;
end.

Furthermore I strongly disagree with your (2) statement. On the 
contrary, if there are all explicitly defined cases covered and there is 
an else statement (like in the TestRanges program above), a warning 
about the unnecessary case statement is very needed because it is an 
"implementation detail" of the compiler if the else block will ever be 
executed or not (AFAIR you supported this). Someone in the future can 
decide that if the compiler finds an unnecessary else block, it will 
just ignore it. Don't call the warning "unreachable code" if you don't 
like it - call it whatever you want but keep it there.

Ondrej