[fpc-devel] "Case statement does not handle all possible cases" Warning

[J. Gareth Moreton]

Yes, I'm jumpy, especially as you always like to open your responses 
with a direct insult of some kind - yes, I checked.  You want to kill me 
or ban me from the mailing list, go right ahead. I'm sure Free Pascal 
will survive without me.

We will agree to disagree on that one.  I see a warning as an actual 
warning that the code is very likely to cause unpredictable results or 
is an obvious oversight, like an uninitialised variable or a conditional 
block that is impossible to branch into (in this case, an 'unreachable 
else' for the case block would be a warning).  A hint is an optimisation 
tip that might also indicate laziness, like a variable declaration 
that's never used because you removed some old code or neglected to use 
it, while a note is a bit more serious than a hint but not as severe as 
a warning.

I'm not the only one who wasn't aware of the ISO standard - evidently 
Borland never followed through with it and most other Pascal 
programmers.  Until recently, even the compiler was full of omitted 
blank else blocks.  I would have thought that the lack of an else 
section on a case block is fairly indictive of one not being necessary, 
and any special values that need to be handled should contain their own 
branches.

Now don't get me wrong, I do agree that a notification is useful - as 
Jonas said, at least 1/3 of the messages pointed to case blocks that 
benefitted from having internal errors added, since an explicit branch 
should always be taken - an example of a manual one that Pierre and I 
implemented is the one that appears in TEntryFile.GetAInt, because 
SizeOf(AInt) should always return 1, 2, 4 or 8.  But a lot of the time, 
one may only want to check a select number of values for special 
operations, but which are numerous to the point that individal if 
statements are inefficient - for example, 
TCpuAsmOptimizer.PeepHoleOptPass1Cpu under x86_64 - having an empty else 
block, while certainly safer, comes off as superfluous since, in that 
case, it doesn't give any extra information.  Granted, if the standard 
of an obligatory 'else' was adhered to from the very beginning, we 
wouldn't be in this situation, but like the problem of 'inline' 
appearing only in the implementation section of subroutines, there is so 
much code out there now that it will be an endless battle to change it 
all to adhere to the standard and not break something.

I just feel that in this case, it should be a note, not a warning, 
unless you're compiling under a dialect of Pascal (e.g. Extended) that 
is stricter in its interpretation of the whitesheet and demands an error 
be returned.

Gareth aka. Kit

P.S. And while I agree that one should not merely look towards C and C++ 
for inspiration (I see the practice of lowercasing everything and not 
having a space between operators and operands as 'colonisation', for 
lack of a better term), it seems that we are a little inconsistent when 
it comes to that, given that Free Pascal supports the C-style assignment 
operators now.


On 19/05/2019 02:42, Ralf Quint wrote:
> On 5/18/2019 3:42 PM, J. Gareth Moreton wrote:
>>
>> Lazy... you've got a bloody cheek.  Well I guess there's no point in 
>> me contributing anything more if that's how you honestly view me.  I 
>> wasn't aware of the ISO that dictated that a case block shouldn't 
>> just fall through until today, and C/C++ and Basic never threw 
>> similar errors or warnings, so I didn't know this was a thing.
>>
> Wow, a bit jumpy, aren't we?
>
> As we are talking here about a Pascal compiler, it doesn't matter what 
> C/C++ or BASIC do. And even in other programming languages, properly 
> acting upon unexpected conditions is good programming practice. And 
> commonly people programming in Pascal are ones that try to adhere to 
> good programming practices, not just ignore them out of whatever 
> convenience they might perceive.
>
> A warning from the compiler is hence an appropriate response, telling 
> the programmer to check the source and make a decision, even if it is 
> adding an empty else clause to the offending case statement. 
> Everything else is just "lazy programming", like it or not...
>
> Ralf
>
>
>
> ---
> This email has been checked for viruses by Avast antivirus software.
> https://www.avast.com/antivirus
>
> _______________________________________________
> fpc-devel maillist  -  fpc-devel at lists.freepascal.org
> http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-devel
>
>