[fpc-devel] [Suggestion] Enumeration range-check intrinsic

[J. Gareth Moreton]

Mucking around with pointers will always produce unpredictable code if 
you don't take care in what you're doing (and in your example, it uses 
tools that already exist), but Jonas, I have to protest your logic.  
Saying it's safer to refuse to do it at all instead of doing something 
that works part of the time is not always a sound design philosophy.  
Enums taking on invalid values is something that can and does happen, 
but by adamantly refusing to allow the programmer to check its validity 
and demanding unpredictable program behaviour is not something I or 
others can accept.  Or saying that all bets are off if a file is corrupt 
or has been tampered with... some programs, by design, are simply not 
allowed to crash.  If a sensor on an aircraft returned an invalid value 
(say for the sake of argument it was an undercarriage feedback that 
returned "Locked", "Stowed" and "Off" (stowed but without hydraulic 
pressure)) and the flight computer crashed because the developers were 
not allowed to create defined behaviour for invalid data, would that be 
acceptable for you?

There have been so many bug reports from all sorts of developers who 
have complained abut this behaviour and telling them that "it's not a 
bug" and not even offering them a workaround from undefined behaviour is 
both arrogant and ignorant.

When entirely self-contained within a program, the type system is fine, 
and there's no need to do range checks on enumerations, but when you 
have an external interface, such as a data file or a third-party library 
(either written in FPC or another language), there needs to be a way to 
check if the enum is within range or not, and using the "is" operator in 
this way is a perfectly reasonable way of doing things.  Because it's 
just an integer value at the lowest level, it will return a sane answer 
unless you go out of your way to try to break it.

As for the thing about keeping Pascal pure and consistent... it never 
was.  You have code blocks wrapped in "begin...end" and then you have 
the oddball that's "repeat...until", which is a carry-over from some 
dialects of BASIC.  And then you have things like the "class procedure 
Etc; static;" directive when a slightly better design might have been 
"static procedure Etc;", but it's too late to change that now.  
Sometimes you have to make design choices based on realistic needs 
rather than an ideal.

It can be enough to put in the documentation for 'case' something like 
the following, since it is these that cause crashes: "*WARNING:* Case 
blocks do not check to see if an enumerated value falls within the valid 
range, and passing an invalid value can trigger an access violation.  To 
avoid this behaviour when the validity of the input is uncertain, 
perform a separate check with "(Value is TEnum)" - a result of False 
indicates an invalid value." - that is all that would be needed.  If 
you're not happy with using "is" for this purpose, then I would like to 
offer my original option of an intrinsic that takes on the form of a 
Boolean function: "if IsValidEnum(Valid) then ...".  Note I do not 
consider trapping EAccessViolation to be an acceptable solution because 
it is very inefficient with performance and is too broad an exception 
that usually indicates a serious problem (e.g. insufficient system memory).

Ultimately, being forced to ditch enumerations for classic C-style 
constants defined against the Integer type (the only real workaround) 
only makes the code more difficult to understand, and a more junior 
developer may think to replace them with enumerations under the 
philosophy of improving readability, not realising that it will cause 
the compiler to shortcut the safety checks.  If I were a senior project 
developer, I simply could not take such a risk and would discard FPC 
completely for a tool what will not cause this problem (or has a 
documented workaround).

You could consider this an ultimatum, but I know it will be pointless to 
do that because you'll just ignore me.  So I'm pleading, Jonas... there 
needs to be a way to check if an enumeration contains an invalid value 
to avoid diving into the realm of undefined behaviour.  Just because you 
wouldn't use it doesn't mean we won't.

Gareth



---
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freepascal.org/pipermail/fpc-devel/attachments/20190705/1298d387/attachment.html>