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

[Ondrej Pokorny]

On 02.07.2019 23:34, Jonas Maebe wrote:
> On 02/07/2019 22:31, Ondrej Pokorny wrote:
>> var
>>    Value: TEnumType;
>> begin
>>    Value := TEnumType(-1);
>>    IsValid := Value is TEnumType; // IsValid gains false
>>
>> The compiler may not do any optimizations here (like return always true
>> if the left side value is the enum type at the right side). This should
>> be clearly stated and documented if the feature is added to FPC.
> Whenever you need an exception like this, it means that the behaviour
> does not fit in the language. As a result, it would make the language
> (more) inconsistent and harder to reason about, even independently of
> optimizations. E.g. if Value is inside a bitpacked array or record,
> IsValid will may well be true instead of false, because a bunch of bits
> were thrown away during the assignment. This shows that the only place
> where the checking can reliably happen, is during the initial conversion
> (which should have been done with "as" in this scenario).

This discussion is useless unless the compiler can assure that a check 
will happen for every possible (initial) conversion/assignment:

E.g. in

1.) Read(MyEnumValue);
2.) TStream.Read(MyEnumValue, SizeOf(MyEnumValue));

etc. etc.

Your philosophy effectively forbids the use of the above methods for 
enums/subranges, because once some value is inside MyEnumValue without a 
range check, you never ever can check if it is valid or not.

Even this does not help:

case Integer(MyEnumValue) of
   Integer(Low(TEnumType))..Integer(High(TEnumType)): Writeln('valid');
else
   Writeln('invalid');
end;

It cannot be used because casting MyEnumValue back to Integer 
("Integer(MyEnumValue)") is undefined for invalid MyEnumValue values. 
Thanks - we came to nothing.

Big wow: you say "the only place where the checking can reliably happen, 
is during the initial conversion". FPC currently doesn't offer any 
checking for conversions to enum. So every read from external data must 
currently consist of these 3 steps:
1.) read the value as an integer
2.) check manually that the integer value is between low and high
3.) assign the integer value to the enum value
This results in a lot of bloated code. And with the "undefined" 
IS-operator it doesn't get any better. Only the 2nd point gets a little 
bit shorter.


> And yes, it also complicates compilers/optimizers, because you have to
> make sure you handle this special case everywhere, now and forever. E.g.
> it would make it impossible to add range annotations for enum types in
> LLVM bitcode without adding volatile load hacks all over the place.

I have to admit that I do not understand what you mean with the text 
above. It's probably my lack of knowledge.


> As to your patch itself: why do you not directly compare the 
> tconstexprint values directly, and use the svalue/uvalue fields instead?

Because I missed that they can be directly compared directly.

Ondrej