std.unittests for (final?) review

[Jonathan M Davis]

On Wednesday 05 January 2011 22:36:32 Jonathan M Davis wrote:
> 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.

Actually, for this to work, you'd also have to consider a function which returns 
bool as a boolean operator even if it isn't an operator, otherwise cases such as 
testing a function which returned a bool would end up just printing out the 
result, which wouldn't be particularly useful.

- Jonathan m Davis