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