std.unittests [updated] for review

[Jonathan M Davis]

On Tuesday 01 February 2011 09:05:18 Jens Mueller wrote:
> Michel Fortin wrote:
> > On 2011-02-01 10:34:26 -0500, Andrei Alexandrescu
> > 
> > <SeeWebsiteForEmail at erdani.org> said:
> > >On 2/1/11 9:21 AM, Don wrote:
> > >>Jonathan M Davis wrote:
> > >>>Do you really find
> > >>>
> > >>>assertPred!"=="(min(5, 7), 5);
> > >>>
> > >>>to be all that harder to understand than
> > >>>
> > >>>assert(min(5, 7) == 5);
> > >>
> > >>I do. *Much* harder. Factor of two, at least.
> > >>In absolute terms, not so much, because it was the original assert is
> > >>very easy to understand. But the relative factor matters enormously.
> > >>Much as comparing:
> > >>a.add(b);
> > >>a += b;
> > >>
> > >>And I think this is a very important issue.
> > >>
> > >> >I don't see how these functions could be anything but an improvement.
> > >> >
> > >> > But even if they get into Phobos, you obviously don't have to use
> > >> > them.
> > >>
> > >>This is not true. Including them in Phobos gives a legitimacy to that
> > >>style of programming. It's a role model.
> > >>
> > >>Including stuff like this could give D a reputation for lack of
> > >>readability. My belief is that right now, the #1 risk for Phobos is
> > >>that it becomes too clever and inaccessible.
> > >>
> > >>IMHO, something simple which has any appearance of being complicated,
> > >>needs a VERY strong justification.
> > >
> > >Does this count as a vote against the submission?
> > 
> > To me this is a compelling argument against. That and the fact that
> > it can't really mimic the true behaviour of assert in some
> > situation. assertPred won't work correctly for 'in' contracts with
> > inheritance, where the compiler generate the assertion code
> > differently.
> > 
> > In my view, the correct way to improve assertion error messages is
> > to improve how the compiler handle assertions (it should output
> > messages like it does for static asserts).
> > 
> > assertPred might be fine as a stopgap solution, but personally I'd
> > not make it part of the public API. We can't tell people to use
> > assert() everywhere and then tell them they should use
> > assertPred!op() if they want a useful error message except for 'in'
> > contracts of virtual functions; that'd just be too confusing.
> 
> Actually we say use assertPred/etc. when writing unittests, don't we? To
> me that is not complicated. There used to be a version(unittest). I
> don't know for what reason it got removed. If it was still there an else
> could help a bit. Like
> version(unittest) {
> }
> else {
>     static assert(false, "assertPred/etc. are only available within
> unittests."); }
> Now every code compiled without -unittest and using assertPred/etc will
> fail to compile. But of course with -unittest it still seems that it
> will work in contracts. I mean at least we can make the problem explicit
> in most cases. Not sure how likely it is that people will distribute
> software that they only compiled with unittests.

The version(unittest) block got removed, because various people thought that it 
should be usable in normal code. It wouldn't be hard to put a warning on 
assertPred about contract inheritance if that were necessary, though I have to 
wonder whether contract inheritance is implemented well if it has to use special 
asserts like that. It makes it harder to use helper functions with that sort of 
restriction, and I frequently create helper functions to check invariants and 
the like (though usually, they return a bool, and then they can be used with 
either assert or enforce).

- Jonathan M Davis