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

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.

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.

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.

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.

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