On Friday, 8 February 2013 at 16:11:14 UTC, Ali Çehreli wrote:
> As the module writer, I don't know whether my function is going to be used as an "API function" or as an "internal function" in somebody else's module.

Then it is an API function, as far as I understand DbC, and should never rely on contract to validate input.

Moreover, what do people think about the situation whereby check's sole payload is to possibly throw an exception?

Normally under an exception-handling system, the behaviour of a function is: Do something, or if you cannot, then throw an exception.

But what is the "something" in the case of check?  As far as I can see, XML validity isn't something that check needs in order to do its business - rather, the validity checking _is_ the business.  On this basis, it probably ought to return a boolean indicating whether the input string is a valid XML document.

If the function is wholly or primarily for the library's internal use, it might make sense to have the checker function just throw the exception that will be passed back to the library user.  But this doesn't seem to be the point in this instance, since the caller swallows the exception and asserts.  It might as well just swallow a boolean return value (or a string return containing an error message) and assert.

And once the module is fixed so that the parser checks as it goes along, I suppose check would remain, but it would be a user function for the odd cases where you just want to check validity and don't want to process the content of the file for whatever reason.  On this basis again, logically it ought to return a pass/fail indication rather than using "expection handling".  But this would be a breaking change....

Stewart.

On 02/08/2013 08:18 AM, Dicebot wrote:
> On Friday, 8 February 2013 at 16:11:14 UTC, Ali Çehreli wrote:
>> As the module writer, I don't know whether my function is going to be
>> used as an "API function" or as an "internal function" in somebody
>> else's module.
>
> Then it is an API function, as far as I understand DbC, and should never
> rely on contract to validate input.

It is an internal function if I use it so:

import std.math;
import std.exception;

void foo(double a, double b)
{
    enforce(a > b, "a must be greater than b");
    sqrt(a - b);
}

void main()
{}

See how sqrt() is *not* on my module's API? I have already enforced that my API function foo() is called correctly. My call to sqrt() is an internal call of my module. Therefore the checks inside sqrt() should not be repeated beyond my testing stage.

Ali

On 02/08/2013 09:00 AM, Stewart Gordon wrote:
> Moreover, what do people think about the situation whereby check's sole
> payload is to possibly throw an exception?
>
> Normally under an exception-handling system, the behaviour of a function
> is: Do something, or if you cannot, then throw an exception.
>
> But what is the "something" in the case of check? As far as I can see,
> XML validity isn't something that check needs in order to do its
> business - rather, the validity checking _is_ the business. On this
> basis, it probably ought to return a boolean indicating whether the
> input string is a valid XML document.

Absolutely.

It should still throw if the task of checking fails e.g. for no memory for internal state.

Ali

On Friday, 8 February 2013 at 17:16:25 UTC, Ali Çehreli wrote:
> On 02/08/2013 08:18 AM, Dicebot wrote:
> > On Friday, 8 February 2013 at 16:11:14 UTC, Ali Çehreli wrote:
> >> As the module writer, I don't know whether my function is
> going to be
> >> used as an "API function" or as an "internal function" in
> somebody
> >> else's module.
> >
> > Then it is an API function, as far as I understand DbC, and
> should never
> > rely on contract to validate input.
>
> It is an internal function if I use it so:
>
> import std.math;
> import std.exception;
>
> void foo(double a, double b)
> {
>     enforce(a > b, "a must be greater than b");
>     sqrt(a - b);
> }
>
> void main()
> {}
>
> See how sqrt() is *not* on my module's API? I have already enforced that my API function foo() is called correctly. My call to sqrt() is an internal call of my module. Therefore the checks inside sqrt() should not be repeated beyond my testing stage.
>
> Ali

What is the point of an "in" contract, if not to check that the *caller* code is correct? Still assuming "sqrt" can't handle negatives, why bother with an "in", when you could just as well assert?

Heck, at this point, how is an "in" any different than a "version(assert) {...}", and an "out" an "version(assert) scope(exit) { ... }" ?

compile this program in debug:
//----
void main()
{
  auto a = [1, 2, 3];
  auto b = [1, 2];
  a[] += b[];
}
//----
Well herp dee derp! Look at it silently running and failing! The idea behind a debug build is that I should get an error for this kind of crap. But I don't. That's bad.

No end-user should be using a non-release version of druntime: That has  already been validated. On the other hand, once compiled in release, all the checks that *should* be executed when the caller is trying to validate his code *aren't* run.

Regardless of "where" the contract is compiled in (implementation detail), its inclusion (or lack thereof) should be at the callers' discretion.

On 02/08/2013 09:55 AM, monarch_dodra wrote:

> Regardless of "where" the contract is compiled in (implementation
> detail), its inclusion (or lack thereof) should be at the callers'
> discretion.

In case it is not obvious, I agree with all you said. :)

Ali

On Friday, 8 February 2013 at 18:06:34 UTC, Ali Çehreli wrote:
> On 02/08/2013 09:55 AM, monarch_dodra wrote:
>
> > Regardless of "where" the contract is compiled in
> (implementation
> > detail), its inclusion (or lack thereof) should be at the
> callers'
> > discretion.
>
> In case it is not obvious, I agree with all you said. :)
>
> Ali

Stupid text-based communication.

Sorry.

On Friday, February 08, 2013 07:54:52 Andrei Alexandrescu wrote:
> On 2/8/13 6:25 AM, monarch_dodra wrote:
> > "in" and "out" contracts themselves are flawed in D in any case, given they are part of the "called" code, as opposed to "caller" code.
> 
> What would be the right design and implementation?

The way that it _should_ work but doesn't (and probably can't given D's linking model) is to insert in and out contracts at the call site so that it's the caller's compilation flags which decide whether the contracts are compiled in or not. They're testing the _caller's_ code after all. But the way that it's currently done, it's up to whoever released the library to decide whether the contracts are compiled in or not. For instance, it would ideally be possible for a program to be built with all of the debug stuff turned on (including assertions) and link against Phobos and have all of the in and out contracts on Phobos functions enabled (because the caller has assertions enabled). Instead, you have to build Phobos with assertions turned on in order to get them, even though you don't care about assertions which are internal to Phobos and don't need any debugging anything enabled in Phobos beyond the in and out contracts (since you're not debugging Phobos, just your code).

Unfortunately, while that's how it really _should_ work, AFAIK, there's no way with D's linking model to make things work that way. You can link against functions without any access to their bodies. Function pointers make it trivial to use a function without the compiler knowing what function your using (meaning that it couldn't insert the contracts at the call point). Etc. Etc. The contracts would have to be passed around with the functions in a manner which made it so that the caller could always insert them if it's being compiled with assertions enabled, and that just won't work.

So, yes. The implementation of D's contracts is flawed in the sense that it doesn't work the way that it would ideally work, but I don't think that it's actually possible to make it work the way that it would ideally work.

- Jonathan M Davis

On 2013-02-08 18:16, Ali Çehreli wrote:
> void foo(double a, double b)
> {
>      enforce(a > b, "a must be greater than b");
>      sqrt(a - b);
> }
>
> void main()
> {}
>
> See how sqrt() is *not* on my module's API? I have already enforced that my API
> function foo() is called correctly. My call to sqrt() is an internal call of my
> module. Therefore the checks inside sqrt() should not be repeated beyond my
> testing stage.


This calls for some way to be able to call sqrt without running checks:

    sqrt(a - b);   // would run all "in" checks
    #sqrt(a - b);  // tells the compiler to bypass the "in" checks in sqrt

Sorry for inventing clumsy syntax, but you get the picture.
Naturally it won't bypass checking if sqrt is in a compiled library.

On 2013-02-08 18:00, Stewart Gordon wrote:

> But this would be a breaking change....

This can be solved by moving the functionality to a new function, say "isValid", which returns a boolean. The implementation of "check" would then call "isValid" and throw an exception if it returns false.

-- 
/Jacob Carlborg