Against enforce()

[spir]

On 03/18/2011 05:07 PM, Steven Schveighoffer wrote:
> On Fri, 18 Mar 2011 11:35:22 -0400, spir <denis.spir at gmail.com> wrote:
>
>> On 03/18/2011 01:37 PM, Steven Schveighoffer wrote:
>>> This is a good example of why it's difficult to decide what "user input" is.
>>> One could consider that the 'user' in this case is the developer using the
>>> library, but I don't think that's the right choice.
>>>
>>> I'd say it's a bug, this is clearly a contract, since the data being passed
>>> into the ctor can easily not be user input (i.e. it's most likely two literals
>>> that will never depend on a user). If it is user input, the caller of the ctor
>>> should enforce the user input before passing it to iota.
>>
>> This is indeed a difficult topic. I'm a bit bluffed when reading people
>> confidently asserting apparently clear positions about the use of enforce vs
>> assert vs contracts and such, or whether such checks should or not stay or
>> not in various distribution builds (mainly -release).
>> I can see at least 5 cases, and am far to be sure what the proper tool is in
>> every case, and in which builds it should stay. In each case, there is
>> potential "wrong" input; but note the variety of cases does seems orthogonal
>> (lol) to what kind of harm it may cause:
>>
>> * colleague: my code is called by code from the same app (same dev team);
>> typically, wrong input logically "cannot" happen
>> * friend: my code is called by code designed to cooperate with it; there is a
>> kind of moral contract
>> In both cases, wrong input reveals a bug; but in the first case, it's my own
>> (team's) bug. I guess, but am not sure, these cases are good candidates for
>> asserts (or contracts?), excluded from release build.
>>
>> * lib call: my code is a typical lib; thus, I have zero control on caller.
>> I would let the check in release mode, thus use enforce. Or, use assert if it
>> remains when the *caller* is compiled in debug mode. There is something
>> unclear here, I guess. Maybe there are two sub-cases:
>> ~ the caller logically should be able to prove its args correct
>> ~ or not
>
> See, this is where I feel we have issues. The clear problem with *always*
> checking is the iota example. One may use iota like this:
>
> foreach(i; iota(0, 5))
>
> Why should checks in iota remain for iota to prove that 0 is less than 5? It
> always will be less than 5, and the check is not necessary.
>
> checks should only be in place during release when the input to the function
> cannot be proven at compile time. When it can be proven, then the checks should
> go away.
>
> The problem I see is it's iota's responsibility to do those checks, but it has
> no idea where the data comes from.
>
> What I would suggest is to check at the point the argument data is created, not
> at the point where it's used. So for instance, if you get the parameters for
> iota from an input file, then you need to check those arguments before passing
> to iota.
>
> This is a difficult problem to solve, because one party knows whether the
> arguments need to be checked, and the other party knows how to check the
> arguments. I don't know if there is a clean way to do this.
>
> My thoughts are that phobos should only use enforce where it can prove the
> arguments are runtime-generated, and rely on asserts otherwise. The obvious
> pitfall is that one could pass runtime-generated data to a phobos function
> which uses asserts, and the program could crash on an otherwise recoverable
> error because the user of phobos did not validate the input first. I think the
> risk here is less important than the reduction in performance that occurs when
> enforce is used instead.

Yes, I think you are correctly surrounding the issue.

But my choice would rather be safety as default. Would you really let the func 
called by a/b "non-check" b!=0? Thus, I would consider allowing the caller 
stating "don't check this call", not the opposite.
Another issue is this creates one more complication in the language; and one 
orthogonal to the whole set of func-calls; with syntax needed, I guess.

Another path is decision via compiler analysis: if arguments can be proved 
constant (I guess /this/ point can be made), then check is removed for release, 
automatically; else it cannot be removed at all.

[Thanks for the precision about enforce vs alwaysAssert.]

Denis
-- 
_________________
vita es estrany
spir.wikidot.com