Yesterday I posted about an issue with type checking in Delphi (and other Pascal) compilers. As mentioned in that post, range checking is fundamentally flawed as a supposed solution to the problem for reasons that are explored further in this post.
To recap: Range checking does not test the types involved in both sides of an operation or assignment, it is is concerned only with the value involved on the one hand and just one of the types on the other. That is, the type of the variable or parameter that is to receive the value involved.
We shall use a simple scratch program to explore the ramifications of this:
program Project1;
{$APPTYPE CONSOLE}
uses
SysUtils;
procedure Foo(bar: Integer);
begin
WriteLn('Foo >> ', IntToStr(bar));
end;
var
i64: Int64;
begin
WriteLn('Range checking ', {$ifopt R+}'ON'{$else}'OFF'{$endif});
i64 := -1;
Foo(i64);
WriteLn('i64 >> ', IntToStr(i64));
ReadLn;
end.
Note: The final ReadLn() is simply to ensure that when we run this from the IDE the console window hangs around for us to check the output.
If you compile and run this (without range checking) and you get this result:
It compiles and runs without error, but what is this ? The output actually appears consistent, despite the fact that we passed a 64-bit representation of -1 into a 32-bit parameter. So let’s try the same thing with range checking enabled (don’t forget to do a full build after changing the project options):
Some might expect this result. After all, -1 is within the valid range for a 32-bit Integer just as much as it is for a 64-bit one. But if we look at the internal representation of these values things are less comfortable:
-1 32-bit $ffffffff
-1 64-bit $ffffffffffffffff
So yes, -1 is in the valid range of both types, but what has actually happened is that the range checking has tested the value of the 64-bit integer and, finding it acceptable, has gone right ahead and passed the truncated 32-bit value through to the function.
On the one hand you might be thinking “So what ?“. After all, -1 is -1 right ?
Who cares if it’s a 64-bit representation versus a 32-bit representation. The value is what counts, surely ? Especially when you realise that this thinking happens to work all the way up (down?) to the maximum (minimum?) negative value of a 32-bit integer (and positive values, obviously).
This if course is exactly the logic behind range checking and is perfectly valid for those values that “pass” this test. It doesn’t care that the possible values involved could be out of bounds, it is only concerned with whether the specific value involved in a particular execution of some code actually is within those bounds.
Which does nothing to identify that this exact same code is certain to fail when values are involved that do not pass the test.
What this means of course is that if you rely on range checking to detect type errors of this nature then you have to leave that range checking in your production code, incurring the not insignificant overhead that involves.
That is, range checking on every assignment of an ordinal value, every passing of an ordinal parameter.
Every.
Single.
One.
Quite apart from the penalty being carried around by all the perfectly valid code just on the off chance that there is some bad code to be identified, this is basically the same as relying on FullDebugMode and FastMM to find places where you have left dangling pointers in your code. Perfectly valid in a debug build, but not something you would want to include in your shipping code.
But what’s the alternative, in the absence of help from the compiler ?
How about good old fashioned self-discipline, diligence and effective code reviews to catch the occasional, inevitable slip-up? Maybe ? Just a thought.
i.e. don’t just say “turn on range checking and wait for the exceptions to find the bugs“, but do yourself what the compiler could and should be doing.
Just because the compiler has dropped the ball is no excuse for you to follow suit.
Footnote: Some Curious Inconsistencies
Warning Says “No“. Generated Code Says “No Problem“
Let’s change the initialisation of i64 to use the internal representation of -1 that we identified:
i64 := $ffffffffffffffff;
When you compile this you do in fact get a warning:
Constant expression violates subrange bounds
This is because the literal constant $ffffffffffffffff is treated as unsigned, where-as the variable is signed. However, if you compile and run this with range checking enabled you get exactly the same result as before. I won’t bother with another screenshot. I know seeing is believing but if you don’t believe me, just try it yourself. 🙂
This is odd. Either that value is signed and therefore out of range of the variable type involved (hence the warning) or it isn’t and we shouldn’t have received the warning. It seems the team responsible for the compiler warnings and the team responsible for the range checking code team weren’t singing off the same hymn sheet here, which is rather worrying.
32-bit Integers Better Behaved Than Their Bigger Cousins
Now try the same thing with a 32-bit Integer:
var
i32: Integer;
begin
WriteLn('Range checking ', {$ifopt R+}'ON'{$else}'OFF'{$endif});
i32 := $ffffffff;
Foo(i32);
WriteLn('i32 >> ', IntToStr(i32));
ReadLn;
end.
This version of the code (the foo() function is exactly as before) produces the same “Constant expression” warning, and when compiled with range checking OFF, produces two lines of “-1”, as before.
The difference in this case however is that when we compile with range checking ON then that warning proves to be spot on. We get a range check error when the code attempts to assign the value to i32.
So we have inconsistency between handling of 32-bit and 64-bit variables and inconsistency in the case of 64-bit values in terms of the warning output and the actual code ultimately produced.
Whichever way you look at it, range checking is fundamentally flawed as a mechanism for identifying fundamental, static errors in code, and has some very nasty smells about it in the Delphi implementation in particular.
Next time someone suggests you lean on this to identify errors in your code you might ask them where they have tied up their horse and what arrangements they have made for collecting the dung. 😉
This is actually a deeper dive into integer assignment compatibility. Range checking works well. It’s designed to be a runtime check. What we are missing are the compiler warnings that other tools provide. For instance -Wconversion in GCC. But range checking itself does its job well, but it ain’t going to help with static checks.
The real problem is crappy design of Integer assignment and type conversion.
Obviously range checking and assignment code gen are separate mechanisms, but to say that range checking “works well” rather misses the point that range checking is entirely unnecessary if you have rigorous type checking (in the compiler, or on the part of the developer themselves). Which is to say that no scenario springs to mind where range checking has a purpose and a point beyond (attempting) to compensate for this omission.
Range checking is, in my mind, primarily concerned with array indexing. It is very valuable there.
Static checking for integer assignment would add value. But you’d still benefit from runtime checks for when you were casting to bypass these putative static checks.
Fair point on the application of range checking to array indexing. To my mind this should be a special case. i.e. I should be able to have the compiler check my array indices separately from checking simple assignments.
But although it is a valid case, I wouldn’t that it is “very valuable”, unless you employ practices which make it valuable.
Arrays either have fixed bounds in which case an appropriate sub-range type should be used for the dimension(s) which can (or should) be enforced by the compiler. Or they have dynamic bounds and due diligence / code reviews should identify where code accessing the array could result in those bounds being exceeded.
In either case problems are quite easily avoided without having to rely on runtime checks to clean up the mess resulting from lazy coding. imho. ymmv.
I guess I’m not as good as you and sometimes make mistakes. It must be nice never to make mistakes. But for the rest of us, having the debug version of the program throw a runtime error when I make a mistake is helpful. I envy your perfection.
Who ever said I never make mistakes ?
Let me tell you a little story about an hour spent chasing an apparent issue in a GDI clipping implementation at the weekend. The problem eventually proved to be nothing to do with errors in my clipping, but in my passing a TColor where a HPEN was required…. DOH! I had an overloaded approach for brushes that distinguished between HBRUSH and TColor (creating a temporary brush of the required color). I hadn’t yet provided the same overload for pens in my [evolving] framework (but thought that I had).
Range checking wouldn’t have helped in that situation of course, since both HPEN and TColor are Integer types. Proper type checking on the other hand….
But when it comes to array indexes I have developed practices which do tend to ensure that I do not make basic, simple mistakes (any more) in such code. Practices that are easily adopted and are far more reliable than leaning on range checking.
In any quality system you should always aim for zero-defects and ensure practices that work towards that goal. Range Checking is fatally flawed as a tool for identifying coding errors. It identifies only execution errors resulting from coding errors (in only a subset of the potential execution cases).
A quality minded approach, diligence and discipline are far more effective tools.
Even so I repeatedly make the point that range checking may have some application/utility as aDEBUG / QA tool. The mistake is to rely on it in production code. by the time your code reaches production, you should not need range checking. An approach that relies on requiring this simply indicates a frankly lazy approach and inadequate code quality processes.
That is all.
Who says you can only use one tool? Why not use multiple tools and methods? You seem to imply that if you enable range checking whilst debugging, that you would somehow not review your code. Or that having reviewed your code you would not test it.
Enabling range checking whilst debugging is something that you do in addition to all the other good practices. It cannot harm you to do so.
For beginners mind you, it is a great help. Astounding numbers of SO questions could be avoided by enabling range checking.
Also, it’s playing with fire to overload HBRUSH and TColor. That sounds fairly insane to me. I can’t get past the fact that overload resolution rules are not documented. The integer assignment compatibility rules of the language make attempting such overloads an act of folly. Don’t do it!!
Why is it folly to take advantage of an aspect of the language which works just because some other aspect does not ? Even though HBRUSH and TColor are assignment compatible, the compiler is never-the-less perfectly able to distinguish between overloads with formal parameters of these types (even in older versions where the compiler could/would not differentiate between TDateTime and Double, among other things, for example, something which changed in Delphi 2009+).
You appear very selective in your approval of techniques.
You just found out why this is folly. You exemplified it perfectly.
That makes no sense at all. Even if I had not ever intended to create an overload of HPEN and TColor variants of this method and only ever provided a HPEN version, there would still have been nothing to stop me ignorantly trying to pass a TColor where that HPEN was required. The overload solves the problem, it does not create it.
Range checking on the other hand does not solve such problems at all. Yet you advocate the latter but consider the former a folly ?
I suspect this has more to do with opposing the messenger and nothing to do with the message.
Fair enough. I agree that overloads don’t make things worse here. I wasn’t thinking that through.
My main problem with overloads on integer types is that the resolution rules are not documented. And apparently changed without fanfare in recent times. And assignment compatibility is so lax, silent conversions can happen. Exactly the issue you are talking about. I personally think that building overloads on top of that shaky foundation is a bad idea.
The only person proposing range checking as a cure all is you. It’s a runtime debugging tool that flushes out some bugs of certain types. That’s all. The fact that it won’t get them all doesn’t mean that it is completely useless. If you enable it whilst debugging, and whilst running test suites then you may catch some defects. Where is the downside?
As for the messenger line, that’s silly. Don’t make this personal. I explained my gripe against integer overload resolution badly because I didn’t think clearly. Nothing more sinister. I agree with lots of what you say, and disagree with some more. What’s wrong with that? Arguing about it is how we all learn to challenge and change our opinions.
What on earth are you smoking?
I have repeatedly stated that I consider range checking utterly inadequate to the task and recommend not using it, and certainly not relying on it, as a means of identifying these sorts of problems. At the same time I have repeatedly said that it can be of (limited) use when debugging but absolutely should not be necessary in production code.
On the other hand there was a David Heffernan who wrote:
Even as a way of finding errors during testing (a distinction that is not made in this recommendation) this is dangerously unqualified and contains hidden, circular thinking. If you cannot see the problem, consider that the accuracy of the advice would be improved greatly by the use of “if” rather than “when“.
As for not making it personal … “It must be nice never to make mistakes … I envy your perfection” … these snide remarks were not directed at me personally ?
Ok, if you say so.
So, to elaborate, what I am smoking is this. You said:
“As mentioned in that post, range checking is fundamentally flawed as a supposed solution to the problem for reasons that are explored further in this post.”
Agreed. The problem is that the compiler won’t tell you when it performs integer type conversions. Decent compilers for other languages, languages that are frequently lambasted by Delphi fans FWIW, will inform you of such conversions.
The thing is though, I don’t think anyone is suggesting that range checking solves this issue. It seems to me that you are the only person that is considering that possibility, and you rightly reject it. But you are attacking the wrong thing. Your argument is akin to criticising hammers because they are not very good at putting screws into a wall.
Range checking won’t perform static analysis. It won’t find all defects of this nature. What it can do is be a debugging aid that can find silly mistakes more quickly and allow the developer to catch those mistakes sooner. There’s no real downside to its use. The only downside I could imagine would be if the user imagined that range checking would find all bounds errors.
But the fact that somebody might misuse a tool does not render that tool useless. I don’t care if A. N. Other uses the tool incorrectly. I know how to use it, and it has value to me. I know what it does, and what it doesn’t do.
The mistake you make is that you think range checking is something that it is not. Inadequate to the task? What task? It’s only you that is expecting more from range checking than it can offer. I’m fully aware of what it can and cannot do. Are you only now realising that it’s a runtime feature?
Again I say, that range checking doesn’t find all defects does not render it useless. Tell me the downside of runtime range checking in debug builds?
My snide and comments regarding perfection were in response to your previous comment. In that comment you appeared to suggest that your coding practices and code review meant that you never made bounds errors and that people using range checking were lazy. Very far from the case. My sarcasm was meant to highlight that. Clearly none of us are perfect.
To keep this focused, remind me again how enabling range checking results in worse code?
My point in all of this is that you are attacking the wrong thing. The problem is not runtime self verification. The problem is silent type conversion.