Thread overview
[phobos] Usage of asserts/contracts and memory safety
Aug 25, 2011
Vladimir Panteleev
Aug 25, 2011
Jonathan M Davis
Aug 25, 2011
Vladimir Panteleev
August 25, 2011
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.?

-- 
Best regards,
  Vladimir                            mailto:vladimir at thecybershadow.net
August 24, 2011
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
August 25, 2011
On Thu, 25 Aug 2011 06:49:09 +0300, Jonathan M Davis <jmdavisProg at gmx.com> wrote:

> 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.

Thanks for the detailed explanation. Based on the above, it's quite clear that my case should use "enforce".

https://github.com/CyberShadow/phobos/commit/8be7c18cf3b5553e0302d8766b0445d4d97e6665

-- 
Best regards,
  Vladimir                            mailto:vladimir at thecybershadow.net