http://d.puremagic.com/issues/show_bug.cgi?id=5547

           Summary: Improve assert to give information on values given to
                    it when it fails
           Product: D
           Version: unspecified
          Platform: All
        OS/Version: All
            Status: NEW
          Severity: enhancement
          Priority: P2
         Component: DMD
        AssignedTo: nobody@puremagic.com
        ReportedBy: jmdavisProg@gmx.com


--- Comment #0 from Jonathan M Davis <jmdavisProg@gmx.com> 2011-02-08 07:47:45 PST ---
Right now, assert gives no useful information beyond the file and line number where the failure occurred. It really should give information on what the failure was. The recent attempt at getting assertPred in Phobos was to fix this, but it was decided that it would be better to fix assert to do it. So, essentially, when asserts like

assert(a == b);
assert(a.canFind(b));

fails, it should print out what the arguments to the expression were. For instance, take this from the assertPred docs:

assert(collectExceptionMsg!AssertError(
            assertPred!"=="("hello", "goodbye")) ==
       `assertPred!"==" failed:` ~ "\n" ~
       `[hello] (lhs)` ~ "\n" ~
       `[goodbye] (rhs).`);

On failure assertPred would print out that == failed and that the values given to it were hello and goodbye. Ideally, assert("hello" == "goodbye"); would print something similar.

Another example would be

assert(collectExceptionMsg!AssertError(
            assertPred!((int a, int b, int c, float d){return a * b < c * d;})
                        (22, 4, 5, 1.7)) ==
       `assertPred failed: arguments: [22], [4], [5], [1.7].`);

Something similar to this - such as

assert((int a, int b, int c, float d){return a * b < c * d;}(22, 4, 5, 1.7));
or
assert(22 * 4 < 5 * 1.7);

- should print something similar to what assertPred printed.

Exactly what the best format should be and exactly what values will be best for assert to print for more complicated expressions, I don't know - that will depend on what is reasonable to implement - but assert should at least make a valiant attempt at printing what the values in the expression were that failed so that the programmer has sufficient debugging information not to have to go and add a debug statement to print out the arguments, because the assert didn't give enough information to properly debug the failure.

Also, it was pointed out that assertPred should print values such that they line up on separate lines so that they're easier to compare (which it does in the == case, though not in the case of the more general predicate - perhas it should have). So, it would probably be a good idea for assert to do something similar.

These improvements need to be made for assert to be properly usable for unit tests. Otherwise, we're really going to need to add something like assertPred to Phobos.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=5547


Don <clugdbug@yahoo.com.au> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |clugdbug@yahoo.com.au


--- Comment #1 from Don <clugdbug@yahoo.com.au> 2011-02-08 10:49:00 PST ---
There's one difficult issue in this: this assert behaviour is not always
desirable, especially outside of unittests.
You have to save a copy of the parameters to assert, and that can be quite
expensive -- extremely expensive, in some cases.
And, you also need to be able to print them out, which creates the risk of
recursion, and potentially links in a huge amount of code.

The basic idea of making assert() be syntax sugar for an internal function
which behaves like your assertPred() does not seem too difficult. But knowing
*when* to do it is difficult.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=5547



--- Comment #2 from Steven Schveighoffer <schveiguy@yahoo.com> 2011-02-08 10:57:27 PST ---
Is it too difficult for this to apply only inside unit tests?  I mean literally within unit test blocks.  This means asserts triggered in non-unit test blocks will not report the extra info, even if called from a unit test block.  But I believe this is equivalent to the original proposal of assertPred (which would only be used inside unittest blocks).

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=5547


kennytm@gmail.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |patch
                 CC|                            |kennytm@gmail.com


--- Comment #3 from kennytm@gmail.com 2011-07-18 08:17:14 PDT ---
DMD pull #263 https://github.com/D-Programming-Language/dmd/pull/263 druntime pull #41 https://github.com/D-Programming-Language/druntime/pull/41 Phobos pull #149 https://github.com/D-Programming-Language/phobos/pull/149

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=5547


kennytm@gmail.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|patch                       |pull


--- Comment #4 from kennytm@gmail.com 2012-02-16 15:10:22 PST ---
Updated pull requests for 2.058. (The IDs are still the same).

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=5547



--- Comment #5 from github-bugzilla@puremagic.com 2012-07-08 13:46:35 PDT ---
Commits pushed to master at https://github.com/D-Programming-Language/druntime

https://github.com/D-Programming-Language/druntime/commit/17db389925581804a3d6d8a10191772077b9713c Bug 5547: assertPred (the druntime part)

https://github.com/D-Programming-Language/druntime/commit/ce783fff516d21c253edfecb40982c833add3e4b Merge pull request #41 from kennytm/bug5547_assertPred

Bug 5547: assertPred (the druntime part)

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=5547


Walter Bright <bugzilla@digitalmars.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
                 CC|                            |bugzilla@digitalmars.com
         Resolution|                            |WONTFIX


--- Comment #6 from Walter Bright <bugzilla@digitalmars.com> 2012-09-05 15:35:51 PDT ---
As I commented on the pull request:

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.



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.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=5547



--- Comment #7 from Jonathan M Davis <jmdavisProg@gmx.com> 2012-09-05 15:45:23 PDT ---
> On further reflection, I think that this is more properly the domain of a
library template

I'm fine with having a helper function that does this, but the whole reason that this enhancement was created in the first place was because enough folks in the newsgroup thought that that it should be built into assert rather than creating a new function to do it.

Though it should be noted that the library function does tend to cause memory issues as well, since you end up with a lot of different instantiations for it.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=5547


bearophile_hugs@eml.cc changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |bearophile_hugs@eml.cc


--- Comment #8 from bearophile_hugs@eml.cc 2012-09-05 16:07:28 PDT ---
(In reply to comment #6)

> 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.

A large program written by D users risks having the same amount of unit-tests, so this is a general problem, not just of Phobos.

So maybe the right solution is not to keep assert() dumb, but to find ways to compile unit-tests using much less memory.

I think this idea also goes well with compiling unit-tests more independently from each other, to give the user a simple summary of what tests have failed.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------