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

[Ralf Quint]

On 5/18/2019 8:16 PM, J. Gareth Moreton wrote:
> Yes, I'm jumpy, especially as you always like to open your responses 
> with a direct insult of some kind - yes, I checked. 
Well, I just checked my send folder and this was the first response to 
you in at least the last two years, if ever...
> You want to kill me or ban me from the mailing list, go right ahead. 
> I'm sure Free Pascal will survive without me.
Kill you? You must be smoking some real bad shit. Banning you from the 
mailing list? Wouldn't be my call in the first place. But certain is 
that you seem to be taking yourself far to serious.
>
> 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.
Yes, an unhandled case value can result in unpredictable results, and 
that is exactly the reason why there should be a warning from the 
compiler, not just a "note" or "hint". Just take a language interpreter 
reading tokens or some data file with data record type identifiers, like 
spreadsheet cells. If you read a value that isn't defined in your case 
list, you will and up with unpredictable results.
>
> 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. 
For the most part, I don't care about the ISO standard, but what I care 
about is responsible programming...
> 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.
Nobody said that there can't be case statements without an else clause. 
However...
>
> 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.
You are contradicting yourself here. If the compiler can determine that 
there is indeed an unhandled case value, it isn't likely to be 
superfluous. Yes, there are cases where no else clause is needed in case 
statement, but then this also should not trigger a compiler warning (or 
note, hint). If you however get that warning, it is up to the programmer 
to verify if those situation are possibly harmless, and then it is not 
an issue to eliminate the warning by adding an empty else clause, at 
least that indicates in the source that this possibility has been 
considered. And in cases where unhandled case can not be verified to be 
harmless, it is prudent to add a more meaningful handling within the 
else clause.
>
> 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.
I don't think that classifying such a message "just as a note" is 
correct, regardless of the the dialect of Pascal, it is potentially 
dangerous in any case and hence should be a warning.
>
> 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. 

Well, the later is certainly not something I petitioned for, despite 
programming in C almost as long as in Pascal. But what IMHO sets Pascal 
apart is that it should advocate good programming practices and a Pascal 
compiler should assist the programmer in writing as clean and error free 
code as possible. And as such, if the compiler is able to detect a 
possibly dangerous situation, it should appropriately "warn" the 
programmer. And then it is still up to the programmer to act upon that 
warning, or just being lazy and ignore that warning. And re-labeling 
such a warning as a note or hint is just another form of being lazy. In 
either case, this doesn't stop the compilation of the code and if either 
note or warning are ignored, ...

Ralf



---
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus