www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - std.unittests [updated] for review

reply Jonathan M Davis <jmdavisProg gmx.com> writes:
In case you didn't know, I have a set of unit test helper functions which have 
been being reviewed for possible inclusion in phobos. Here's an update.

Most recent code: http://is.gd/F1OHat

Okay. I took the previous suggestions into consideration and adjusted the code
a 
bit more. However, most of the changes are to the documentation (though there 
are some changes to the code). Some of the code duplication was removed, and
the 
way that some of the assertPred functions' errors are formatted has been
altered 
so that values line up vertically, making them easier to compare. The big
change 
is the docs though. There's now a fake version of assertPred at the top with an 
overall description for assertPred followed by the individual versions with as 
little documentation as seemed appropriate while still getting all of the 
necessary information across. A couple of the functions still have irritatingly 
long example sections, but anything less wouldn't get the functionality across.

In any case. Here's the updated code. Review away. Andrei set the vote deadline 
for February 7th, at which point, if it passes majority vote, then it will go 
into Phobos. The number of functions is small enough now (thanks to having 
consolidated most of them into the fantastically versatile assertPred) that it 
looks like it will likely go in std.exception if the vote passes rather than 
becoming a new module. So, the std.unittests title has now become a bit of a 
misnomer, but that's what I've been calling it, so it seemed appropriate to 
continue to label it that way in the thread's title.

- Jonathan M Davis
Jan 24 2011
next sibling parent "Nick Sabalausky" <a a.a> writes:
"Jonathan M Davis" <jmdavisProg gmx.com> wrote in message 
news:mailman.888.1295879701.4748.digitalmars-d puremagic.com...
 In case you didn't know, I have a set of unit test helper functions which 
 have
 been being reviewed for possible inclusion in phobos. Here's an update.

 Most recent code: http://is.gd/F1OHat

 Okay. I took the previous suggestions into consideration and adjusted the 
 code a
 bit more. However, most of the changes are to the documentation (though 
 there
 are some changes to the code). Some of the code duplication was removed, 
 and the
 way that some of the assertPred functions' errors are formatted has been 
 altered
 so that values line up vertically, making them easier to compare. The big 
 change
 is the docs though. There's now a fake version of assertPred at the top 
 with an
 overall description for assertPred followed by the individual versions 
 with as
 little documentation as seemed appropriate while still getting all of the
 necessary information across. A couple of the functions still have 
 irritatingly
 long example sections, but anything less wouldn't get the functionality 
 across.

 In any case. Here's the updated code. Review away. Andrei set the vote 
 deadline
 for February 7th, at which point, if it passes majority vote, then it will 
 go
 into Phobos. The number of functions is small enough now (thanks to having
 consolidated most of them into the fantastically versatile assertPred) 
 that it
 looks like it will likely go in std.exception if the vote passes rather 
 than
 becoming a new module. So, the std.unittests title has now become a bit of 
 a
 misnomer, but that's what I've been calling it, so it seemed appropriate 
 to
 continue to label it that way in the thread's title.

Very nice! One minor change I'd suggest, for both readability and consistency: assertPred!"a == b"(1, 2); Current result: assertPred!"a == b" failed: [1] (a), [2] (b). Suggested result: assertPred!"a == b" failed: [1] (a) [2] (b) Or maybe even: assertPred!"a == b" failed: 1 == 2 [1] (a) [2] (b) Note the variable->value subsitution made on the original predicate string.
Jan 24 2011
prev sibling next sibling parent Tomek =?UTF-8?B?U293acWEc2tp?= <just ask.me> writes:
Dnia 2011-01-24, o godz. 06:34:49
Jonathan M Davis <jmdavisProg gmx.com> napisa=C5=82(a):

 In case you didn't know, I have a set of unit test helper functions which=

 been being reviewed for possible inclusion in phobos. Here's an update.
=20
 Most recent code: http://is.gd/F1OHat
=20
 Okay. I took the previous suggestions into consideration and adjusted the=

 bit more. However, most of the changes are to the documentation (though t=

 are some changes to the code). Some of the code duplication was removed, =

 way that some of the assertPred functions' errors are formatted has been =

 so that values line up vertically, making them easier to compare.

That's a solid improvement, thanks.
 The big change=20
 is the docs though. There's now a fake version of assertPred at the top w=

 overall description for assertPred followed by the individual versions wi=

 little documentation as seemed appropriate while still getting all of the=

 necessary information across. A couple of the functions still have irrita=

 long example sections, but anything less wouldn't get the functionality a=

I'm not sure... Examples: assertPred!"+"(7, 5, 12); assertPred!"-"(7, 5, 2); assertPred!"*"(7, 5, 35); assertPred!"/"(7, 5, 1); assertPred!"%"(7, 5, 2); assertPred!"^^"(7, 5, 16_807); assertPred!"&"(7, 5, 5); assertPred!"|"(7, 5, 7); assertPred!"^"(7, 5, 2); assertPred!"<<"(7, 1, 14); assertPred!">>"(7, 1, 3); assertPred!">>>"(-7, 1, 2_147_483_644); assertPred!"~"("hello ", "world", "hello world"); assert(collectExceptionMsg(assertPred!"+"(7, 5, 11)) =3D=3D "assertPred!\"+\" failed: [7] + [5]:\n" ~ "[12] (actual)\n" ~ "[11] (expected)."); assert(collectExceptionMsg(assertPred!"/"(11, 2, 6, "It failed!")) =3D=3D "assertPred!\"/\" failed: [11] / [2]:\n" ~ "[5] (actual)\n" ~ "[6] (expected): It failed!"); Picking only one or two from the above would be enough to "get it". It's th= e description that ought to explain the function's behavior in all cases, e= xamples are for jump-starting the user to action. Oh, one more thing. Previously you asked me why a generic collectThrown is = useful and I forgot to answer. One use is the same as collectExceptionMsg()= without being tied to the msg property. auto e =3D collectThrown!MyException(expr); assert(e); assert(e.errorCode =3D=3D expectedCode); assert(cast(MyCauseException) e.next); I'm not proposing to yank collectExceptionMsg or assertThrown in favor of c= ollectThrown, they're useful idioms. But having also collectThrown (a gener= ic replacement for existing collectException) would definitely be of value.
 In any case. Here's the updated code. Review away. Andrei set the vote de=

 for February 7th, at which point, if it passes majority vote, then it wil=

 into Phobos. The number of functions is small enough now (thanks to havin=

 consolidated most of them into the fantastically versatile assertPred) th=

 looks like it will likely go in std.exception if the vote passes rather t=

 becoming a new module. So, the std.unittests title has now become a bit o=

 misnomer, but that's what I've been calling it, so it seemed appropriate =

 continue to label it that way in the thread's title.

Good luck! --=20 Tomek
Jan 24 2011
prev sibling next sibling parent reply "Masahiro Nakagawa" <repeatedly gmail.com> writes:
On Mon, 24 Jan 2011 23:34:49 +0900, Jonathan M Davis <jmdavisProg gmx.com>  
wrote:

 In case you didn't know, I have a set of unit test helper functions  
 which have
 been being reviewed for possible inclusion in phobos. Here's an update.

 Most recent code: http://is.gd/F1OHat

 Okay. I took the previous suggestions into consideration and adjusted  
 the code a
 bit more. However, most of the changes are to the documentation (though  
 there
 are some changes to the code). Some of the code duplication was removed,  
 and the
 way that some of the assertPred functions' errors are formatted has been  
 altered
 so that values line up vertically, making them easier to compare. The  
 big change
 is the docs though. There's now a fake version of assertPred at the top  
 with an
 overall description for assertPred followed by the individual versions  
 with as
 little documentation as seemed appropriate while still getting all of the
 necessary information across. A couple of the functions still have  
 irritatingly
 long example sections, but anything less wouldn't get the functionality  
 across.

 In any case. Here's the updated code. Review away. Andrei set the vote  
 deadline
 for February 7th, at which point, if it passes majority vote, then it  
 will go
 into Phobos. The number of functions is small enough now (thanks to  
 having
 consolidated most of them into the fantastically versatile assertPred)  
 that it
 looks like it will likely go in std.exception if the vote passes rather  
 than
 becoming a new module. So, the std.unittests title has now become a bit  
 of a
 misnomer, but that's what I've been calling it, so it seemed appropriate  
 to
 continue to label it that way in the thread's title.

I vote Andrei's suggestion, std.exception is better than new std.unittests. I think testing module should provide more features(e.g. Mock, Stub...). Your helpers help assert writing style but not help testing. In addition, std.exception already defined similar functions. Masahiro
Jan 30 2011
next sibling parent Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 01/30/2011 06:13 AM, Jens Mueller wrote:
 Masahiro Nakagawa wrote:
 I vote Andrei's suggestion, std.exception is better than new std.unittests.
 I think testing module should provide more features(e.g. Mock, Stub...).
 Your helpers help assert writing style but not help testing.
 In addition, std.exception already defined similar functions.

I do not like putting it in std.exception. Maybe the name std.unittest is also not good. I would propose std.assert if assert wasn't a keyword. When I use std.exception I want to handle situations that are part of the spec (i.e. exceptions) whereas Jonathan's module helps me writing asserts (that's most of the time unittests). Basically it helps me verifying behavior according to a spec. I want to keep the dichotomy of errors and exceptions. Putting both things in one module is rather strange to me. What are the arguments for putting it in std.exception? I find the size a rather weak argument. I thought about providing an assertDeath ones std.process is redone. And even though enforce and assert are mirroring each other they are used in different contexts. I would _not_ expect helpers for writing assertions (Assert_Error_) in a module named std.exception. Jens

assertThrows and its converse are a good fit for std.exception. Then we're left with a couple of concepts that don't deserve their own module and are difficult to fit elsewhere. I reckon that in a perfect world there would be a better match, but I don't cringe at the thought of std.exception holding them. Andrei
Jan 30 2011
prev sibling next sibling parent spir <denis.spir gmail.com> writes:
On 01/30/2011 01:13 PM, Jens Mueller wrote:
 I do not like putting it in std.exception. Maybe the name std.unittest
 is also not good. I would propose std.assert if assert wasn't a keyword.
 [...]

 assertions (Assert_Error_) in a module named std.exception.

Same for me. Find it strange. Would never search assertion helper funcs inside std.exception. Why not std.assertion? std.unittests would be fine if there were some more stuff in there, I mean not only assertions. Else, the obvious name imo is std.assertion. Denis -- _________________ vita es estrany spir.wikidot.com
Jan 31 2011
prev sibling next sibling parent spir <denis.spir gmail.com> writes:
On 01/30/2011 02:13 PM, Jens Mueller wrote:
 The only argument against putting in its own module is it's size. That
 seems to be your main point. I think putting something in new module
 should mainly be concerned with separating stuff logically. Later on it
 should be easy to add a new module that imports std.exception and let's
 say std.assertion.
 But all you say is valid. I just fear that fixing names later will be
 costly. Having two modules allows us to handle this better. Just because
 std.exception is already kind of messy doesn't justify putting in
 there.

I feel the same. Denis -- _________________ vita es estrany spir.wikidot.com
Jan 31 2011
prev sibling parent spir <denis.spir gmail.com> writes:
On 02/01/2011 12:49 AM, Jens Mueller wrote:
 spir wrote:
 On 01/30/2011 01:13 PM, Jens Mueller wrote:
 I do not like putting it in std.exception. Maybe the name std.unittest
 is also not good. I would propose std.assert if assert wasn't a keyword.
 [...]

 assertions (Assert_Error_) in a module named std.exception.

Same for me. Find it strange. Would never search assertion helper funcs inside std.exception. Why not std.assertion? std.unittests would be fine if there were some more stuff in there, I mean not only assertions. Else, the obvious name imo is std.assertion.

Nice. I just wonder what others think. I'd like to start a poll http://doodle.com/vn2ceenuvfwtx38e In general are those polls useful? I mean there where some discussions in the last time where a poll may help. git vs. hg. 80 vs. 90 characters per line. If all arguments are on the table it can be useful to have an opinion poll to finally settle the discussion. It may even be that I'm totally wrong here. But I think the module naming needs to be done very careful. It's what a D newcomer needs to grasp as easy as possible. Am I too picky? Somehow I care (too?) much about names. Jens

I care extremely about names, in general (as people may have noticed). Even more, as you say, for standard modules, which names should be as obvious as possible for newcomers. This also requires a clear policy for stdlib structure / organisation; present one, if any, is a mystery for me. I doubt, though, a poll may be good decision means; at best, it may help and have a rough review of what names /not/ to choose. (voted for your poll anyway, just for fun ;-) Denis -- _________________ vita es estrany spir.wikidot.com
Feb 01 2011
prev sibling next sibling parent Jens Mueller <jens.k.mueller gmx.de> writes:
Masahiro Nakagawa wrote:
 
 I vote Andrei's suggestion, std.exception is better than new std.unittests.
 I think testing module should provide more features(e.g. Mock, Stub...).
 Your helpers help assert writing style but not help testing.
 In addition, std.exception already defined similar functions.

I do not like putting it in std.exception. Maybe the name std.unittest is also not good. I would propose std.assert if assert wasn't a keyword. When I use std.exception I want to handle situations that are part of the spec (i.e. exceptions) whereas Jonathan's module helps me writing asserts (that's most of the time unittests). Basically it helps me verifying behavior according to a spec. I want to keep the dichotomy of errors and exceptions. Putting both things in one module is rather strange to me. What are the arguments for putting it in std.exception? I find the size a rather weak argument. I thought about providing an assertDeath ones std.process is redone. And even though enforce and assert are mirroring each other they are used in different contexts. I would _not_ expect helpers for writing assertions (Assert_Error_) in a module named std.exception. Jens
Jan 30 2011
prev sibling next sibling parent Jonathan M Davis <jmdavisProg gmx.com> writes:
On Sunday 30 January 2011 04:13:59 Jens Mueller wrote:
 Masahiro Nakagawa wrote:
 I vote Andrei's suggestion, std.exception is better than new
 std.unittests. I think testing module should provide more features(e.g.
 Mock, Stub...). Your helpers help assert writing style but not help
 testing.
 In addition, std.exception already defined similar functions.

I do not like putting it in std.exception. Maybe the name std.unittest is also not good. I would propose std.assert if assert wasn't a keyword. When I use std.exception I want to handle situations that are part of the spec (i.e. exceptions) whereas Jonathan's module helps me writing asserts (that's most of the time unittests). Basically it helps me verifying behavior according to a spec. I want to keep the dichotomy of errors and exceptions. Putting both things in one module is rather strange to me. What are the arguments for putting it in std.exception? I find the size a rather weak argument. I thought about providing an assertDeath ones std.process is redone. And even though enforce and assert are mirroring each other they are used in different contexts. I would _not_ expect helpers for writing assertions (Assert_Error_) in a module named std.exception.

Would you expect assumeUnique or pointsTo to be in std.exception? I sure wouldn't, but for whatever reason, they're there. std.exception is a bit of a an odd module anyway. enforce and collectException are the only two which are exception related, and there aren't many functions in there at all. collectExceptionMsg, which is one of my proposed functions, definitely belongs in std.exception, since it's similar to collectException. The others are more debatable. They all throw AssertErrors on failure, and two of them - assertThrown and assertNotThrown - test for exceptions, but they aren't really exception-related overall. However, std.exception is by far the best fit of the currently existing modules, and enforce is essentially an assert that throws an exception instead of an AssertError, and it's in std.exception. So, yeah. It's a bit odd to stick them in std.exception, but std.exception is already rather odd, and if we don't want to create a new module, it's by far the best place to put them. And unless we forsee added a bunch of new unit testing functions to Phobos, creating a new std.unittests would mean having a pretty small module. Ignoring overloads, std.unittests would have only 4 functions in it. And if we put collectExceptionMsg in std.exception where it belongs, then std.unittests would have only 3 functions. And unless we really expect a bunch of new unit testing functions, that really doesn't seem like a good idea to me. And virtually every unit testing function that I've come up with has been merged into assertPred. So, don't forsee there being much in the way of additions to std.unittests unless someone comes up with something drastically different which makes it into Phobos. So, while I don't think that std.exception is a great fit, I do think that it's a reasonable fit. If anything, I think that it would be better to come up with a better name for std.exception than it would be to stick my new functions in a separate module. But we _already_ rename std.contracts to std.exception, so that would mean renaming that module _again_, and I have no idea what a better name would be anyway. - Jonathan M Davis
Jan 30 2011
prev sibling next sibling parent Jens Mueller <jens.k.mueller gmx.de> writes:
Jonathan M Davis wrote:
 On Sunday 30 January 2011 04:13:59 Jens Mueller wrote:
 I do not like putting it in std.exception. Maybe the name std.unittest
 is also not good. I would propose std.assert if assert wasn't a keyword.
 When I use std.exception I want to handle situations that are part of
 the spec (i.e. exceptions) whereas Jonathan's module helps me writing
 asserts (that's most of the time unittests).
 Basically it helps me verifying behavior according to a spec. I want to
 keep the dichotomy of errors and exceptions. Putting both things in one
 module is rather strange to me. What are the arguments for putting it in
 std.exception? I find the size a rather weak argument. I thought about
 providing an assertDeath ones std.process is redone.
 And even though enforce and assert are mirroring each other they are
 used in different contexts. I would _not_ expect helpers for writing
 assertions (Assert_Error_) in a module named std.exception.

Would you expect assumeUnique or pointsTo to be in std.exception? I sure wouldn't, but for whatever reason, they're there. std.exception is a bit of a an odd module anyway. enforce and collectException are the only two which are exception related, and there aren't many functions in there at all. collectExceptionMsg, which is one of my proposed functions, definitely belongs in std.exception, since it's similar to collectException. The others are more debatable. They all throw AssertErrors on failure, and two of them - assertThrown and assertNotThrown - test for exceptions, but they aren't really exception-related overall. However, std.exception is by far the best fit of the currently existing modules, and enforce is essentially an assert that throws an exception instead of an AssertError, and it's in std.exception. So, yeah. It's a bit odd to stick them in std.exception, but std.exception is already rather odd, and if we don't want to create a new module, it's by far the best place to put them. And unless we forsee added a bunch of new unit testing functions to Phobos, creating a new std.unittests would mean having a pretty small module. Ignoring overloads, std.unittests would have only 4 functions in it. And if we put collectExceptionMsg in std.exception where it belongs, then std.unittests would have only 3 functions. And unless we really expect a bunch of new unit testing functions, that really doesn't seem like a good idea to me. And virtually every unit testing function that I've come up with has been merged into assertPred. So, don't forsee there being much in the way of additions to std.unittests unless someone comes up with something drastically different which makes it into Phobos. So, while I don't think that std.exception is a great fit, I do think that it's a reasonable fit. If anything, I think that it would be better to come up with a better name for std.exception than it would be to stick my new functions in a separate module. But we _already_ rename std.contracts to std.exception, so that would mean renaming that module _again_, and I have no idea what a better name would be anyway.

My preference for distinct modules follows that line of separating errors and exceptions. The only argument against putting in its own module is it's size. That seems to be your main point. I think putting something in new module should mainly be concerned with separating stuff logically. Later on it should be easy to add a new module that imports std.exception and let's say std.assertion. But all you say is valid. I just fear that fixing names later will be costly. Having two modules allows us to handle this better. Just because std.exception is already kind of messy doesn't justify putting in there. Jens
Jan 30 2011
prev sibling next sibling parent reply SHOO <zan77137 nifty.com> writes:
(2011/01/24 23:34), Jonathan M Davis wrote:
 In case you didn't know, I have a set of unit test helper functions which have
 been being reviewed for possible inclusion in phobos. Here's an update.

 Most recent code: http://is.gd/F1OHat

 Okay. I took the previous suggestions into consideration and adjusted the code
a
 bit more. However, most of the changes are to the documentation (though there
 are some changes to the code). Some of the code duplication was removed, and
the
 way that some of the assertPred functions' errors are formatted has been
altered
 so that values line up vertically, making them easier to compare. The big
change
 is the docs though. There's now a fake version of assertPred at the top with an
 overall description for assertPred followed by the individual versions with as
 little documentation as seemed appropriate while still getting all of the
 necessary information across. A couple of the functions still have irritatingly
 long example sections, but anything less wouldn't get the functionality across.

 In any case. Here's the updated code. Review away. Andrei set the vote deadline
 for February 7th, at which point, if it passes majority vote, then it will go
 into Phobos. The number of functions is small enough now (thanks to having
 consolidated most of them into the fantastically versatile assertPred) that it
 looks like it will likely go in std.exception if the vote passes rather than
 becoming a new module. So, the std.unittests title has now become a bit of a
 misnomer, but that's what I've been calling it, so it seemed appropriate to
 continue to label it that way in the thread's title.

 - Jonathan M Davis

To be frank, I don't think that such a helper is necessary. I think these helpers will harm intuitive readability of unittest code. For unittest code, it is necessary to be able to understand easily even if without the document.
Jan 30 2011
parent reply Don <nospam nospam.com> writes:
Jonathan M Davis wrote:
 On Sunday 30 January 2011 05:28:36 SHOO wrote:

 To be frank, I don't think that such a helper is necessary.
 I think these helpers will harm intuitive readability of unittest code.
 For unittest code, it is necessary to be able to understand easily even
 if without the document.

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.
Feb 01 2011
next sibling parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 2/1/11 9:21 AM, Don wrote:
 Jonathan M Davis wrote:
 On Sunday 30 January 2011 05:28:36 SHOO wrote:

 To be frank, I don't think that such a helper is necessary.
 I think these helpers will harm intuitive readability of unittest code.
 For unittest code, it is necessary to be able to understand easily even
 if without the document.

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? Andrei
Feb 01 2011
parent Michel Fortin <michel.fortin michelf.com> writes:
On 2011-02-01 10:34:26 -0500, Andrei Alexandrescu 
<SeeWebsiteForEmail 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. -- Michel Fortin michel.fortin michelf.com http://michelf.com/
Feb 01 2011
prev sibling next sibling parent Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 2/1/11 9:21 AM, Don wrote:
 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.

I think this is also an argument in favor of making containers straight classes. Andrei
Feb 01 2011
prev sibling next sibling parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 2/1/11 9:21 AM, Don wrote:
 Jonathan M Davis wrote:
 On Sunday 30 January 2011 05:28:36 SHOO wrote:

 To be frank, I don't think that such a helper is necessary.
 I think these helpers will harm intuitive readability of unittest code.
 For unittest code, it is necessary to be able to understand easily even
 if without the document.

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.

One more thought. Don, you are in the unique position to (a) be strongly opinionated about this and (b) have insider knowledge about the compiler. You could, therefore, modify the definition of assert to automatically rewrite failed calls of the form assert(expr == expr), assert(expr < expr) etc. etc. as discussed in this thread. The rewritten forms would give the runtime the compared values to do whatever with. There is the sensitive issue of converting values of arbitrary types to strings at the runtime level, but we may be able to find a good solution to that. Andrei
Feb 01 2011
parent reply Michel Fortin <michel.fortin michelf.com> writes:
On 2011-02-01 11:31:54 -0500, Andrei Alexandrescu 
<SeeWebsiteForEmail erdani.org> said:

 On 2/1/11 9:21 AM, Don wrote:
 Jonathan M Davis wrote:
 On Sunday 30 January 2011 05:28:36 SHOO wrote:

 To be frank, I don't think that such a helper is necessary.
 I think these helpers will harm intuitive readability of unittest code.
 For unittest code, it is necessary to be able to understand easily even
 if without the document.

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.

One more thought. Don, you are in the unique position to (a) be strongly opinionated about this and (b) have insider knowledge about the compiler. You could, therefore, modify the definition of assert to automatically rewrite failed calls of the form assert(expr == expr), assert(expr < expr) etc. etc. as discussed in this thread. The rewritten forms would give the runtime the compared values to do whatever with. There is the sensitive issue of converting values of arbitrary types to strings at the runtime level, but we may be able to find a good solution to that.

TypeInfo holds a pointer to the toString function, so if the compiler passes the two operands as D-style variadic arguments to the assert handler, the assert handler can use toString to print them. The operator should be passed as a string. -- Michel Fortin michel.fortin michelf.com http://michelf.com/
Feb 01 2011
parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 2/1/11 10:51 AM, Michel Fortin wrote:
 On 2011-02-01 11:31:54 -0500, Andrei Alexandrescu
 <SeeWebsiteForEmail erdani.org> said:
 TypeInfo holds a pointer to the toString function, so if the compiler
 passes the two operands as D-style variadic arguments to the assert
 handler, the assert handler can use toString to print them. The operator
 should be passed as a string.

In that case problem solved. Don, if you arrange things such that this user-level code: int a = 42; double b = 3.14; assert(a <= b, "Something odd happened"); ultimately calls this runtime function: assertCmpFailed("<=", "42", "3.14", "Something odd happened"); I promise I'll discuss with Sean and implement what it takes in druntime to get that completed. We need to finalize that before Feb 7 though because on that date the vote for Jonathan's library closes. If you do implement that, probably we'll need to reject the library in the current form and propose back an amended version. Andrei
Feb 01 2011
next sibling parent bearophile <bearophileHUGS lycos.com> writes:
Andrei:

 Don, if you arrange things such that this user-level code:
 
 int a = 42;
 double b = 3.14;
 assert(a <= b, "Something odd happened");
 
 ultimately calls this runtime function:
 
 assertCmpFailed("<=", "42", "3.14", "Something odd happened");
 
 I promise I'll discuss with Sean and implement what it takes in druntime 
 to get that completed.

This makes me happier. Bye, bearophile
Feb 01 2011
prev sibling next sibling parent Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 2/1/11 11:29 AM, Jonathan M Davis wrote:
 On Tuesday 01 February 2011 09:12:16 Andrei Alexandrescu wrote:
 On 2/1/11 10:51 AM, Michel Fortin wrote:
 On 2011-02-01 11:31:54 -0500, Andrei Alexandrescu
 <SeeWebsiteForEmail erdani.org>  said:
 TypeInfo holds a pointer to the toString function, so if the compiler
 passes the two operands as D-style variadic arguments to the assert
 handler, the assert handler can use toString to print them. The operator
 should be passed as a string.

In that case problem solved. Don, if you arrange things such that this user-level code: int a = 42; double b = 3.14; assert(a<= b, "Something odd happened"); ultimately calls this runtime function: assertCmpFailed("<=", "42", "3.14", "Something odd happened"); I promise I'll discuss with Sean and implement what it takes in druntime to get that completed. We need to finalize that before Feb 7 though because on that date the vote for Jonathan's library closes. If you do implement that, probably we'll need to reject the library in the current form and propose back an amended version.

You do need to remember to take into account though that expressions can be quite a bit more complex than a<= b, and you still want to be able to print out something useful. It's been discussed a bit already, but IIRC what appeared to be the best solution was that when an expression evaluated to false, you take the expression to evaluated and stop evaluating it when all that was left is operators or functions which resulted in bool. Then the value of those operands would be printed. e.g. assert(min(5 + 2, 4)< 2&& max(5, 7)< 10); would print out the values 4, 2, 7, and 10.

It's all about the top-level AST node. In this case I think it's reasonable to print "false && __unevaluated__" or simply "false". If the user wants the separate values, they can always write: assert(min(5 + 2, 4) < 2); assert(max(5, 7) < 10);
 and

 assert(canFind("hello world", "goodbye"));

 would print out the values

 "hello world" and "goodbye".

I guess again taking the function call as the top node, assert could print the arguments passed into the function. Andrei
Feb 01 2011
prev sibling parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 2/4/11 3:27 AM, Brad Roberts wrote:
 On 2/1/2011 9:12 AM, Andrei Alexandrescu wrote:
 On 2/1/11 10:51 AM, Michel Fortin wrote:
 On 2011-02-01 11:31:54 -0500, Andrei Alexandrescu
 <SeeWebsiteForEmail erdani.org>  said:
 TypeInfo holds a pointer to the toString function, so if the compiler
 passes the two operands as D-style variadic arguments to the assert
 handler, the assert handler can use toString to print them. The operator
 should be passed as a string.

In that case problem solved. Don, if you arrange things such that this user-level code: int a = 42; double b = 3.14; assert(a<= b, "Something odd happened"); ultimately calls this runtime function: assertCmpFailed("<=", "42", "3.14", "Something odd happened"); I promise I'll discuss with Sean and implement what it takes in druntime to get that completed. We need to finalize that before Feb 7 though because on that date the vote for Jonathan's library closes. If you do implement that, probably we'll need to reject the library in the current form and propose back an amended version. Andrei

I agree with Don. The proposed set of functions are great from an output perspective, but awful from a crisp syntax standpoint. They're a work around for the underlying problem: assert needs a UI face lift (though I remember when assert didn't even support a text output -- adding that was my first contribution to the compiler/language). There are certainly complexities in pulling apart the bool expression though, and I'm not sure what a good solution is for that. If there's any sort of consensus around focusing on assert instead (and I recognize that there isn't yet), some pre-set arbitrary deadline isn't terribly relevant. Having one by the 7th is kinda unlikely. As a further point against the assertPred!op model, consider why the use of postfix/rpn calculators is restricted to a very small subset of advanced calculators. It's just now how most people think about expressions. Later, Brad

We need to have at least a definite decision by Feb 7 from Don and Walter that work will be put or not into improving assert as discussed. Andrei
Feb 04 2011
parent reply bearophile <bearophileHUGS lycos.com> writes:
Andrei:

 We need to have at least a definite decision by Feb 7 from Don and 
 Walter that work will be put or not into improving assert as discussed.

I think that fixing assert() is a better strategy, even it may require more work. A problem with assert is that it wants to do too many things, so there are problems: - assert(0); to mean HALT, that gives some problems - assert(class_instance); that calls its invariant. (I vaguely remember a problem with this with struct instance invariants). - Not good enough for unittests, as seen in this thread. So an effort to improve assert may solve all four those problems. Removing the two purposes (moving them to specialized and explicit features, specialized little tools) and adding the functionality we desire in unittesting. Bye, bearophile
Feb 04 2011
next sibling parent reply bearophile <bearophileHUGS lycos.com> writes:
 Removing the two purposes (moving them to specialized and explicit features,
specialized little tools)

I want to stress the fact that moving those two unrelated sub-features away from assert into specialized features is not going to increase the language complexity, because those features are _already_ present and programmers need to remember them already. But currently they are hidden, half-invisible, and they interact badly with other things. So moving them into explicit little features makes the language more explicit, more readable and helps to remove warts, it doesn't make the language more complex (it may just increase front-end compiler complexity a bit). Bye, bearophile
Feb 04 2011
parent reply bearophile <bearophileHUGS lycos.com> writes:
 So moving them into explicit little features makes the language more

The two little features I am talking about are: - a halt() somewhere like in core.bitop or std.intrinsic or the default object module to replace assert(0); - And to use class_instance.invariant() to replace assert(class_instance). Bye, bearophile
Feb 04 2011
parent Daniel Gibson <metalcaedes gmail.com> writes:
Am 04.02.2011 15:57, schrieb spir:
 On 02/04/2011 01:40 PM, bearophile wrote:
 So moving them into explicit little features makes the language more

The two little features I am talking about are: - a halt() somewhere like in core.bitop or std.intrinsic or the default object module to replace assert(0);

What about exit()? What would be the practical or semantic diff of halt() compared to exit()? Denis

IIRC halt should produce a core-dump, so you can use a debugger to see what went wrong afterwards. Cheers, - Daniel
Feb 04 2011
prev sibling next sibling parent bearophile <bearophileHUGS lycos.com> writes:
Jonathan M Davis:

 However, I don't see why there would be any problem with 
 assert(0) meaning halt. It's a normal assert in non-release mode and it sticks 
 around in release mode, becoming a halt instruction. I use it that way all the 
 time. I don't see any problem with it whatsoever.

There was a recent discussion about this. The final consensus was that assert(0) to mean HALT is a small wart in the D language, but it's not worth fixing it, because it's a small enough defect. But if now assert() gets a significant facelift, then this little problem may be fixed along with the other two. Bye, bearophile
Feb 04 2011
prev sibling parent reply Adam Ruppe <destructionator gmail.com> writes:
bearophile wrote:
 - assert(0); to mean HALT, that gives some problems

I like it. It's convenient and clear - assert(0) means you cannot proceed in any situation, and that should include release modes. (Indeed, IMO any assert that can be proven to fail at compile time should always remain and one proven to succeed can always be removed. Only if the compiler isn't sure does it actually do a runtime check. -release simply says "if not sure, trust me and don't bother" instead of the default of "if not sure, check for me". I think the compiler pretty much does this already.)
 - assert(class_instance); that calls its invariant. (I vaguely
 remember a problem with this with struct instance invariants).

Again, I think this makes sense too. assert(n) is saying, in principle, "ensure n is valid". Calling the invariant is part of ensuring it is valid. The only problem is a minor bug - it forgets to check for null before calling the invariant. That should, of course, be fixed, but the main behavior makes sense.
Feb 04 2011
next sibling parent reply bearophile <bearophileHUGS lycos.com> writes:
Jonathan M Davis:

 assert(0) has the advantage of being a normal assertion in non-release mode.

What is this useful for? To me this looks like a significant disadvantage. If I want a HALT (to tell the compiler a point can't be reached, etc) I want it in every kind of compilation of the program.
 It also makes it clear that that code path should _never_ be reached.

The replacement for assert(0) is meant to be more clear in its purpose compared to assert(0). It may be named thisCantHappen(), or assertZero(), etc.
 The real question though is whether you can convince Walter (which I doubt,
but I don't know).

This topic was already discussed, and I think the result of the discussion was that this change of assert(false) is not worth it. But if asserts gets inproved for other purposes, then this is a chance to work on improving assert(0) too.
 Still, making such a change _would_ contradict TDPL, which is supposed to be a
major no-no at this point.<

I like TDPL, I respect Andrei and you, I agree that TDPL is a kind of reference for D2, but please stop using TDPL as a The Bible in many of your posts. Not even Andrei himself looks so religiously attached as you to the contents of TDPL :-) A little flexibility is acceptable. -------------------------- Adam Ruppe:
Again, I think this makes sense too. assert(n) is saying, in principle, "ensure
n is valid". Calling the invariant is part of ensuring it is valid. The only
problem is a minor bug - it forgets to check for null before calling the
invariant. That should, of course, be fixed, but the main behavior makes sense.<

Something like class_instance.invariant() is better because: - It's explicit and readable. While you need to read the D manual to know that assert(class_instance) tests the invariant too, and not just that class_instance reference is not null. I remember other persons in D.learn not knowning/suprised by that assert(class_instance) tests the invariant too. - It removes a special case, and special cases kill languages. Struct instances too allow an invariant, but they are not callable with assert(), see below. A syntax like structlass_instance.invariant() is shared with the class one. struct Foo { int x; invariant() { assert(x == 1); } } void main() { Foo f; assert(f); } DMD 2.051: test.d(7): Error: expression f of type Foo does not have a boolean value Bye, bearophile
Feb 04 2011
next sibling parent Adam D. Ruppe <destructionator gmail.com> writes:
bearophile wrote:
 What is [normal assert exceptions] useful for?

Getting a more helpful error message and stack trace.
 If I want a HALT (to tell the compiler a point can't be reached,
 etc) I want it in every kind of compilation of the program.

assert(0) does just that. Either way, the program stops.
 The replacement for assert(0) is meant to be more clear in its
 purpose compared to assert(0). It may be named [...] assertZero(), etc.

LOL! But, assert(0) does exactly what it says - assert this situation is invariably invalid.
 Something like class_instance.invariant() is better because:
 - It's explicit and readable.

So is assert(obj); If the null check bug wasn't there, I doubt anyone would complain about the invariant. Hell, people who didn't read the manual would probably have no idea it even did it! The invariant is, well, invariant. A method isn't allowed to return without checking it, meaning the only reference where it might fail is assert(this). (Assuming the null bug was fixed.) assert(this) inside a method doesn't make too much sense for null checks anyway. this is rarely null.
 - It removes a special case, and special cases kill languages.

This isn't really special. You're asserting the object is valid, which includes the invariant. If you want to only assert it is not null, you should write assert(obj !is null); Also, if you change to obj.invariant(), it will probably never be used. assert() is your one-stop shop for sanity tests.
 Struct instances too allow an invariant, but they are not
 callable with assert(), see below.

IMO that's the bug. It'd make a lot more sense to fix it so assert(struct) checks the invariant than to break assert(class) so it doesn't.
Feb 04 2011
prev sibling parent "Nick Sabalausky" <a a.a> writes:
"bearophile" <bearophileHUGS lycos.com> wrote in message 
news:iihr42$rqp$1 digitalmars.com...
 The replacement for assert(0) is meant to be more clear in its purpose 
 compared to assert(0). It may be named thisCantHappen(), or assertZero(), 
 etc.

I vote for "fubar();" :) ...or "fellOffTheEdgeOfTheWorld();" ..."fubar" is shorter though.
Feb 05 2011
prev sibling parent spir <denis.spir gmail.com> writes:
On 02/05/2011 08:29 AM, Jonathan M Davis wrote:
 On Friday 04 February 2011 13:29:38 bearophile wrote:
 Jonathan M Davis:
 assert(0) has the advantage of being a normal assertion in non-release
 mode.

What is this useful for? To me this looks like a significant disadvantage. If I want a HALT (to tell the compiler a point can't be reached, etc) I want it in every kind of compilation of the program.
 It also makes it clear that that code path should _never_ be reached.

The replacement for assert(0) is meant to be more clear in its purpose compared to assert(0). It may be named thisCantHappen(), or assertZero(), etc.

assert(0) will actually give a stack trace with a file and line number. It will also give you a message if you include one with it. HALT just kills the program. I _much_ prefer that assert(0) be a normal assert in no-release mode. Leaving it in as a HALT has the advantage that the program will just die if it reaches that point in release mode rather than trying to continue with the assert gone, but I very much want a normal assert in non-release mode. It's much more useful.
 The real question though is whether you can convince Walter (which I
 doubt, but I don't know).

This topic was already discussed, and I think the result of the discussion was that this change of assert(false) is not worth it. But if asserts gets inproved for other purposes, then this is a chance to work on improving assert(0) too.
 Still, making such a change _would_ contradict TDPL, which is supposed to
 be a major no-no at this point.<

I like TDPL, I respect Andrei and you, I agree that TDPL is a kind of reference for D2, but please stop using TDPL as a The Bible in many of your posts. Not even Andrei himself looks so religiously attached as you to the contents of TDPL :-) A little flexibility is acceptable.

I believe that Walter and Andrei have made it fairly clear that if we do anything that contradicts TDPL, it needs to have a very good reason to be done. TDPL is _supposed_ to have been the final word. Unfortunately, the implementation is behind, so _it_ is possible that we're going to have to make changes which contradict it. However, if we do, those changes have to be needed or at least really merit the cost of contradicting TDPL. Something as small as changing assert(0) is unlikely to do that.
 struct Foo {
      int x;
      invariant() { assert(x == 1); }
 }
 void main() {
      Foo f;
      assert(f);
 }


 DMD 2.051:
 test.d(7): Error: expression f of type Foo does not have a boolean value

Actually, the more I think about it, the less I see assert(class_instance) to be a problem. Normally it would check that the reference was non-null. Assuming that the feature isn't buggy, it'll still do that, but it'll check the invariant in addition. And since the invariant is always supposed to be true, that shouldn't be a problem. I really don't think that assert needs to be fundamentally changed with regards to assert(0) or assert(class_instance). - Jonathan M Davis

All right, I guess I get your point. I still think that checking a class's invariant should be explicit. Assert(whatever) means for me "check whatever is not (equivalent to) false", not "check whatever is not (equivalent to) false and whatever's invariant condition, if any, is fulfilled". Note that an object's invariant is *not* part of its regular truth value (1st assertion in unittest below indeed passes). Thus, it is clearly incorrect that assert(x) checks its invariant implicitely: assert ( x ); assert ( cast(bool)x == true ) ; should always have the same outcome. Thus, the idiom to check an object's invariant should be different, at least in a visible detail. A side-issue is the following code: class C { int i; invariant () { assert(i == 1); } } unittest { auto c = new C(); assert ( cast(bool)c == true ) ; assert ( c ) ; } throws a plain "Assertion error" with no additional comment. No mention of invariant checking, thus no hint to debug. I guess the problem would be more acceptable if asserts in invariants had at least slightly different error message forms, if only "Invariant assertion error". (Yes, one can customize error messages, sure; but defaults should be as helpful as possible.) Denis -- _________________ vita es estrany spir.wikidot.com
Feb 05 2011
prev sibling next sibling parent Brad Roberts <braddr puremagic.com> writes:
On 2/1/2011 9:12 AM, Andrei Alexandrescu wrote:
 On 2/1/11 10:51 AM, Michel Fortin wrote:
 On 2011-02-01 11:31:54 -0500, Andrei Alexandrescu
 <SeeWebsiteForEmail erdani.org> said:
 TypeInfo holds a pointer to the toString function, so if the compiler
 passes the two operands as D-style variadic arguments to the assert
 handler, the assert handler can use toString to print them. The operator
 should be passed as a string.

In that case problem solved. Don, if you arrange things such that this user-level code: int a = 42; double b = 3.14; assert(a <= b, "Something odd happened"); ultimately calls this runtime function: assertCmpFailed("<=", "42", "3.14", "Something odd happened"); I promise I'll discuss with Sean and implement what it takes in druntime to get that completed. We need to finalize that before Feb 7 though because on that date the vote for Jonathan's library closes. If you do implement that, probably we'll need to reject the library in the current form and propose back an amended version. Andrei

I agree with Don. The proposed set of functions are great from an output perspective, but awful from a crisp syntax standpoint. They're a work around for the underlying problem: assert needs a UI face lift (though I remember when assert didn't even support a text output -- adding that was my first contribution to the compiler/language). There are certainly complexities in pulling apart the bool expression though, and I'm not sure what a good solution is for that. If there's any sort of consensus around focusing on assert instead (and I recognize that there isn't yet), some pre-set arbitrary deadline isn't terribly relevant. Having one by the 7th is kinda unlikely. As a further point against the assertPred!op model, consider why the use of postfix/rpn calculators is restricted to a very small subset of advanced calculators. It's just now how most people think about expressions. Later, Brad
Feb 04 2011
prev sibling next sibling parent spir <denis.spir gmail.com> writes:
On 02/04/2011 01:16 PM, bearophile wrote:
 Removing the two purposes (moving them to specialized and explicit features,
specialized little tools)

I want to stress the fact that moving those two unrelated sub-features away from assert into specialized features is not going to increase the language complexity, because those features are _already_ present and programmers need to remember them already. But currently they are hidden, half-invisible, and they interact badly with other things. So moving them into explicit little features makes the language more explicit, more readable and helps to remove warts, it doesn't make the language more complex (it may just increase front-end compiler complexity a bit).

+++ Denis -- _________________ vita es estrany spir.wikidot.com
Feb 04 2011
prev sibling next sibling parent spir <denis.spir gmail.com> writes:
On 02/04/2011 01:40 PM, bearophile wrote:
 So moving them into explicit little features makes the language more

The two little features I am talking about are: - a halt() somewhere like in core.bitop or std.intrinsic or the default object module to replace assert(0);

What about exit()? What would be the practical or semantic diff of halt() compared to exit()? Denis -- _________________ vita es estrany spir.wikidot.com
Feb 04 2011
prev sibling parent spir <denis.spir gmail.com> writes:
On 02/04/2011 10:29 PM, bearophile wrote:
 Jonathan M Davis:

 assert(0) has the advantage of being a normal assertion in non-release mode.

What is this useful for? To me this looks like a significant disadvantage. If I want a HALT (to tell the compiler a point can't be reached, etc) I want it in every kind of compilation of the program.
 It also makes it clear that that code path should _never_ be reached.

The replacement for assert(0) is meant to be more clear in its purpose compared to assert(0). It may be named thisCantHappen(), or assertZero(), etc.
 The real question though is whether you can convince Walter (which I doubt,
but I don't know).

This topic was already discussed, and I think the result of the discussion was that this change of assert(false) is not worth it. But if asserts gets inproved for other purposes, then this is a chance to work on improving assert(0) too.
 Still, making such a change _would_ contradict TDPL, which is supposed to be a
major no-no at this point.<

I like TDPL, I respect Andrei and you, I agree that TDPL is a kind of reference for D2, but please stop using TDPL as a The Bible in many of your posts. Not even Andrei himself looks so religiously attached as you to the contents of TDPL :-) A little flexibility is acceptable.

agreed
 --------------------------

 Adam Ruppe:

 Again, I think this makes sense too. assert(n) is saying, in principle,
"ensure n is valid". Calling the invariant is part of ensuring it is valid. The
only problem is a minor bug - it forgets to check for null before calling the
invariant. That should, of course, be fixed, but the main behavior makes sense.<

Something like class_instance.invariant() is better because: - It's explicit and readable. While you need to read the D manual to know that assert(class_instance) tests the invariant too, and not just that class_instance reference is not null. I remember other persons in D.learn not knowning/suprised by that assert(class_instance) tests the invariant too. - It removes a special case, and special cases kill languages. Struct instances too allow an invariant, but they are not callable with assert(), see below. A syntax like structlass_instance.invariant() is shared with the class one. struct Foo { int x; invariant() { assert(x == 1); } } void main() { Foo f; assert(f); } DMD 2.051: test.d(7): Error: expression f of type Foo does not have a boolean value Bye, bearophile

ditto (implicit magic semantics is bad) Denis -- _________________ vita es estrany spir.wikidot.com
Feb 04 2011
prev sibling next sibling parent Jonathan M Davis <jmdavisProg gmx.com> writes:
On Sunday 30 January 2011 05:13:19 Jens Mueller wrote:
 Jonathan M Davis wrote:
 On Sunday 30 January 2011 04:13:59 Jens Mueller wrote:
 I do not like putting it in std.exception. Maybe the name std.unittest
 is also not good. I would propose std.assert if assert wasn't a
 keyword. When I use std.exception I want to handle situations that are
 part of the spec (i.e. exceptions) whereas Jonathan's module helps me
 writing asserts (that's most of the time unittests).
 Basically it helps me verifying behavior according to a spec. I want to
 keep the dichotomy of errors and exceptions. Putting both things in one
 module is rather strange to me. What are the arguments for putting it
 in std.exception? I find the size a rather weak argument. I thought
 about providing an assertDeath ones std.process is redone.
 And even though enforce and assert are mirroring each other they are
 used in different contexts. I would _not_ expect helpers for writing
 assertions (Assert_Error_) in a module named std.exception.

Would you expect assumeUnique or pointsTo to be in std.exception? I sure wouldn't, but for whatever reason, they're there. std.exception is a bit of a an odd module anyway. enforce and collectException are the only two which are exception related, and there aren't many functions in there at all. collectExceptionMsg, which is one of my proposed functions, definitely belongs in std.exception, since it's similar to collectException. The others are more debatable. They all throw AssertErrors on failure, and two of them - assertThrown and assertNotThrown - test for exceptions, but they aren't really exception-related overall. However, std.exception is by far the best fit of the currently existing modules, and enforce is essentially an assert that throws an exception instead of an AssertError, and it's in std.exception. So, yeah. It's a bit odd to stick them in std.exception, but std.exception is already rather odd, and if we don't want to create a new module, it's by far the best place to put them. And unless we forsee added a bunch of new unit testing functions to Phobos, creating a new std.unittests would mean having a pretty small module. Ignoring overloads, std.unittests would have only 4 functions in it. And if we put collectExceptionMsg in std.exception where it belongs, then std.unittests would have only 3 functions. And unless we really expect a bunch of new unit testing functions, that really doesn't seem like a good idea to me. And virtually every unit testing function that I've come up with has been merged into assertPred. So, don't forsee there being much in the way of additions to std.unittests unless someone comes up with something drastically different which makes it into Phobos. So, while I don't think that std.exception is a great fit, I do think that it's a reasonable fit. If anything, I think that it would be better to come up with a better name for std.exception than it would be to stick my new functions in a separate module. But we _already_ rename std.contracts to std.exception, so that would mean renaming that module _again_, and I have no idea what a better name would be anyway.

My preference for distinct modules follows that line of separating errors and exceptions. The only argument against putting in its own module is it's size. That seems to be your main point. I think putting something in new module should mainly be concerned with separating stuff logically. Later on it should be easy to add a new module that imports std.exception and let's say std.assertion. But all you say is valid. I just fear that fixing names later will be costly. Having two modules allows us to handle this better. Just because std.exception is already kind of messy doesn't justify putting in there.

I think that it makes perfect sense for the likes of assertPred and assertThrown to be in the same module as enforce. They serve similar functions. It's just that one throws an assertError on failure, whereas the other usually throws an Exception. What is odd is sticking them in a module named std.exception. But while assertPred might be similar to enforce, it definitely wouldn't make even less sense to put enforce in a module named std.unittests. I have no idea what a good name would be. So, yes, it's a bit odd that std.exception would have assertPred in it, but it's the best fit out of all of the pre-existing modules, and std.exception does have semi-related functions. And honestly, I _do_ think that the size of a module matters. If it doesn't have enough functions in it, then it really shouldn't be its own module. The problem is when you have miscellaneous functions which don't quite fit any any module - which may be why assumeUnique and pointsTo ended up in std.exception. - Jonathan M Davis
Jan 30 2011
prev sibling next sibling parent Jonathan M Davis <jmdavisProg gmx.com> writes:
On Sunday 30 January 2011 05:28:36 SHOO wrote:
 (2011/01/24 23:34), Jonathan M Davis wrote:
 In case you didn't know, I have a set of unit test helper functions which
 have been being reviewed for possible inclusion in phobos. Here's an
 update.
 
 Most recent code: http://is.gd/F1OHat
 
 Okay. I took the previous suggestions into consideration and adjusted the
 code a bit more. However, most of the changes are to the documentation
 (though there are some changes to the code). Some of the code
 duplication was removed, and the way that some of the assertPred
 functions' errors are formatted has been altered so that values line up
 vertically, making them easier to compare. The big change is the docs
 though. There's now a fake version of assertPred at the top with an
 overall description for assertPred followed by the individual versions
 with as little documentation as seemed appropriate while still getting
 all of the necessary information across. A couple of the functions still
 have irritatingly long example sections, but anything less wouldn't get
 the functionality across.
 
 In any case. Here's the updated code. Review away. Andrei set the vote
 deadline for February 7th, at which point, if it passes majority vote,
 then it will go into Phobos. The number of functions is small enough now
 (thanks to having consolidated most of them into the fantastically
 versatile assertPred) that it looks like it will likely go in
 std.exception if the vote passes rather than becoming a new module. So,
 the std.unittests title has now become a bit of a misnomer, but that's
 what I've been calling it, so it seemed appropriate to continue to label
 it that way in the thread's title.
 
 - Jonathan M Davis

To be frank, I don't think that such a helper is necessary. I think these helpers will harm intuitive readability of unittest code. For unittest code, it is necessary to be able to understand easily even if without the document.

Do you really find assertPred!"=="(min(5, 7), 5); to be all that harder to understand than assert(min(5, 7) == 5); I find that assertPred results in code which is quite easy to read. And the functions assertThrown and assertNotThrown are error-prone to reimplement, and definitely cut down on boiler plate code if you use them to test that functions properly throw or don't throw exceptions. I fail to see how these functions would make unit testing code harder to read. Every unit testing framework that I've ever used (aside from what's built in to D) has had helper functions to do what it does. And they result in much better error messages than assert gives you. 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. Worst case, you have to edit code where someone else used them. Regardless, I really don't think that they hard code understandability at all. - Jonathan M Davis
Jan 30 2011
prev sibling next sibling parent Jens Mueller <jens.k.mueller gmx.de> writes:
Jonathan M Davis wrote:
 On Sunday 30 January 2011 05:13:19 Jens Mueller wrote:
 
 My preference for distinct modules follows that line of separating
 errors and exceptions.
 The only argument against putting in its own module is it's size. That
 seems to be your main point. I think putting something in new module
 should mainly be concerned with separating stuff logically. Later on it
 should be easy to add a new module that imports std.exception and let's
 say std.assertion.
 But all you say is valid. I just fear that fixing names later will be
 costly. Having two modules allows us to handle this better. Just because
 std.exception is already kind of messy doesn't justify putting in
 there.

I think that it makes perfect sense for the likes of assertPred and assertThrown to be in the same module as enforce. They serve similar functions. It's just that one throws an assertError on failure, whereas the other usually throws an Exception.

Somehow I'm a bit off here. Because assert throws an error and enforce an exception make them not similar for me. I mean that's the whole point of distinguishing error and exceptions.
 What is odd is sticking them in a module named std.exception. But while 
 assertPred might be similar to enforce, it definitely wouldn't make even less 
 sense to put enforce in a module named std.unittests. I have no idea what a
good 
 name would be.

That's true. Since we do not have a good name for assert and enforce together I wouldn't put them in the same module. I suppose there is a reason why we are having difficulties finding a name.
 So, yes, it's a bit odd that std.exception would have assertPred in it, but
it's 
 the best fit out of all of the pre-existing modules, and std.exception does
have 
 semi-related functions. And honestly, I _do_ think that the size of a module 
 matters. If it doesn't have enough functions in it, then it really shouldn't
be 
 its own module. The problem is when you have miscellaneous functions which
don't 
 quite fit any any module - which may be why assumeUnique and pointsTo ended up
in 
 std.exception.

For me it's first logical separation. This way I never end up with functions in a module that do not belong there. But of course I may end up with many modules. So I may end up with the same problem now with modules. So I'm completely fine with hierarchies a la Java (maybe less deep). I'll guess you're not. I think of these modules as hierarchies as in a file system. Because it's very for me to navigate in a file system. No need to reply. I think you have good arguments that are valid. And in the very end it often boils down to a matter of style. It's very difficult to argue about style. Jens
Jan 30 2011
prev sibling next sibling parent Jonathan M Davis <jmdavisProg gmx.com> writes:
On Sunday 30 January 2011 06:10:25 Jens Mueller wrote:
 Jonathan M Davis wrote:
 On Sunday 30 January 2011 05:13:19 Jens Mueller wrote:
 My preference for distinct modules follows that line of separating
 errors and exceptions.
 The only argument against putting in its own module is it's size. That
 seems to be your main point. I think putting something in new module
 should mainly be concerned with separating stuff logically. Later on it
 should be easy to add a new module that imports std.exception and let's
 say std.assertion.
 But all you say is valid. I just fear that fixing names later will be
 costly. Having two modules allows us to handle this better. Just
 because std.exception is already kind of messy doesn't justify putting
 in there.

I think that it makes perfect sense for the likes of assertPred and assertThrown to be in the same module as enforce. They serve similar functions. It's just that one throws an assertError on failure, whereas the other usually throws an Exception.

Somehow I'm a bit off here. Because assert throws an error and enforce an exception make them not similar for me. I mean that's the whole point of distinguishing error and exceptions.
 What is odd is sticking them in a module named std.exception. But while
 assertPred might be similar to enforce, it definitely wouldn't make even
 less sense to put enforce in a module named std.unittests. I have no
 idea what a good name would be.

That's true. Since we do not have a good name for assert and enforce together I wouldn't put them in the same module. I suppose there is a reason why we are having difficulties finding a name.

I guess that I just don't think of Exceptions and Errors as being all that different. They're both Throwables after all. True, you don't use them exactly the same way, but they're still very similar in purpose.
 So, yes, it's a bit odd that std.exception would have assertPred in it,
 but it's the best fit out of all of the pre-existing modules, and
 std.exception does have semi-related functions. And honestly, I _do_
 think that the size of a module matters. If it doesn't have enough
 functions in it, then it really shouldn't be its own module. The problem
 is when you have miscellaneous functions which don't quite fit any any
 module - which may be why assumeUnique and pointsTo ended up in
 std.exception.

For me it's first logical separation. This way I never end up with functions in a module that do not belong there. But of course I may end up with many modules. So I may end up with the same problem now with modules. So I'm completely fine with hierarchies a la Java (maybe less deep). I'll guess you're not. I think of these modules as hierarchies as in a file system. Because it's very for me to navigate in a file system. No need to reply. I think you have good arguments that are valid. And in the very end it often boils down to a matter of style. It's very difficult to argue about style.

A lot of stuff comes down to preference and style, which is part of the problem. Far too often, you can't argue objectively about stuff like that, because too much of it is objective. Personally, I wouldn't mind a hierarchy of modules in Phobos, and it wouldn't surprise me if we end up with one once we get enough modules, since tons of modules in a flat hierarchy is a bit unwieldy. However, I still don't like the idea of having a module with only 3 or 4 functions in it, which is the problem here. - Jonathan M Davis
Jan 30 2011
prev sibling next sibling parent Jens Mueller <jens.k.mueller gmx.de> writes:
Andrei Alexandrescu wrote:
 On 01/30/2011 06:13 AM, Jens Mueller wrote:
Masahiro Nakagawa wrote:
I vote Andrei's suggestion, std.exception is better than new std.unittests.
I think testing module should provide more features(e.g. Mock, Stub...).
Your helpers help assert writing style but not help testing.
In addition, std.exception already defined similar functions.

I do not like putting it in std.exception. Maybe the name std.unittest is also not good. I would propose std.assert if assert wasn't a keyword. When I use std.exception I want to handle situations that are part of the spec (i.e. exceptions) whereas Jonathan's module helps me writing asserts (that's most of the time unittests). Basically it helps me verifying behavior according to a spec. I want to keep the dichotomy of errors and exceptions. Putting both things in one module is rather strange to me. What are the arguments for putting it in std.exception? I find the size a rather weak argument. I thought about providing an assertDeath ones std.process is redone. And even though enforce and assert are mirroring each other they are used in different contexts. I would _not_ expect helpers for writing assertions (Assert_Error_) in a module named std.exception. Jens

assertThrows and its converse are a good fit for std.exception. Then we're left with a couple of concepts that don't deserve their own module and are difficult to fit elsewhere. I reckon that in a perfect world there would be a better match, but I don't cringe at the thought of std.exception holding them.

I going to get used to it. I'll guess I just don't want to loose the concept of errors vs. exceptions. Jens
Jan 31 2011
prev sibling next sibling parent Jens Mueller <jens.k.mueller gmx.de> writes:
spir wrote:
 On 01/30/2011 01:13 PM, Jens Mueller wrote:
I do not like putting it in std.exception. Maybe the name std.unittest
is also not good. I would propose std.assert if assert wasn't a keyword.
[...]

assertions (Assert_Error_) in a module named std.exception.

Same for me. Find it strange. Would never search assertion helper funcs inside std.exception. Why not std.assertion? std.unittests would be fine if there were some more stuff in there, I mean not only assertions. Else, the obvious name imo is std.assertion.

Nice. I just wonder what others think. I'd like to start a poll http://doodle.com/vn2ceenuvfwtx38e In general are those polls useful? I mean there where some discussions in the last time where a poll may help. git vs. hg. 80 vs. 90 characters per line. If all arguments are on the table it can be useful to have an opinion poll to finally settle the discussion. It may even be that I'm totally wrong here. But I think the module naming needs to be done very careful. It's what a D newcomer needs to grasp as easy as possible. Am I too picky? Somehow I care (too?) much about names. Jens
Jan 31 2011
prev sibling next sibling parent Jonathan M Davis <jmdavisProg gmx.com> writes:
On Monday 31 January 2011 15:49:11 Jens Mueller wrote:
 spir wrote:
 On 01/30/2011 01:13 PM, Jens Mueller wrote:
I do not like putting it in std.exception. Maybe the name std.unittest
is also not good. I would propose std.assert if assert wasn't a keyword.
[...]

assertions (Assert_Error_) in a module named std.exception.

Same for me. Find it strange. Would never search assertion helper funcs inside std.exception. Why not std.assertion? std.unittests would be fine if there were some more stuff in there, I mean not only assertions. Else, the obvious name imo is std.assertion.

Nice. I just wonder what others think. I'd like to start a poll http://doodle.com/vn2ceenuvfwtx38e In general are those polls useful? I mean there where some discussions in the last time where a poll may help. git vs. hg. 80 vs. 90 characters per line. If all arguments are on the table it can be useful to have an opinion poll to finally settle the discussion. It may even be that I'm totally wrong here. But I think the module naming needs to be done very careful. It's what a D newcomer needs to grasp as easy as possible. Am I too picky? Somehow I care (too?) much about names.

I wouldn't actually be looking for "assertion helpers" anywhere. I might be looking for unit test helper, but I wouldn't be thinking about assertions at all, even if the unit test helpers threw AssertError like they do. But truth be told, I don't generally look for modules with a particular name unless I know what I'm looking for. I look at the list of modules and see which names seem like they'd have what I'd want. As such, std.unittests would make me think that the module held unit test stuff, whereas I really wouldn't know what to expect in std.assertion or std.exception at all - _especially_ std.assertion. However, given the small number of unit test functions, it makes no sense for them to be their own module. So, I really don't think that std.unittests makes sense at this point. std.exception may not be the best name, but given what's currently in it, the unit testing functions fit in there reasonably well. So, the issue then is what to name std.exception if that name is not good enough. std.assertion makes no sense - particularly with enforce in there - and std.unittests doesn't work, because it's not just unit testing stuff in there. So, unless you can come up with a better name for std.exception with the idea that it's going to be holding my new functions along with what's already in there, I don't think that discussing names is going to mean much. From a contents standpoint, it does make sense that the stuff in std.exception and my stuff would be lumped together, and it does not make sense for my stuff to be in its own module at this point. There isn't enough of it, and since it fits well in std.exception, there's even less reason to create a new module for it. So, if a change is to be made, I think that it's going to have to be to the name of std.exception, and I can't think of a better name nor have I see a better name suggested. - Jonathan M Davis
Feb 01 2011
prev sibling next sibling parent Jens Mueller <jens.k.mueller gmx.de> writes:
Jonathan M Davis wrote:
 On Monday 31 January 2011 15:49:11 Jens Mueller wrote:
 spir wrote:
 On 01/30/2011 01:13 PM, Jens Mueller wrote:
I do not like putting it in std.exception. Maybe the name std.unittest
is also not good. I would propose std.assert if assert wasn't a keyword.
[...]

assertions (Assert_Error_) in a module named std.exception.

Same for me. Find it strange. Would never search assertion helper funcs inside std.exception. Why not std.assertion? std.unittests would be fine if there were some more stuff in there, I mean not only assertions. Else, the obvious name imo is std.assertion.

Nice. I just wonder what others think. I'd like to start a poll http://doodle.com/vn2ceenuvfwtx38e In general are those polls useful? I mean there where some discussions in the last time where a poll may help. git vs. hg. 80 vs. 90 characters per line. If all arguments are on the table it can be useful to have an opinion poll to finally settle the discussion. It may even be that I'm totally wrong here. But I think the module naming needs to be done very careful. It's what a D newcomer needs to grasp as easy as possible. Am I too picky? Somehow I care (too?) much about names.

I wouldn't actually be looking for "assertion helpers" anywhere. I might be looking for unit test helper, but I wouldn't be thinking about assertions at all, even if the unit test helpers threw AssertError like they do. But truth be told, I don't generally look for modules with a particular name unless I know what I'm looking for. I look at the list of modules and see which names seem like they'd have what I'd want. As such, std.unittests would make me think that the module held unit test stuff, whereas I really wouldn't know what to expect in std.assertion or std.exception at all - _especially_ std.assertion.

When I hear std.assertion and std.exception then I assume that these modules contain stuff for working with assertions and exceptions respectively. Just the fact that I want to throw an exception makes me go to std.exception to check out what's in there.
 However, given the small number of unit test functions, it makes no sense for 
 them to be their own module. So, I really don't think that std.unittests makes 
 sense at this point. std.exception may not be the best name, but given what's 
 currently in it, the unit testing functions fit in there reasonably well. So,
the 
 issue then is what to name std.exception if that name is not good enough. 
 std.assertion makes no sense - particularly with enforce in there - and 
 std.unittests doesn't work, because it's not just unit testing stuff in there.

I really dislike the size argument. Because as a programmer I do not care how big a module is in the first place. First I care about how to find it. If I have found it I may say: "Oh that's a little module". But that usually doesn't bother me much. And putting something in a module where it does not belong is not an option. I think we agree on that since you would like to have a better name for std.exception. Just checked: std.bigint is quite small looking at it's documentation. std.complex as well. std.demangle is very small. std.functional has seven functions. std.getopt has one big function in it. std.uni is very small. There are more I'll guess. It's not the size of a module that comes first when to decide where to put something.
 So, unless you can come up with a better name for std.exception with the idea 
 that it's going to be holding my new functions along with what's already in 
 there, I don't think that discussing names is going to mean much. From a 
 contents standpoint, it does make sense that the stuff in std.exception and my 
 stuff would be lumped together, and it does not make sense for my stuff to be
in 
 its own module at this point. There isn't enough of it, and since it fits well
in 
 std.exception, there's even less reason to create a new module for it. So, if
a 
 change is to be made, I think that it's going to have to be to the name of 
 std.exception, and I can't think of a better name nor have I see a better name 
 suggested.

The poll is less about a good name. It should be more about where do you expect things to be. And since I do not have a good name for a module that contains exception and error handling I don't like putting both things in a module with a bad name. Because it will be very difficult to fix this later. Even adding functions to modules may cause compile problems. I may have defined an assertPred already in one of my modules. I created the poll in the hope for more feedback how others relate to this issue. I'm very unsure how important these module naming and hierarchy issues are. But I fear that it may be too late to fix these later. But there is a thread about renaming/removing modules in std. The more people start using Phobos the less it will be possible to fix these things. And I have the impression that there are more people using it day to day. But after all if you do not feel the pain maybe I'm too sensitive. Andrei is also fine with it. Jens
Feb 01 2011
prev sibling next sibling parent Jonathan M Davis <jmdavisProg gmx.com> writes:
On Tuesday 01 February 2011 06:44:56 Jens Mueller wrote:
 Jonathan M Davis wrote:
 On Monday 31 January 2011 15:49:11 Jens Mueller wrote:
 spir wrote:
 On 01/30/2011 01:13 PM, Jens Mueller wrote:
I do not like putting it in std.exception. Maybe the name
std.unittest is also not good. I would propose std.assert if assert
wasn't a keyword. [...]

assertions (Assert_Error_) in a module named std.exception.

Same for me. Find it strange. Would never search assertion helper funcs inside std.exception. Why not std.assertion? std.unittests would be fine if there were some more stuff in there, I mean not only assertions. Else, the obvious name imo is std.assertion.

Nice. I just wonder what others think. I'd like to start a poll http://doodle.com/vn2ceenuvfwtx38e In general are those polls useful? I mean there where some discussions in the last time where a poll may help. git vs. hg. 80 vs. 90 characters per line. If all arguments are on the table it can be useful to have an opinion poll to finally settle the discussion. It may even be that I'm totally wrong here. But I think the module naming needs to be done very careful. It's what a D newcomer needs to grasp as easy as possible. Am I too picky? Somehow I care (too?) much about names.

I wouldn't actually be looking for "assertion helpers" anywhere. I might be looking for unit test helper, but I wouldn't be thinking about assertions at all, even if the unit test helpers threw AssertError like they do. But truth be told, I don't generally look for modules with a particular name unless I know what I'm looking for. I look at the list of modules and see which names seem like they'd have what I'd want. As such, std.unittests would make me think that the module held unit test stuff, whereas I really wouldn't know what to expect in std.assertion or std.exception at all - _especially_ std.assertion.

When I hear std.assertion and std.exception then I assume that these modules contain stuff for working with assertions and exceptions respectively. Just the fact that I want to throw an exception makes me go to std.exception to check out what's in there.
 However, given the small number of unit test functions, it makes no sense
 for them to be their own module. So, I really don't think that
 std.unittests makes sense at this point. std.exception may not be the
 best name, but given what's currently in it, the unit testing functions
 fit in there reasonably well. So, the issue then is what to name
 std.exception if that name is not good enough. std.assertion makes no
 sense - particularly with enforce in there - and std.unittests doesn't
 work, because it's not just unit testing stuff in there.

I really dislike the size argument. Because as a programmer I do not care how big a module is in the first place. First I care about how to find it. If I have found it I may say: "Oh that's a little module". But that usually doesn't bother me much. And putting something in a module where it does not belong is not an option. I think we agree on that since you would like to have a better name for std.exception. Just checked: std.bigint is quite small looking at it's documentation. std.complex as well. std.demangle is very small. std.functional has seven functions. std.getopt has one big function in it. std.uni is very small. There are more I'll guess. It's not the size of a module that comes first when to decide where to put something.
 So, unless you can come up with a better name for std.exception with the
 idea that it's going to be holding my new functions along with what's
 already in there, I don't think that discussing names is going to mean
 much. From a contents standpoint, it does make sense that the stuff in
 std.exception and my stuff would be lumped together, and it does not
 make sense for my stuff to be in its own module at this point. There
 isn't enough of it, and since it fits well in std.exception, there's
 even less reason to create a new module for it. So, if a change is to be
 made, I think that it's going to have to be to the name of
 std.exception, and I can't think of a better name nor have I see a
 better name suggested.

The poll is less about a good name. It should be more about where do you expect things to be. And since I do not have a good name for a module that contains exception and error handling I don't like putting both things in a module with a bad name. Because it will be very difficult to fix this later. Even adding functions to modules may cause compile problems. I may have defined an assertPred already in one of my modules. I created the poll in the hope for more feedback how others relate to this issue. I'm very unsure how important these module naming and hierarchy issues are. But I fear that it may be too late to fix these later. But there is a thread about renaming/removing modules in std. The more people start using Phobos the less it will be possible to fix these things. And I have the impression that there are more people using it day to day. But after all if you do not feel the pain maybe I'm too sensitive. Andrei is also fine with it.

I don't think that std.exception is a great name, but I _do_ think that my unit testing functions fit in well with what's already there (save for pointsTo and assumeUnique - I have _no_ idea why those two are in there - probably because they don't actually fit in well anywhere). So, from the perspective of the _contents_ of the module, it makes sense to put them in there. The _name_ of the module isn't great, but I don't know what a better name would be. Perhaps the the old std.contracts name fits a bit better with the addition of functions like assertPred which can reasonably be used in contracts, but it's not like std.contracts is a great name either. Yes, we _could_ create a std.unittests with assertPred, assertThrown, and assertNotThrown in it, but they're similar enough to enforce that I do like the idea of them being in the same module as enforce. I'm open to putting my functions wherever makes the most sense, but I don't think that it's entirely clear where that would be. I do think that putting them in std.exception makes a fair bit of sense, and that's where Andrei thinks that they should go, so I think that that works just fine. - Jonathan M Davis
Feb 01 2011
prev sibling next sibling parent Jonathan M Davis <jmdavisProg gmx.com> writes:
On Tuesday 01 February 2011 07:21:54 Don wrote:
 Jonathan M Davis wrote:
 On Sunday 30 January 2011 05:28:36 SHOO wrote:
 To be frank, I don't think that such a helper is necessary.
 I think these helpers will harm intuitive readability of unittest code.
 For unittest code, it is necessary to be able to understand easily even
 if without the document.

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.

Well, it's quite common for unit testing frameworks to have stuff like assertEqual, assertNotEqual, assertLessThan, etc. assertPred!"==" isn't all that different, except it embeds the actual operator in it via a template argument, so if anything, it's arguably easier to read than those. And honestly, I think that the added debugging benefit of assertPred far outweighs any potential readability issues. assert as it stands is quite poor in comparison.
  >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.

Valid point. However, I do think that the added usefulness of assertPred far outweighs any readability issues, but honestly, I don't think that it's all that hard to read - especially when it's very typical for unit testing frameworks to have function such as assertEqual and assertGreaterThan. - Jonathan M Davis
Feb 01 2011
prev sibling next sibling parent Jens Mueller <jens.k.mueller gmx.de> writes:
Michel Fortin wrote:
 On 2011-02-01 10:34:26 -0500, Andrei Alexandrescu
 <SeeWebsiteForEmail 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. Jens
Feb 01 2011
prev sibling next sibling parent Jonathan M Davis <jmdavisProg gmx.com> writes:
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 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
Feb 01 2011
prev sibling next sibling parent Jonathan M Davis <jmdavisProg gmx.com> writes:
On Tuesday 01 February 2011 09:12:16 Andrei Alexandrescu wrote:
 On 2/1/11 10:51 AM, Michel Fortin wrote:
 On 2011-02-01 11:31:54 -0500, Andrei Alexandrescu
 <SeeWebsiteForEmail erdani.org> said:
 TypeInfo holds a pointer to the toString function, so if the compiler
 passes the two operands as D-style variadic arguments to the assert
 handler, the assert handler can use toString to print them. The operator
 should be passed as a string.

In that case problem solved. Don, if you arrange things such that this user-level code: int a = 42; double b = 3.14; assert(a <= b, "Something odd happened"); ultimately calls this runtime function: assertCmpFailed("<=", "42", "3.14", "Something odd happened"); I promise I'll discuss with Sean and implement what it takes in druntime to get that completed. We need to finalize that before Feb 7 though because on that date the vote for Jonathan's library closes. If you do implement that, probably we'll need to reject the library in the current form and propose back an amended version.

You do need to remember to take into account though that expressions can be quite a bit more complex than a <= b, and you still want to be able to print out something useful. It's been discussed a bit already, but IIRC what appeared to be the best solution was that when an expression evaluated to false, you take the expression to evaluated and stop evaluating it when all that was left is operators or functions which resulted in bool. Then the value of those operands would be printed. e.g. assert(min(5 + 2, 4) < 2 && max(5, 7) < 10); would print out the values 4, 2, 7, and 10. and assert(canFind("hello world", "goodbye")); would print out the values "hello world" and "goodbye". Regardless, trying to do it in assert complicates things a fair bit. assertPred gives you more control, because you can choose how to break up the expression and arguments, and then those arguments are the ones which get printed. assert has to be much smarter about figuring out what it should print. Done correctly, it would be fantastic for assert to be that smart, but I wouldn't expect it to be easy. - Jonathan M davis
Feb 01 2011
prev sibling next sibling parent Jonathan M Davis <jmdavisProg gmx.com> writes:
On Tuesday 01 February 2011 11:32:08 Andrei Alexandrescu wrote:
 On 2/1/11 11:29 AM, Jonathan M Davis wrote:
 On Tuesday 01 February 2011 09:12:16 Andrei Alexandrescu wrote:
 On 2/1/11 10:51 AM, Michel Fortin wrote:
 On 2011-02-01 11:31:54 -0500, Andrei Alexandrescu
 <SeeWebsiteForEmail erdani.org>  said:
 TypeInfo holds a pointer to the toString function, so if the compiler
 passes the two operands as D-style variadic arguments to the assert
 handler, the assert handler can use toString to print them. The
 operator should be passed as a string.

In that case problem solved. Don, if you arrange things such that this user-level code: int a = 42; double b = 3.14; assert(a<= b, "Something odd happened"); ultimately calls this runtime function: assertCmpFailed("<=", "42", "3.14", "Something odd happened"); I promise I'll discuss with Sean and implement what it takes in druntime to get that completed. We need to finalize that before Feb 7 though because on that date the vote for Jonathan's library closes. If you do implement that, probably we'll need to reject the library in the current form and propose back an amended version.

You do need to remember to take into account though that expressions can be quite a bit more complex than a<= b, and you still want to be able to print out something useful. It's been discussed a bit already, but IIRC what appeared to be the best solution was that when an expression evaluated to false, you take the expression to evaluated and stop evaluating it when all that was left is operators or functions which resulted in bool. Then the value of those operands would be printed. e.g. assert(min(5 + 2, 4)< 2&& max(5, 7)< 10); would print out the values 4, 2, 7, and 10.

It's all about the top-level AST node. In this case I think it's reasonable to print "false && __unevaluated__" or simply "false". If the user wants the separate values, they can always write: assert(min(5 + 2, 4) < 2); assert(max(5, 7) < 10);
 and
 
 assert(canFind("hello world", "goodbye"));
 
 would print out the values
 
 "hello world" and "goodbye".

I guess again taking the function call as the top node, assert could print the arguments passed into the function.

Well, what the best way to handle it is and what exactly is possible, I don't know. However, it needs to print out the actual values of the expression and be reasonably close to what the programmer would be looking for in terms of which values would be printed. The simple examples are obvious. The complex ones aren't as obvious. And it may be that we can't get it to quite print out what your average programmer would want, but if we're close - particularly if we get the simple expressions right, then it may be good enough. Whatever assert can be made to do, it's unlikely to be as flexible as assertPred, but if we can get to do most of what assertPred does, then the remainder could be broken out into other functions for that purpose, or maybe those functions don't need to make it into Phobos. Regardless, I think that we definitely need better error reporting on unit test failure. assertPred does that. But if the built in assert could do that, then we'd have the best assert out of any language that I've ever seen. - Jonathan M Davis
Feb 01 2011
prev sibling next sibling parent Jonathan M Davis <jmdavisProg gmx.com> writes:
On Friday 04 February 2011 04:10:37 bearophile wrote:
 Andrei:
 We need to have at least a definite decision by Feb 7 from Don and
 Walter that work will be put or not into improving assert as discussed.

I think that fixing assert() is a better strategy, even it may require more work. A problem with assert is that it wants to do too many things, so there are problems: - assert(0); to mean HALT, that gives some problems - assert(class_instance); that calls its invariant. (I vaguely remember a problem with this with struct instance invariants). - Not good enough for unittests, as seen in this thread. So an effort to improve assert may solve all four those problems. Removing the two purposes (moving them to specialized and explicit features, specialized little tools) and adding the functionality we desire in unittesting.

Making assert smarter with regards to errors so that it's better suited for unit tests makes sense, and I do find the assert(class_instance) bit to be rather disturbing given that it conflicts with how conditional checks using class instances usually works (I only recently found out that assert worked that way with class instances). However, I don't see why there would be any problem with assert(0) meaning halt. It's a normal assert in non-release mode and it sticks around in release mode, becoming a halt instruction. I use it that way all the time. I don't see any problem with it whatsoever. - Jonathan M Davis
Feb 04 2011
prev sibling next sibling parent Jonathan M Davis <jmdavisProg gmx.com> writes:
On Friday 04 February 2011 04:30:42 bearophile wrote:
 Jonathan M Davis:
 However, I don't see why there would be any problem with
 assert(0) meaning halt. It's a normal assert in non-release mode and it
 sticks around in release mode, becoming a halt instruction. I use it
 that way all the time. I don't see any problem with it whatsoever.

There was a recent discussion about this. The final consensus was that assert(0) to mean HALT is a small wart in the D language, but it's not worth fixing it, because it's a small enough defect. But if now assert() gets a significant facelift, then this little problem may be fixed along with the other two.

Well, aside from the fact that I don' t think that it's a wart at all, I would point out that TDPL specifically points out that assert(false) is converted to a halt instruction. So, given that it's not a serious problem, I really don't think that it's reasonable to change it. At this point, for us to change something that's in TDPL, it pretty much needs to need changing, and I really don't think that assert(0) qualifies. - Jonathan M Davis
Feb 04 2011
prev sibling next sibling parent Jonathan M Davis <jmdavisProg gmx.com> writes:
On Friday 04 February 2011 04:40:35 bearophile wrote:
 So moving them into explicit little features makes the language more

The two little features I am talking about are: - a halt() somewhere like in core.bitop or std.intrinsic or the default object module to replace assert(0); - And to use class_instance.invariant() to replace assert(class_instance).

assert(0) has the advantage of being a normal assertion in non-release mode. It also makes it clear that that code path should _never_ be reached. I don't think that halt would be as clear in that regard and it _certainly_ wouldn't imply that it would be a normal assert in non-release mode. We'd then have problems with halt not actually being halt in non-release mode... I'm _definitely_ against changing assert(0). The real question though is whether you can convince Walter (which I doubt, but I don't know). Still, making such a change _would_ contradict TDPL, which is supposed to be a major no-no at this point. - Jonathan M Davis
Feb 04 2011
prev sibling next sibling parent Jonathan M Davis <jmdavisProg gmx.com> writes:
On Friday 04 February 2011 13:29:38 bearophile wrote:
 Jonathan M Davis:
 assert(0) has the advantage of being a normal assertion in non-release
 mode.

What is this useful for? To me this looks like a significant disadvantage. If I want a HALT (to tell the compiler a point can't be reached, etc) I want it in every kind of compilation of the program.
 It also makes it clear that that code path should _never_ be reached.

The replacement for assert(0) is meant to be more clear in its purpose compared to assert(0). It may be named thisCantHappen(), or assertZero(), etc.

assert(0) will actually give a stack trace with a file and line number. It will also give you a message if you include one with it. HALT just kills the program. I _much_ prefer that assert(0) be a normal assert in no-release mode. Leaving it in as a HALT has the advantage that the program will just die if it reaches that point in release mode rather than trying to continue with the assert gone, but I very much want a normal assert in non-release mode. It's much more useful.
 The real question though is whether you can convince Walter (which I
 doubt, but I don't know).

This topic was already discussed, and I think the result of the discussion was that this change of assert(false) is not worth it. But if asserts gets inproved for other purposes, then this is a chance to work on improving assert(0) too.
 Still, making such a change _would_ contradict TDPL, which is supposed to
 be a major no-no at this point.<

I like TDPL, I respect Andrei and you, I agree that TDPL is a kind of reference for D2, but please stop using TDPL as a The Bible in many of your posts. Not even Andrei himself looks so religiously attached as you to the contents of TDPL :-) A little flexibility is acceptable.

I believe that Walter and Andrei have made it fairly clear that if we do anything that contradicts TDPL, it needs to have a very good reason to be done. TDPL is _supposed_ to have been the final word. Unfortunately, the implementation is behind, so _it_ is possible that we're going to have to make changes which contradict it. However, if we do, those changes have to be needed or at least really merit the cost of contradicting TDPL. Something as small as changing assert(0) is unlikely to do that.
 --------------------------
 
 Adam Ruppe:
Again, I think this makes sense too. assert(n) is saying, in principle,
"ensure n is valid". Calling the invariant is part of ensuring it is
valid. The only problem is a minor bug - it forgets to check for null
before calling the invariant. That should, of course, be fixed, but the
main behavior makes sense.<

Something like class_instance.invariant() is better because: - It's explicit and readable. While you need to read the D manual to know that assert(class_instance) tests the invariant too, and not just that class_instance reference is not null. I remember other persons in D.learn not knowning/suprised by that assert(class_instance) tests the invariant too. - It removes a special case, and special cases kill languages. Struct instances too allow an invariant, but they are not callable with assert(), see below. A syntax like structlass_instance.invariant() is shared with the class one. struct Foo { int x; invariant() { assert(x == 1); } } void main() { Foo f; assert(f); } DMD 2.051: test.d(7): Error: expression f of type Foo does not have a boolean value

Actually, the more I think about it, the less I see assert(class_instance) to be a problem. Normally it would check that the reference was non-null. Assuming that the feature isn't buggy, it'll still do that, but it'll check the invariant in addition. And since the invariant is always supposed to be true, that shouldn't be a problem. I really don't think that assert needs to be fundamentally changed with regards to assert(0) or assert(class_instance). - Jonathan M Davis
Feb 04 2011
prev sibling parent Jonathan M Davis <jmdavisProg gmx.com> writes:
On Saturday 05 February 2011 02:43:56 spir wrote:
 On 02/05/2011 08:29 AM, Jonathan M Davis wrote:
 On Friday 04 February 2011 13:29:38 bearophile wrote:
 Jonathan M Davis:
 assert(0) has the advantage of being a normal assertion in non-release
 mode.

What is this useful for? To me this looks like a significant disadvantage. If I want a HALT (to tell the compiler a point can't be reached, etc) I want it in every kind of compilation of the program.
 It also makes it clear that that code path should _never_ be reached.

The replacement for assert(0) is meant to be more clear in its purpose compared to assert(0). It may be named thisCantHappen(), or assertZero(), etc.

assert(0) will actually give a stack trace with a file and line number. It will also give you a message if you include one with it. HALT just kills the program. I _much_ prefer that assert(0) be a normal assert in no-release mode. Leaving it in as a HALT has the advantage that the program will just die if it reaches that point in release mode rather than trying to continue with the assert gone, but I very much want a normal assert in non-release mode. It's much more useful.
 The real question though is whether you can convince Walter (which I
 doubt, but I don't know).

This topic was already discussed, and I think the result of the discussion was that this change of assert(false) is not worth it. But if asserts gets inproved for other purposes, then this is a chance to work on improving assert(0) too.
 Still, making such a change _would_ contradict TDPL, which is supposed
 to be a major no-no at this point.<

I like TDPL, I respect Andrei and you, I agree that TDPL is a kind of reference for D2, but please stop using TDPL as a The Bible in many of your posts. Not even Andrei himself looks so religiously attached as you to the contents of TDPL :-) A little flexibility is acceptable.

I believe that Walter and Andrei have made it fairly clear that if we do anything that contradicts TDPL, it needs to have a very good reason to be done. TDPL is _supposed_ to have been the final word. Unfortunately, the implementation is behind, so _it_ is possible that we're going to have to make changes which contradict it. However, if we do, those changes have to be needed or at least really merit the cost of contradicting TDPL. Something as small as changing assert(0) is unlikely to do that.
 struct Foo {
 
      int x;
      invariant() { assert(x == 1); }
 
 }
 void main() {
 
      Foo f;
      assert(f);
 
 }
 
 
 DMD 2.051:
 test.d(7): Error: expression f of type Foo does not have a boolean value

Actually, the more I think about it, the less I see assert(class_instance) to be a problem. Normally it would check that the reference was non-null. Assuming that the feature isn't buggy, it'll still do that, but it'll check the invariant in addition. And since the invariant is always supposed to be true, that shouldn't be a problem. I really don't think that assert needs to be fundamentally changed with regards to assert(0) or assert(class_instance). - Jonathan M Davis

All right, I guess I get your point. I still think that checking a class's invariant should be explicit. Assert(whatever) means for me "check whatever is not (equivalent to) false", not "check whatever is not (equivalent to) false and whatever's invariant condition, if any, is fulfilled". Note that an object's invariant is *not* part of its regular truth value (1st assertion in unittest below indeed passes). Thus, it is clearly incorrect that assert(x) checks its invariant implicitely: assert ( x ); assert ( cast(bool)x == true ) ; should always have the same outcome. Thus, the idiom to check an object's invariant should be different, at least in a visible detail. A side-issue is the following code: class C { int i; invariant () { assert(i == 1); } } unittest { auto c = new C(); assert ( cast(bool)c == true ) ; assert ( c ) ; } throws a plain "Assertion error" with no additional comment. No mention of invariant checking, thus no hint to debug. I guess the problem would be more acceptable if asserts in invariants had at least slightly different error message forms, if only "Invariant assertion error". (Yes, one can customize error messages, sure; but defaults should be as helpful as possible.)

The AssertError should give the file and line number which should then tell you exactly which assert failed. Also, if stack traces work (which they do on Linux, but I don't think that they do on Windows yet), then you get a stack trace. It might be nice if the AssertError also said that it was in an invariant, but I don't think that it much matters where the assert was when an assert fails - be in a function, or in an invariant, or in a in block, or wherever. What matters is knowing which assert failed so that you can deal with it. The file and line number give you that. The fact that an assert was in an invariant as opposed to anywhere else is kind of irrelevant as far as the error message goes. - Jonathan M Davis
Feb 05 2011