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?

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

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.

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

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.

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

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?

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.

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

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