std.unittests for (final?) review

Wed Jan 5 22:36:32 PST 2011

On Wednesday 05 January 2011 22:04:24 Jonathan M Davis wrote:
> On Wednesday 05 January 2011 21:09:07 Michel Fortin wrote:
> > On 2011-01-05 22:57:00 -0500, Jonathan M Davis <jmdavisProg at gmx.com> said:
> > > On Wednesday 05 January 2011 19:35:13 Michel Fortin wrote:
> > >> I'm not sold on the concept. The whole point of this module seems to
> > >> offer a way to replace the built-in assertion mechanism with a
> > >> customized one, with the sole purpose of giving better error messages.
> > >> 
> > >> So we're basically encouraging the use of:
> > >> 	assertPredicate!"a > b"(a, b, "message");
> > >> 
> > >> instead of:
> > >> 	assert(a > b, "message");
> > >> 
> > >> It looks like an uglification of the language to me.
> > >> 
> > >> I agree that getting better error messages is important (very
> > >> important in fact), but keeping the code clean is important too. If
> > >> the built-in assert doesn't give us good enough error messages,
> > >> perhaps it's the built-in assert that should be improved. The
> > >> compiler could give the values on both side of the operator to the
> > >> assertion handler, which would in turn print values and operator as
> > >> part of the error message.
> > >> 
> > >> So to me this module is a temporary fix until the compiler is capable
> > >> of giving the necessary information to the assertion handler. I sure
> > >> hope it won't be needed for too long.
> > >> 
> > >> (Note: this criticism doesn't apply to those assertions dealing with
> > >> exceptions.)
> > > 
> > > Well, I'm not about to claim that assert can't be fixed to give better
> > > error messages, but right now all it takes is a value which converts to
> > > bool for the test. a > b may obviously be convertible to something
> > > similar to assertPred!">"(a, b), but what about something like 1 + 1 <
> > > b or a < b < c. As expressions get progressively more complicated, it
> > > very quickly becomes non- obvious what someone would really want to
> > > print on error. Would 1 + 1 < b print 2 and b's value? Would it print
> > > 1, 1, and b's value? 1, 1, 2, and b's value? Sure, it may be obvious
> > > to the programmer what they intended, but it doesn't take much for it
> > > to be very difficult for the compiler to figure it out for you.
> > 
> > I think "assert(1+a < b)" should print the same thing as "static
> > assert(1+a < b)" does. What "static assert(1+a < b)" prints when a == 1
> > and b == 0 is "(2 < 0) is false". Try it yourself.
> > 
> > > Also, assertPred!">"(a, b) would print out a more informative error
> > > message on its own. You wouldn't need to give it an additional message
> > > for it to be more informative. That would defeat the point. Even
> > > assertPred!"a > b"(a, b) could be
> > > more informative (assuming that it treats a > b as a general predicate
> > > rather than determining that it's actually >) by printing the values
> > > that it's given. So, that's definitely a leg up on assert(a > b) right
> > > there.
> > 
> > I don't believe it to be that difficult. From inside the compiler, you
> > have access to the expression tree. All the compiler needs to do is
> > check whether the top level expression is a binary op, and if so
> > 
> > decompose it this way (assuming no given message here):
> > 	auto a = operand1;
> > 	auto b = operand2;
> > 	if (a <binaryop> b)
> > 	
> > 		_d_assert_msg2("(%s <binaryop> %s) is false", __FILE__, __LINE__, &a,
> > 
> > typeid(a), &b, typeid(b));
> > 
> > As for other expressions it could simply print the value by lowering it
> > 
> > this way:
> > 	auto result = <expression>;
> > 	if (result)
> > 	
> > 		_d_assert_msg1("(%s) is false", __FILE__, __LINE__, &result,
> > 
> > typeid(result));
> > 
> > That would basically give you the same error messages as static assert.
> > 
> > Currently, assertions are lowered like this instead:
> > 	if (expression)
> > 	
> > 		_d_assertm(moduleinfo, __LINE__);
> > 
> > or like this when a message is provided:
> > 	if (expression)
> > 	
> > 		_d_assertm(<message>, __FILE__, __LINE__);
> > 
> > Sure, it's more complicated than doing it for static asserts where
> > everything is known at compile-time, but I don't believe it to be that
> > difficult.
> > 
> > > By passing each of the values to assertPred, we're able to print them
> > > out on failure without the computer having to understand what the
> > > predicate does, even when the values are arbitrary expressions. That
> > > would be very hard to do with an
> > > improved assert which just took the expression. I mean, try and write a
> > > function
> > > that took 1 + 1 > b or a < b < c as a string and tried to correctly
> > > print out values which are meaningful to the programmer. That would be
> > > _really_ hard. And while assertPred may not be able to understand a
> > > generic predicate, it can know about specific operators and/or
> > > functions and therefore give more informative error messages than it
> > > would be able to do with a generic predicate.
> > 
> > It's hard to do using a function. But it's easy for 'assert' because
> > it's a language construct handled by the compiler.
> > 
> > > So, correctly implemented, I think that assertPred actually makes a lot
> > > more sense than trying to soup up assert and getting the compiler to
> > > guess at what the programmer really wants.
> > 
> > I don't really see what the compiler has to guess. The compiler just
> > takes the top-level expression and pass its value to the assertion
> > handler, and for binary expressions it can pass two values plus the
> > operator's string. What cases are not covered by that?
> 
> If you write
> 
> assertPred!"<"(foo(), 7)
> 
> and it fails, it would print out the value of foo(). Something like, "5 is
> not less than 7". What should assert(foo() < 7) print? The value of the
> expression is false. We know that because the assertion failed, so there's
> no point in printing that. And if you want anything like "5 is not less
> than 7", what are you going to do? If you want it to print something like
> "assertion failed: 5 < 7", how does it know that you wanted to stop the
> evaluation of the expression at the point where foo() has been evaluated?
> Simply because there was only one evaluation left? That would deal with
> plenty of binary cases, but it wouldn't scale. What about something like a
> < b && c < d? If assertPred!() takes more than two parameters (as I would
> hope it would), then you could do something like assertPred!((a, b, c,
> d){return a < b && c < d;})(foo(), bar(), hello(), world()) and it could
> not only tell you that the predicate failed, but it could tell you that
> the values that it was given were 1, 5, 4, and 2. How could assert(foo() <
> bar() && hello() < world()) do that? It has to know where to stop the
> expressions evaluation to print it. Stopping with only one evaluation left
> (true && false) wouldn't be particularly useful, and it certainly wouldn't
> be what happened with assertPred!(). So, in many cases, the compiler
> either has to somehow guess where you want the evaluation to stop, or it's
> going to print sub- optimal information.
> 
> assertPred!() would allow you to have control over what values get printed.
> The whole point of something like assertPred!() IMHO is to improve the
> output on error - in particular to print out the values being tested. I
> don't see how assert() could do that quite as nicely, even if it became as
> smart as you suggest.

Okay. I thought this through a bit more, and I think that if the evaluation was 
stopped when all that was left in the expression was boolean operators and their 
operands, then that pretty much has to be what the programmer was trying to 
print. That being the case, you could theoretically get assert to do it, but I 
would expect that that would make assert awfully complicated. Done properly, it 
would be fantastic, but since it can be done in a library with something like 
assertPred!() much more easily, I wouldn't expect such an enhancement to assert 
to be implemented any time soon.

A few of the things that I'm thinking of doing with assertPred!() have to be 
special cased though in a way which wouldn't work with assert. For instance, I'm 
thinking of doing something like assertPred!("opCmp", 0)(foo(), bar()), which 
would do what assertOpCmp!() does now. As such, opCmp() isn't a predicate, if 
you rewrote it to assert(foo().opCmp(bar()) == 0), even stopping evaluation when 
all you have left is boolean operators and their operands wouldn't work, since 
then you'd get something like 1 == 0 rather than (assuming that foo() and bar() 
return something like BigInts which have the values "4" and "2" respectively)  
something like "opCmp() == 0 failed: 4 > 2".

Sure, you could make it so that assert could do that sort of thing too, but then 
you're adding special cases and complicating assert even further, and it's also 
much harder to add such cases, because then you're actually changing the 
_language_ rather than the standard library, and every compiler would have to 
follow suit.

So, yes assert could theoretically be improved to do a lot more and do a lot 
better, but it won't ever do as much as I could do with assertPred!(), and even 
if assert _is_ improved as you suggest (which would certainly be cool), then 
there's still some value in assertPred!().

- Jonathan M Davis