On 12/05/2019 18:30, Ondrej Pokorny wrote:
On 12.05.2019 17:45, Jonas Maebe wrote:
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.
Whether or not they are valid does not make it any less confusing.
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:
I know, but I think that is not obvious from the declaration.
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.
As far as range checking and undefined behaviour is concerned, they do.
I.e., you won't get undefined behaviour by assigning any value between
low(enum)..high(enum) to them.
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.
My (1) simply means that if you declare an enum with three explicit
values, those are probably the only ones you'll want to explicitly
handle in a case-statement. But yes, you'll probably still want an
else-statement to catch errors in case another value got in there
somehow, so I'll leave the warning as it is now.
Btw. subrange types have the same problem with implicit values - the
same with (2):
Subrange types don't have "implicit values". You have values that are
valid/in the range, and you have values that are invalid/out of range.
There is nothing in between, unlike with enumeration types with holes.
program TestRanges;
{$mode objfpc}
type
TMyRange = 2..5;
var
R: TMyRange;
begin
R := Default(TMyRange); // assign implicit value
The bug here is that this compiles even when range checking is enabled.
This stores an invalid value in R, hence anything that uses R after this
statement has undefined behaviour. The compiler should warn about that
without range checking, and given an error with range checking (like
fore other range checking errors). An alternative is for Default() to
return a different value than 0 in this case, but I don't know if that
would fix more than it breaks.
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).
The TestRanges program is not about an implementation detail, but about
undefined behaviour. Those are two completely different things.
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.
The compiler will always warn before removing unreachable code and will
call it as such.
Jonas
_______________________________________________
fpc-devel maillist - fpc-devel@lists.freepascal.org
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-devel
