www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - assert vs. enforce in Phobos code

reply Joseph Rushton Wakeling <joseph.wakeling webdrake.net> writes:
Hello all,

This is a subject I think we've visited before, but I can't find the old 
discussion threads :-(

As some of you know I'm working on a successor to std.random -- it's been put
on 
hold the last 8 weeks or so by country/life/work changes, but now I'm back onto 
it.  One of the things I'm examining in the course of that is the error
checking.

For example, a call to uniform(a, b) sees the function ensure that the interval 
is appropriate via an enforce(a <= b).  The result is that passing the wrong 
values to uniform() will bring your program down.  There are many other 
comparable cases in the module.

Part of me feels this is absolutely right and that such errors should be 
strictly objected to in this way.  On the other hand, this seems a wrong 
approach given that these kinds of functions are surely going to be called deep 
within the program -- we're not verifying user input here, it would be a
logical 
error in the program to pass wrong bounds to uniform() and so on.

My inclination is to clean this stuff up, to generally replace enforce's with 
asserts, to provide in- and out-contracts where appropriate, and so on.

Thoughts, remarks, ... ?

Thanks & best wishes,

      -- Joe
Feb 23 2014
parent reply "monarch_dodra" <monarchdodra gmail.com> writes:
On Sunday, 23 February 2014 at 09:55:29 UTC, Joseph Rushton 
Wakeling wrote:

 Hello all,

 This is a subject I think we've visited before, but I can't 
 find the old discussion threads :-(

 As some of you know I'm working on a successor to std.random -- 
 it's been put on hold the last 8 weeks or so by 
 country/life/work changes, but now I'm back onto it.  One of 
 the things I'm examining in the course of that is the error 
 checking.

 For example, a call to uniform(a, b) sees the function ensure 
 that the interval is appropriate via an enforce(a <= b).  The 
 result is that passing the wrong values to uniform() will bring 
 your program down.  There are many other comparable cases in 
 the module.

 Part of me feels this is absolutely right and that such errors 
 should be strictly objected to in this way.  On the other hand, 
 this seems a wrong approach given that these kinds of functions 
 are surely going to be called deep within the program -- we're 
 not verifying user input here, it would be a logical error in 
 the program to pass wrong bounds to uniform() and so on.

 My inclination is to clean this stuff up, to generally replace 
 enforce's with asserts, to provide in- and out-contracts where 
 appropriate, and so on.

 Thoughts, remarks, ... ?

 Thanks & best wishes,

      -- Joe


As a rule of thumb, "enforce" is not necessarily for things 
"user-input" but for things "outside the programmer's control" 
eg: "things that can legitimately fail", Notably, IO, threads, 
databases etc...

If you see any phobos code that validates the range of its inputs 
via an enforce, please knock yourself out and assert it/contract 
it.
Feb 23 2014
next sibling parent reply Joseph Rushton Wakeling <joseph.wakeling webdrake.net> writes:
On 23/02/2014 17:10, monarch_dodra wrote:

 As a rule of thumb, "enforce" is not necessarily for things "user-input" but
for
 things "outside the programmer's control" eg: "things that can legitimately
 fail", Notably, IO, threads, databases etc...


It seems to me that "enforce" is used extensively in Phobos for other purposes 
than that, and quite often seems to be used for obligatory runtime checks.


 If you see any phobos code that validates the range of its inputs via an
 enforce, please knock yourself out and assert it/contract it.


Well, these are some examples where it seems to me both understandable that 
someone wanted to really, really make sure the values were correct, and at the 
same time to fall outside the scope of "things that can legitimately fail":

https://github.com/D-Programming-Language/phobos/blob/master/std/random.d#L359-L360

https://github.com/D-Programming-Language/phobos/blob/master/std/random.d#L1253-L1255

https://github.com/D-Programming-Language/phobos/blob/master/std/random.d#L1334-L1335

https://github.com/D-Programming-Language/phobos/blob/master/std/random.d#L1345-L1347

https://github.com/D-Programming-Language/phobos/blob/master/std/random.d#L1357-L1359

https://github.com/D-Programming-Language/phobos/blob/master/std/random.d#L1629

https://github.com/D-Programming-Language/phobos/blob/master/std/random.d#L1703

https://github.com/D-Programming-Language/phobos/blob/master/std/random.d#L2061-L2070

I referred to checking user input because the only rationale I can see for
these 
statements being enforce is if the values being checked were being treated as 
user input and therefore something that could "legitimately fail".

Or have I missed something about when "enforce" is appropriate? ;-)

Assuming I'm right about these enforce statements being inappropriate, I'll
make 
a patch.
Feb 23 2014
parent reply "Jesse Phillips" <Jesse.K.Phillips+D gmail.com> writes:
On Sunday, 23 February 2014 at 17:51:34 UTC, Joseph Rushton 
Wakeling wrote:

 Assuming I'm right about these enforce statements being 
 inappropriate, I'll make a patch.


I believe you are correct, but the larger issue is that Phobos 
doesn't come with a debug build. So while they probably should be 
contracts, they will not be verified since the contracts will be 
compiled out.
Feb 23 2014
parent "monarch_dodra" <monarchdodra gmail.com> writes:
On Sunday, 23 February 2014 at 20:00:48 UTC, Jesse Phillips wrote:

 On Sunday, 23 February 2014 at 17:51:34 UTC, Joseph Rushton 
 Wakeling wrote:

 Assuming I'm right about these enforce statements being 
 inappropriate, I'll make a patch.


 I believe you are correct, but the larger issue is that Phobos 
 doesn't come with a debug build. So while they probably should 
 be contracts, they will not be verified since the contracts 
 will be compiled out.


It's "less" of an issue with phobos, which massivelly templated. 
Only very few things are non-template (AFAIK).

The bigger issue is things such as druntime, which is basically 
99% pre-compiled.

That, and there's also the broader issue of the conditionality of 
contracts should be decided by the "calling" library.
Feb 23 2014
prev sibling parent Jonathan M Davis <jmdavisProg gmx.com> writes:
On Sunday, February 23, 2014 16:10:02 monarch_dodra wrote:

 As a rule of thumb, "enforce" is not necessarily for things
 "user-input" but for things "outside the programmer's control"
 eg: "things that can legitimately fail", Notably, IO, threads,
 databases etc...
 
 If you see any phobos code that validates the range of its inputs
 via an enforce, please knock yourself out and assert it/contract
 it.


assert is used on input to a function when you consider it a programming bug 
for it to be given input that does not fulfill that condition. Exceptions are 
used when it's considered reasonable for the invalid input to be passed to the 
function. Where exactly to draw that line depends on what the function does 
and how it's likely to be used. A function that's likely to be used on data 
that was input into the program is more likely to use exceptions, whereas on 
whose parameters would not have originated from I/O at all are much more 
likely to use assertions. Unfortunately, knowing when to use one or the other 
is a bit of an art and definitely subjective.

However, in general, I would have thought that std.random would use 
assertions, given that its initialization and use does not normally involve 
operating on anything that originated from the user even indirectly, and as 
such, it's reasonable to require that the programmer give it valid input.

- Jonathan M Davis
Feb 25 2014