On 08/18/2010 05:49 PM, David Simcha wrote:
While I was debugging std.range and std.algorithm I noticed that there's
tons of inconsistency about when enforce() vs. assert() is used for
checking input to range primitives. I think this needs to be made
consistent, and there are good arguments for both.
As mentioned, I've studied this exchange, thought a lot about it, and
also discussed it with Walter.
Clearly right now Phobos is inconsistent. Walter and other used asserts
and contracts and other means etc. My approach since I started has been
to sprinkle enforce() everywhere and worry about it later, and that
later has come.
One problem with that approach is that, as Lars mentioned, there are 3-4
checks on a simple operation due to the use of higher-order ranges that
wrap other ranges, like Stride!Chain!Whatever. This issue is not
difficult to tackle: we can introduce in Phobos the notion that a range
wrapping another range defers the checking to the wrapped range. That
way the wrapper only uses assert() and essentially trusts that the other
range protects its own integrity. Consider as an example Take's
implementation of popFront:
void popFront()
{
enforce(_maxAvailable > 0,
"Attempting to popFront() past the end of a "
~ Take.stringof);
original.popFront();
--_maxAvailable;
}
The enforce is redundant - we could decide that Take defers checking to
original. So this code should be allowed:
void popFront()
{
assert(_maxAvailable > 0, ...);
original.popFront();
--_maxAvailable;
}
But (very careful) Take should still protect its _own_ integrity,
assuming original also does. So the following code would be incorrect:
void popFront()
{
assert(_maxAvailable > 0, ...);
--_maxAvailable;
original.popFront();
}
If original.popFront throws, _maxAvailable has already been decremented
so Take is in an inconsistent state. So rule #1 could be something like:
Rule #1: A higher-order entity in Phobos can generally assume that the
lower-order entities it builds on protect their own integrity, and
protect its own under that premise.
I am sure there would be fuzzy corners to this rule, but by and large it
shold cut down on a lot of enforce calls. (We could and should replace
them with asserts).
The second rule concerns safety. To my dismay, this program still
compiles and runs "successfully" with -release:
import std.stdio;
void main(string[] args)
{
writeln(args[10]);
}
It should throw, and it should only run with -noboundscheck enabled.
That flag was defined by Walter under my pressure, and I think it's
misnamed - it should really be -safe.
Safety is one matter I am extremely rigid about. There is a point where
we must draw a line in the sand regarding checks, and safety is it.
Since there is a simple and clear definition of it, we can't redefine it
as we'd prefer. So I'd say:
Rule #2: All checks protecting memory safety must be done with enforce()
or rely on built-in array indexing (which in turn obeys -noboundscheck).
Finally, we have several places where we call APIs such as the I/O code
etc. For those I think it's fine to mandate use of enforce() for several
reasons, a simple one being that such calls would not be slowed down
considerably by the extra test. So:
Rule #3: All external API calls must be protected by enforce().
Of course there are many case-by-case judgments, but I think having such
guidelines in mind should be helpful. Thoughts?
Andrei
_______________________________________________
phobos mailing list
[email protected]
http://lists.puremagic.com/mailman/listinfo/phobos
