Skip to content

Conversation

kennytm
Copy link
Contributor

@kennytm kennytm commented Jul 18, 2011

This pull request implements bug 5547: Improve assert to give information on values given to it when it fails.

Important: The pull request dlang/druntime#41 should be merged before pulling this.

The improved assert is activated only if it is in the unittest block, and there is no message (the 2nd argument).

Currently, it only recognizes expressions of the form:

    e1 <rel> e2
    !e1
    feqrel(...)
    approxEqual(...)
    opEqual(...)
    e1.opEqual(...)
    e1.opCmp(...)

where <rel> is one of the comparison operators (==, >=, !<>= etc) or the in, is, !is, &&, || operators, and e1, etc can be any expressions of the form above. When the assert fails, an AssertError with message e.g.

4 == 5 && 6 == 7  is unexpected

will be thrown.

Users who want the old behavior can provide an explicit message:

    assert(expr, "Assert Error");

Internally, the assert will be transformed into

       assert(p + q == r + s && feqrel(u * v, 40.0) > 45)

    => (auto __assertPred123 = p + q,
        auto __assertPred124 = r + s,
        auto __assertPred125 = u * v,
        assert(__assertPred123 == __assertPred124 && feqrel(__assertPred125, 40.0) > 45,
               __unittest_toString(__assertPred123)
                ~ " == "
                ~ __unittest_toString(__assertPred124)
                ~ " && " ~ "feqrel" ~ "("
                ~ __assertPred125 ~ ", " ~ "40" ~ ")" ~ " > " ~ "45"
                ~ "  is unexpected")
        )

where the function __unittest_toString is effectively std.conv.to!string, but it is put into druntime to ensure a certain degree of library-independence.

requires: druntime git://github.com/kennytm/druntime.git bug5547_assertPred

@yebblies
Copy link
Contributor

If you add a requires line to the description the autotester will be able to process this pull, currently it fails with link errors (naturally).
Something like requires: druntime git://github.com/kennytm/druntime.git bug5547_assertPred

@kennytm
Copy link
Contributor Author

kennytm commented Feb 19, 2012

Because of how the code was rewritten and bug 7546, a Phobos unit test will fail in 64-bit.

A workaround is

diff --git a/std/math.d b/std/math.d
index 9709dbb..6998853 100644
--- a/std/math.d
+++ b/std/math.d
@@ -2664,7 +2664,7 @@ unittest
     assert(sgn(168.1234) == 1);
     assert(sgn(-168.1234) == -1);
     assert(sgn(0.0) == 0);
-    assert(sgn(-0.0) == 0);
+    assert(sgn(-0.0f) == 0);
 }

 // Functions for NaN payloads

@WalterBright
Copy link
Member

fails auto tester

@kennytm
Copy link
Contributor Author

kennytm commented Feb 20, 2012

@WalterBright that's because dlang/druntime#41 is required, and that pull request in turn requires #725, and the auto tester does not resolve the dependency.

This commit makes expressions following some patterns in unittests emit
some more useful information. See assertpred.c for explanation.
@alexrp
Copy link
Contributor

alexrp commented Jul 10, 2012

Let's see what the auto tester says now.

Conflicts:
	src/posix.mak
	src/win32.mak
@kennytm
Copy link
Contributor Author

kennytm commented Jul 10, 2012

@alexrp : One does not simply expect a 5-month-old pull request still work automatically ☺. Fixed the merge break, waiting for the auto-testing to update the result.

@WalterBright
Copy link
Member

I have two problems with this:

  1. Currently, there are so many unittests in Phobos that the compiler sometimes runs out of memory. This will add a lot more and may push a lot more over the edge.
  2. I am not too happy about the dependencies on specific library names and functionality.

@WalterBright
Copy link
Member

On further reflection, I think that this is more properly the domain of a library template, such as

testEqual(arg1, arg2);

std.math.approxEqual can easily be extended to print its args on failure.

@donc
Copy link
Collaborator

donc commented Sep 6, 2012

std.math.approxEqual can easily be extended to print its args on failure.
No, it can't, at least not sensibly. The special thing about assert() is that it gets its arguments in string form. To work around this you'd get massive template bloat and unreadable code. Note that the main Phobos unittest memory problems come from std.datetime, which does pretty much that.

braddr pushed a commit to braddr/dmd that referenced this pull request Oct 22, 2012
dnadlinger added a commit to dnadlinger/druntime that referenced this pull request Jun 30, 2014
This was introduced as part of GitHub pull request dlang#41, which
was questionable (!) support code for dlang/dmd#263.
However, the latter never materialized and __unittest_toString plus
version(druntime_unittest) were just confusing oddities at this point.
@wilzbach wilzbach mentioned this pull request Jul 23, 2018
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants