[phobos] Usage of asserts/contracts and memory safety

[Jonathan M Davis]

On Thursday, August 25, 2011 06:30:56 Vladimir Panteleev wrote:
> Hi,
> 
> I noticed that Phobos's makefile specifies -release, however there are
> places in Phobos which use asserts and contracts. I recall there being a
> discussion regarding verification of parameters in Phobos, but I do not
> know the current consensus. DMD has -defaultlib and -debuglib switches,
> which could justify usage of asserts/contracts, but they are not used in
> the default configuration.
> 
> I'm currently working on making std.socket.SocketSet memory-safe (adding
> sockets past the fd_set capacity is not currently checked), and I'd like
> to know if these checks should be done with "enforce" in the function
> body, or "assert" in "in" contracts etc.?

If it's a checking for a bug in Phobos, use assert. If it's something that 
depends on the program's state rather than whether the code is correct (e.g. 
failed connection), use enforce. Other than that, it gets debatable. 
Certainly, do _not_ use enforce simply because asserts aren't compiled into 
the binary of Phobos which is released. If someone really wants assertions in 
Phobos, they can compile it in non-release mode.

Personally, I'm generally against using assert for testing arguments, since 
they often depend on program state, in which case enforce is really what you 
want. However, there _are_ cases where it's definitely a bug if an invalid 
argument is passed to a function. The problem with in contracts is that 
they're actually testing client code, not the function that they're in, so it 
really should be up to the caller to determine whether an assertion or 
exception should be used, but the language isn't set up to do that. There's 
certainly an argument for using assertions and letting client code choose to 
use enforce on its own if they want that behavior, but again, it's up for 
debate.

If anything, I'd say that it's a case-by-case thing at this point. Does it 
make the most sense for a particular function to assert on its arguments, in 
which case the tests may never happen (e.g. if the code is compiled with -
release), or does it make the most sense to throw an failure? It's very 
context-dependent.

std.datetime uses exceptions for bad arguments, whereas std.algorithm uses 
assert. But if a function in std.algorithm is given a bad argument, it's 
almost certainly a bug in the client code, whereas std.datetime's functions 
could easily be operating on values given from I/O.

So, I'd say that you should pick whether to use assert or enforce on function 
arguments based on how bad it is if the test is skipped (as happens with 
assertions and -release), how performant the code must be (truly performance-
critical code may not be able to afford the cost of testing in -release mode as 
would happen with enforce), and how likely it is that bad arguments are a 
result of a bug in the program rather than the state of a particular run of 
the program.

- Jonathan M Davis