www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.learn - assert(false)

reply Joseph Rushton Wakeling <joseph.wakeling webdrake.net> writes:
Is it considered good/recommended practice to insert an assert(false) statement
in parts of the code that should be unreachable?  Or simply an optional choice
that may be useful for debugging?
Jun 20 2013
next sibling parent reply =?UTF-8?B?QWxpIMOHZWhyZWxp?= <acehreli yahoo.com> writes:
On 06/20/2013 12:48 PM, Joseph Rushton Wakeling wrote:

 Is it considered good/recommended practice to insert an assert(false) 
statement
 in parts of the code that should be unreachable?  Or simply an 
optional choice
 that may be useful for debugging?
And a reminder that assert(false) (and assert(0)) is never removed from code (even when other asserts are). So, they are always there to prevent the program from producing unexpected results. Ali
Jun 20 2013
next sibling parent Joseph Rushton Wakeling <joseph.wakeling webdrake.net> writes:
On 06/20/2013 09:28 PM, Ali Çehreli wrote:
 On 06/20/2013 12:48 PM, Joseph Rushton Wakeling wrote:
 
 Is it considered good/recommended practice to insert an assert(false) statement
 in parts of the code that should be unreachable?  Or simply an optional choice
 that may be useful for debugging?
And a reminder that assert(false) (and assert(0)) is never removed from code (even when other asserts are). So, they are always there to prevent the program from producing unexpected results.
OK. I noticed them present in some code in Phobos, and there's a place in RandomSample where one could reasonably expect to see one but there isn't right now. I was considering whether to include it in some patches I'm preparing.
Jun 20 2013
prev sibling next sibling parent "Jonathan M Davis" <jmdavisProg gmx.com> writes:
On Thursday, June 20, 2013 21:47:55 Joseph Rushton Wakeling wrote:
 On 06/20/2013 09:28 PM, Ali Çehreli wrote:
 On 06/20/2013 12:48 PM, Joseph Rushton Wakeling wrote:
 Is it considered good/recommended practice to insert an assert(false)
 statement in parts of the code that should be unreachable? Or simply an
 optional choice that may be useful for debugging?
And a reminder that assert(false) (and assert(0)) is never removed from code (even when other asserts are). So, they are always there to prevent the program from producing unexpected results.
OK. I noticed them present in some code in Phobos, and there's a place in RandomSample where one could reasonably expect to see one but there isn't right now. I was considering whether to include it in some patches I'm preparing.
I definitely use assert(false) for what is supposed to be unreachable code. The two prime places that I can think of off the top of my head are 1. In a catch block when it's supposed to be impossible for the try block to throw (e.g. when calling format in a nothrow function, and you know that format won't throw with the arguments that you're passing it). 2. In the default case of a switch statement when the default case should be unreachable. If those ever _are_ reached, then your code has serious problems, and killing the program is almost certainly a good thing. - Jonathan M Davis
Jun 20 2013
prev sibling next sibling parent Joseph Rushton Wakeling <joseph.wakeling webdrake.net> writes:
On 06/20/2013 10:15 PM, Jonathan M Davis wrote:
 I definitely use assert(false) for what is supposed to be unreachable code.
The 
 two prime places that I can think of off the top of my head are
 
 1. In a catch block when it's supposed to be impossible for the try block to 
 throw (e.g. when calling format in a nothrow function, and you know that 
 format won't throw with the arguments that you're passing it).
 
 2. In the default case of a switch statement when the default case should be 
 unreachable.
 
 If those ever _are_ reached, then your code has serious problems, and killing 
 the program is almost certainly a good thing.
In my case it was a bunch of if/else where the program should return from one of the blocks. I saw comparable cases in RandomCover with an assert(false) at the very end of the function, so thought I'd better tweak my code accordingly.
Jun 20 2013
prev sibling parent "bearophile" <bearophileHUGS lycos.com> writes:
Ali Çehreli:

 And a reminder that assert(false) (and assert(0)) is never 
 removed from code (even when other asserts are).
Because in release mode assert(false) is not an assert any more, it's an HALT instruction, and it doesn't show the error message: assert(false, "Not shown in release optimized mode"); I don't like a lot this conflation of a different semantics, but after some discussion Walter in the end decided to keep things this way, and not introduce a proper different built-in halt intrinsic. Bye, bearophile
Jun 20 2013
prev sibling next sibling parent "Jonathan M Davis" <jmdavisProg gmx.com> writes:
On Thursday, June 20, 2013 22:21:28 Joseph Rushton Wakeling wrote:
 In my case it was a bunch of if/else where the program should return from
 one of the blocks. I saw comparable cases in RandomCover with an
 assert(false) at the very end of the function, so thought I'd better tweak
 my code accordingly.
Putting it at the end of the function is unnecessary. The compiler already does that for you. - Jonathan M Davis
Jun 20 2013
prev sibling next sibling parent Joseph Rushton Wakeling <joseph.wakeling webdrake.net> writes:
On 06/20/2013 10:25 PM, Jonathan M Davis wrote:
 Putting it at the end of the function is unnecessary. The compiler already 
 does that for you.
Then there are a bunch of functions in std.random ending with unnecessary assert(false) statements. Shall I make this tweak as an addition to current pull request?
Jun 20 2013
prev sibling next sibling parent Joseph Rushton Wakeling <joseph.wakeling webdrake.net> writes:
On 06/20/2013 10:33 PM, Joseph Rushton Wakeling wrote:
 On 06/20/2013 10:25 PM, Jonathan M Davis wrote:
 Putting it at the end of the function is unnecessary. The compiler already 
 does that for you.
Then there are a bunch of functions in std.random ending with unnecessary assert(false) statements. Shall I make this tweak as an addition to current pull request?
... one is necessary as it's in a function that is supposed to return a value. There are also some return; statements at the end of void functions.
Jun 20 2013
prev sibling next sibling parent "Jonathan M Davis" <jmdavisProg gmx.com> writes:
On Thursday, June 20, 2013 22:37:17 Joseph Rushton Wakeling wrote:
 On 06/20/2013 10:33 PM, Joseph Rushton Wakeling wrote:
 On 06/20/2013 10:25 PM, Jonathan M Davis wrote:
 Putting it at the end of the function is unnecessary. The compiler
 already
 does that for you.
Then there are a bunch of functions in std.random ending with unnecessary assert(false) statements. Shall I make this tweak as an addition to current pull request?
If they're not needed, remove them.
 ... one is necessary as it's in a function that is supposed to return a
 value.
I don't think that that's supposed to be necessary. The only reason that that should happen is if the compiler can deterimine that the execution can definitely reach the end of the function.
 There are also some return; statements at the end of void functions.
That's pointless. - Jonathan M Davis
Jun 20 2013
prev sibling next sibling parent Timothee Cour <thelastmammoth gmail.com> writes:
A)
does assert(foo) where foo is an expression that cat be evaluated at
compile time (eg via CTFE etc) behave same as assert(true)/assert(false)?


B)

It is usually "bad practice" to keep an assert in release
 ("assert(false)"), because the code should already have been checked, and
 you shouldn't pay for the check in release.
 BUT: Since the code is theoretically unreachable, there is no real world
 penalty to the check anyway, so you might put it in anyways, on the off
 chance it saves your but (IMO).
What do you mean by 'you shouldn't pay for the check in release' ? If the boolean argument is known at compile time, there is no check, so no overhead right? (ie compiler should optimize away) On Thu, Jun 20, 2013 at 2:25 PM, Jonathan M Davis <jmdavisProg gmx.com>wrote:
 On Thursday, June 20, 2013 22:21:28 Joseph Rushton Wakeling wrote:
 In my case it was a bunch of if/else where the program should return from
 one of the blocks. I saw comparable cases in RandomCover with an
 assert(false) at the very end of the function, so thought I'd better
tweak
 my code accordingly.
Putting it at the end of the function is unnecessary. The compiler already does that for you. - Jonathan M Davis
Jun 20 2013
prev sibling next sibling parent "Jonathan M Davis" <jmdavisProg gmx.com> writes:
On Thursday, June 20, 2013 15:45:14 Timothee Cour wrote:
 A)
 does assert(foo) where foo is an expression that cat be evaluated at
 compile time (eg via CTFE etc) behave same as assert(true)/assert(false)?
If it _is_ known at compile time, then yes, but simply calling a function that _could_ be called during compile time won't result in it being checked at compile time.
 
 B)
 
 It is usually "bad practice" to keep an assert in release
 
 ("assert(false)"), because the code should already have been checked, and
 you shouldn't pay for the check in release.
 BUT: Since the code is theoretically unreachable, there is no real world
 penalty to the check anyway, so you might put it in anyways, on the off
 chance it saves your but (IMO).
What do you mean by 'you shouldn't pay for the check in release' ? If the boolean argument is known at compile time, there is no check, so no overhead right? (ie compiler should optimize away)
It's only optimized away if the section of code that it's in is optimized away, and unreachable code is not necessarily optimized away. The compiler would have to determine that it's unreachable in order to do that, and it frequently can't do that. And assert(false) is turned into a HLT instruction with -release, so it never goes away because of -release, unlike other assertions. - Jonathan M Davis
Jun 20 2013
prev sibling next sibling parent reply Joseph Rushton Wakeling <joseph.wakeling webdrake.net> writes:
On 06/20/2013 11:36 PM, Jonathan M Davis wrote:
 ... one is necessary as it's in a function that is supposed to return a
 value.
I don't think that that's supposed to be necessary. The only reason that that should happen is if the compiler can deterimine that the execution can definitely reach the end of the function.
It's in diceImpl: private size_t diceImpl(Rng, Range)(ref Rng rng, Range proportions) if (isForwardRange!Range && isNumeric!(ElementType!Range) && isForwardRange!Rng) { double sum = reduce!("(assert(b >= 0), a + b)")(0.0, proportions.save); enforce(sum > 0, "Proportions in a dice cannot sum to zero"); immutable point = uniform(0.0, sum, rng); assert(point < sum); auto mass = 0.0; size_t i = 0; foreach (e; proportions) { mass += e; if (point < mass) return i; i++; } // this point should not be reached assert(false); } Obviously one can see logically that this assert will never be reached, but I don't think there's anything the compiler can pick up on to be certain. Incidentally, I just realized that with diceImpl, Phobos has a very nice function in place to support a simple (though possibly inefficient) implementation of the Barabasi-Albert algorithm for generating scale free networks ... :-)
Jun 20 2013
parent reply "monarch_dodra" <monarchdodra gmail.com> writes:
On Thursday, 20 June 2013 at 23:36:35 UTC, Joseph Rushton 
Wakeling wrote:
 On 06/20/2013 11:36 PM, Jonathan M Davis wrote:
 ... one is necessary as it's in a function that is supposed 
 to return a
 value.
I don't think that that's supposed to be necessary. The only reason that that should happen is if the compiler can deterimine that the execution can definitely reach the end of the function.
It's in diceImpl: private size_t diceImpl(Rng, Range)(ref Rng rng, Range proportions) if (isForwardRange!Range && isNumeric!(ElementType!Range) && isForwardRange!Rng) { double sum = reduce!("(assert(b >= 0), a + b)")(0.0, proportions.save); enforce(sum > 0, "Proportions in a dice cannot sum to zero"); immutable point = uniform(0.0, sum, rng); assert(point < sum); auto mass = 0.0; size_t i = 0; foreach (e; proportions) { mass += e; if (point < mass) return i; i++; } // this point should not be reached assert(false); } Obviously one can see logically that this assert will never be reached, but I don't think there's anything the compiler can pick up on to be certain.
Reading that code, it's not immediately obvious to me: The assert makes it self documenting. In any case, this goes back to what I first said: If you remove that assert, the compiler will complain that not all paths return.
Jun 21 2013
parent reply Joseph Rushton Wakeling <joseph.wakeling webdrake.net> writes:
On 06/21/2013 09:51 AM, monarch_dodra wrote:
 Reading that code, it's not immediately obvious to me: The assert makes it self
 documenting.
I did also have to think about it for a few seconds. You have a sum of values from an array; you generate a uniformly-distributed random number point in [0.0, sum); you sequentially sum values from the same array, and return when that partial sum ("mass") exceeds the value of point. Since point < sum you must do this eventually, but the compiler has no way to recognize that the maximum possible value of the variable mass will inevitably exceed the value of point.
 In any case, this goes back to what I first said: If you remove that assert,
the
 compiler will complain that not all paths return.
Indeed. :-)
Jun 21 2013
parent =?UTF-8?B?QWxpIMOHZWhyZWxp?= <acehreli yahoo.com> writes:
On 06/21/2013 02:56 AM, Joseph Rushton Wakeling wrote:

 I did also have to think about it for a few seconds.  You have
 a sum of values from an array; you generate a
 uniformly-distributed random number point in [0.0, sum); you
 sequentially sum values from the same array, and return when
 that partial sum ("mass") exceeds the value of point.  Since
 point < sum you must do this eventually
I spent a considerable amount of time reading that function as well. :) I was afraid whether two separate floating point sums would give the same result. After all, the initial sum is a 'double' but the type of the elements of 'proportions' may be different. (I don't know the answer.) Ali
Jun 21 2013
prev sibling next sibling parent "Steven Schveighoffer" <schveiguy yahoo.com> writes:
On Thu, 20 Jun 2013 15:48:38 -0400, Joseph Rushton Wakeling  
<joseph.wakeling webdrake.net> wrote:

 Is it considered good/recommended practice to insert an assert(false)  
 statement
 in parts of the code that should be unreachable?  Or simply an optional  
 choice
 that may be useful for debugging?
I think it's more than good practice, the compiler may require it if it cannot prove via flow analysis that you are not forgetting to return a value. Note that assert(false) can make the compiler generate code differently, if the compiler can prove the function never returns. I'm more fuzzy on this detail. -Steve
Jun 21 2013
prev sibling parent "H. S. Teoh" <hsteoh quickfur.ath.cx> writes:
On Fri, Jun 21, 2013 at 12:36:13AM +0100, Joseph Rushton Wakeling wrote:
[...]
     private size_t diceImpl(Rng, Range)(ref Rng rng, Range proportions)
     if (isForwardRange!Range && isNumeric!(ElementType!Range) &&
isForwardRange!Rng)
     {
         double sum = reduce!("(assert(b >= 0), a + b)")(0.0, proportions.save);
         enforce(sum > 0, "Proportions in a dice cannot sum to zero");
         immutable point = uniform(0.0, sum, rng);
         assert(point < sum);
         auto mass = 0.0;
 
         size_t i = 0;
         foreach (e; proportions)
         {
             mass += e;
             if (point < mass) return i;
             i++;
         }
         // this point should not be reached
         assert(false);
     }
[...] On Fri, Jun 21, 2013 at 09:45:55AM -0700, Ali ehreli wrote:
 On 06/21/2013 02:56 AM, Joseph Rushton Wakeling wrote:
 
 I did also have to think about it for a few seconds.  You have
 a sum of values from an array; you generate a
 uniformly-distributed random number point in [0.0, sum); you
 sequentially sum values from the same array, and return when
 that partial sum ("mass") exceeds the value of point.  Since
 point < sum you must do this eventually
I spent a considerable amount of time reading that function as well. :) I was afraid whether two separate floating point sums would give the same result. After all, the initial sum is a 'double' but the type of the elements of 'proportions' may be different. (I don't know the answer.)
[...] Hmm. This is a very good point. I actually found two potential problems with this code, due to the peculiarities of floating-point arithmetic: If 'proportions' contains an NaN, then sum will also be nan, and the enforce line will fail (and with a misleading message). If 'proportions' contains +inf, then sum will also be +inf, so the enforce will pass. Now what does uniform() return if sum is +inf? I tested this, and found that uniform(0.0, float.infinity) consistently returns double.infinity, and uniform(0.0, real.infinity) consistently returns real.infinity. Both will cause the next assert() will fail. (Had this assert not been there, the foreach could terminate without returning, e.g. if the last element of 'proportions' is +inf, then point=+inf and point < mass is never true.) There's also a potential pitfall if 'proportions' is a range of floats, with some adjacent elements that differ too much in magnitude such that summing them as floats will give a different answer from summing them as doubles. Fortunately, reduce is called with 0.0 as seed value, which is a double, so this problem doesn't occur here (each element is promoted to double before summation, so no loss of accuracy occurs). The reverse problem of a range of reals on x86, where sum may lose precision from very small real values that don't fit in a double, doesn't cause any problem either, because mass is also a double, and any precision loss is duplicated in both the reduce and the foreach, so there's no possibility of the loop terminating without returning. So, long story short, the assert(false) actually will never be reached. But I can't see the compiler perform this level of analysis (involving floating-point analysis on hypothetical template arguments!) to reach that conclusion! So yeah, the assert(false) should be left there. :) (Don't you just love floating-point? They make life so much more interesting than if we'd been able to use mathematically-exact reals! :-P) T -- Written on the window of a clothing store: No shirt, no shoes, no service.
Jun 21 2013