www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - Fixing the issue with "Statement unreachable"

reply Steven Schveighoffer <schveiguy gmail.com> writes:
Is there a way we can fix this?

I have a situation with a large set of nested static ifs, coupled with 
returns based on runtime code:

-----
static if(condA)
{
    static if(condB)
    {
       static if(condC)
          if(funA()) return true;
       else static if(condD)
          if(funB()) return true;
       else
          if(funC()) return true;
    }
    else static if(condD)
       if(funD()) return true;
    else
       if(funE()) return true;
}

return false;
-----


Now, I have a situation where I'm inserting a condition nested way down. 
And it always is going to return true:

----
static if(condA)
{
    static if(condB)
    {
       static if(NEW_CONDITION) return true;
       else static if(condC)
          if(funA()) return true;
       else static if(condD)
          if(funB()) return true;
       else
          if(funC()) return true;
    }
    else static if(condD)
       if(funD()) return true;
    else
       if(funE()) return true;
}

return false;

----

Now, all of a sudden, the return false at the end is no good. If condA, 
condB, and NEW_CONDITION are true, the return false becomes a "Statement 
not reachable".

I see 2 ways to fix this, and both are horrible.

Way 1: I have a special static if before-hand that combines all the 
various conditions that get to that new branch, and then else everything 
else. e.g.:

static if(condA && condB && NEW_CONDITION) return true;
else { ... original code ... }

Way 2: I move "return false" in all the other branches. I can refactor 
out some of them, but I need a lot, which is super-annoying.

It bugs me that the compiler can't figure this out. Is there any way we 
can fix this? The "warning" is not really true -- the statement IS 
reachable in some instantiations.

Personally, I'd rather not have the compiler complain about ANY 
unreachable statements than complain about ones that are actually 
reachable. If it determines it's not reachable, just don't put it in the 
binary.

Would there be a way to say "if this part of the code is reachable, then 
include the following"?

(NOTE: I know I could switch all the "return true" to return the result 
of the functions, but the real code is more complex and not simple 
boolean calls)

-Steve
Apr 26 2020
next sibling parent drug <drug2004 bk.ru> writes:
Not fix, just an other way - instead of final `return false;` (not tested):

static if(!condA || !condB || !NEW_CONDITION) return false;
Apr 26 2020
prev sibling next sibling parent Mathias LANG <geod24 gmail.com> writes:
On Monday, 27 April 2020 at 03:59:57 UTC, Steven Schveighoffer 
wrote:

 Is there a way we can fix this?

 [...]

 Would there be a way to say "if this part of the code is 
 reachable, then include the following"?

 (NOTE: I know I could switch all the "return true" to return 
 the result of the functions, but the real code is more complex 
 and not simple boolean calls)

 -Steve


Personally when I can't find any satisfactory solution, I just 
add `bool hack` at the top and an `if (!hack)`. The backend / 
inliner might be smart enough to optimize it, but it's enough to 
shut the frontend up.
This is essentially 
https://issues.dlang.org/show_bug.cgi?id=14835 .
As you can see, Andrei proposed to insert an `else` if there's a 
static if. I'm personally not fan of this, I just think we should 
fix DMD frontend to stop emitting useless warnings.
Apr 26 2020
prev sibling parent reply Walter Bright <newshound2 digitalmars.com> writes:
I guessed at what was necessary to finish the example:

-------------
bool test()
{

static if(condA)
{
    static if(condB)
    {
       static if(NEW_CONDITION) return true;
       else static if(condC)
          if(funA()) return true;
       else static if(condD)
          if(funB()) return true;
       else
          if(funC()) return true;
    }
    else static if(condD)
       if(funD()) return true;
    else
       if(funE()) return true;
}

return false;
}

enum bool condA = true, condB = true, condC = true, condD = true;
enum NEW_CONDITION = true;
bool funA();
bool funB();
bool funC();
bool funD();
bool funE();
----------------------

which compiles without error. Evidently, your example must be different from 
this. Please post a complete example.
Apr 29 2020
parent reply Timon Gehr <timon.gehr gmx.ch> writes:
On 30.04.20 08:00, Walter Bright wrote:

 Evidently, your example must be different from this. Please post a 
 complete example.


You need -W. -W also notices the dangling elses, the intended code is this:

bool test(){
     static if(condA){
         static if(condB){
             static if(NEW_CONDITION) return true;
             else static if(condC){
                 if(funA()) return true;
             }else static if(condD) {
                 if(funB()) return true;
             }else{
                 if(funC()) return true;
             }
         }else static if(condD){
             if(funD()) return true;
         }else{
             if(funE()) return true;
         }
     }
     return false; // Warning: statement is not reachable
}

enum bool condA = true, condB = true, condC = true, condD = true;
enum NEW_CONDITION = true;
bool funA();
bool funB();
bool funC();
bool funD();
bool funE();
Apr 30 2020
next sibling parent reply matheus <matheus gmail.com> writes:
On Thursday, 30 April 2020 at 09:10:16 UTC, Timon Gehr wrote:

 On 30.04.20 08:00, Walter Bright wrote:

 Evidently, your example must be different from this. Please 
 post a complete example.


 You need -W. -W also notices the dangling elses, the intended 
 code is this:


True and by the way I minimized the code:

// THIS WORKS - https://d.godbolt.org/z/3v-gW2:
import std.stdio;
bool test(){
     static if(cond){
         if(func()){return true;}
     }
     return false;
}

enum bool cond = true;
bool func();

void main(string[ ] args) {
     writeln(test());
}


// WHILE BELOW GIVES THE ERROR: https://d.godbolt.org/z/8rNdjJ
import std.stdio;
bool test(){
     static if(cond){
         return true;
     }
     return false;
}

enum bool cond = true;
bool func();

void main(string[ ] args) {
     writeln(test());
}

As you can see, when you explicitly define a return inside a 
static if, the compiler emits the warning.

I think this is a wrong behavior and the warning only should be 
emitted if the code above had an "else", like for example:

// https://d.godbolt.org/z/fgY34m
import std.stdio;

bool test(){
     static if(cond){
         return true;
     }else{
         return false;
     }
     return false;
}

enum bool cond = true;
bool func();

void main(string[ ] args) {
     writeln(test());
}

In the above the warning makes sense.

Matheus.
Apr 30 2020
parent reply Steven Schveighoffer <schveiguy gmail.com> writes:
On 4/30/20 10:02 AM, matheus wrote:

 On Thursday, 30 April 2020 at 09:10:16 UTC, Timon Gehr wrote:

 On 30.04.20 08:00, Walter Bright wrote:

 Evidently, your example must be different from this. Please post a 
 complete example.


 You need -W. -W also notices the dangling elses, the intended code is 
 this:


 
 True and by the way I minimized the code:
 
 // THIS WORKS - https://d.godbolt.org/z/3v-gW2:
 import std.stdio;
 bool test(){
      static if(cond){
          if(func()){return true;}
      }
      return false;
 }
 
 enum bool cond = true;
 bool func();
 
 void main(string[ ] args) {
      writeln(test());
 }
 
 
 // WHILE BELOW GIVES THE ERROR: https://d.godbolt.org/z/8rNdjJ
 import std.stdio;
 bool test(){
      static if(cond){
          return true;
      }
      return false;
 }
 
 enum bool cond = true;
 bool func();
 
 void main(string[ ] args) {
      writeln(test());
 }
 
 As you can see, when you explicitly define a return inside a static if, 
 the compiler emits the warning.


Right, the first would still include the return false as generated code, 
this is not in dispute. And in fact, in your case, I would say the error 
is legitimate (that line is not reachable).

But in the case of a template where the condition is passed in, the line 
is reachable if the condition is one way.

However, there are other reasons why this might be "reachable" in other 
cases. For example, you could have:

version(abc)
    enum cond = false;
else
    enum cond = true;

which means that if you *compile* it a certain way, then it's reachable.


 I think this is a wrong behavior and the warning only should be emitted 
 if the code above had an "else", like for example:
 
 // https://d.godbolt.org/z/fgY34m
 import std.stdio;
 
 bool test(){
      static if(cond){
          return true;
      }else{
          return false;
      }
      return false;
 }
 
 enum bool cond = true;
 bool func();
 
 void main(string[ ] args) {
      writeln(test());
 }
 
 In the above the warning makes sense.


This makes a lot of sense, and probably is the way to go

-Steve
Apr 30 2020
parent reply matheus <matheus gmail.com> writes:
On Thursday, 30 April 2020 at 14:29:27 UTC, Steven Schveighoffer 
wrote:

 On 4/30/20 10:02 AM, matheus wrote:

 ...
 // WHILE BELOW GIVES THE ERROR: https://d.godbolt.org/z/8rNdjJ
 import std.stdio;
 bool test(){
      static if(cond){
          return true;
      }
      return false;
 }
 
 enum bool cond = true;
 bool func();
 
 void main(string[ ] args) {
      writeln(test());
 }
 
 As you can see, when you explicitly define a return inside a 
 static if, the compiler emits the warning.


 Right, the first would still include the return false as 
 generated code, this is not in dispute. And in fact, in your 
 case, I would say the error is legitimate (that line is not 
 reachable).


Yes I got what you mean, and I would agree with that logic too, 
except that while this gives a warning:

bool test(){
     static if(cond){
         return true;
     }
     return false;
}

Just adding "else" to the code above will make it work normally:

bool test(){
     static if(cond){
         return true;
     }else
         return false;
}

This is a bit weird for me, I really thought the compiler would 
be smart enough to handle this.

If I was doing a benchmark between functions I'd like to do:

void func(){
     static if(TryDoSomethingA){
         doSomethingA();
         return;
     }
     doSomethingB();
}

But to make this work I'll need to add an extra "else", which I 
hate and I always try to avoid in my code.

Well my background is C and maybe there is a reason for the use 
of "else" with "static if".


 But in the case of a template where the condition is passed in, 
 the line is reachable if the condition is one way.

 However, there are other reasons why this might be "reachable" 
 in other cases. For example, you could have:

 version(abc)
    enum cond = false;
 else
    enum cond = true;

 which means that if you *compile* it a certain way, then it's 
 reachable.


Exactly and I agree.

Matheus.
Apr 30 2020
parent Steven Schveighoffer <schveiguy gmail.com> writes:
On 4/30/20 2:25 PM, matheus wrote:

 On Thursday, 30 April 2020 at 14:29:27 UTC, Steven Schveighoffer wrote:

 On 4/30/20 10:02 AM, matheus wrote:

 ...
 // WHILE BELOW GIVES THE ERROR: https://d.godbolt.org/z/8rNdjJ
 import std.stdio;
 bool test(){
      static if(cond){
          return true;
      }
      return false;
 }

 enum bool cond = true;
 bool func();

 void main(string[ ] args) {
      writeln(test());
 }

 As you can see, when you explicitly define a return inside a static 
 if, the compiler emits the warning.


 Right, the first would still include the return false as generated 
 code, this is not in dispute. And in fact, in your case, I would say 
 the error is legitimate (that line is not reachable).


 
 Yes I got what you mean, and I would agree with that logic too, except 
 that while this gives a warning:
 
 bool test(){
      static if(cond){
          return true;
      }
      return false;
 }
 
 Just adding "else" to the code above will make it work normally:
 
 bool test(){
      static if(cond){
          return true;
      }else
          return false;
 }
 
 This is a bit weird for me, I really thought the compiler would be smart 
 enough to handle this.


So what is happening is that the static if is literally including or 
trimming out the code based on the boolean.

It's as if you wrote:

bool test() {
    // static if(cond) { => included for cond == true
       return true;
    // }
    return false;
}

which the "linter" or whatever generates the warning looks at and says 
it's no good, because it looks like 2 sequential return statements.

What it really should do is simply remove the second return and shut up ;)


 
 If I was doing a benchmark between functions I'd like to do:
 
 void func(){
      static if(TryDoSomethingA){
          doSomethingA();
          return;
      }
      doSomethingB();
 }
 
 But to make this work I'll need to add an extra "else", which I hate and 
 I always try to avoid in my code.


There is one possibility, which is only valid for static conditionals:

void func() {
    static if(cond)
      return;
    else: // note the colon
    return; // note the indentation
}

Basically, it's saying, do everything else in this scope, but don't make 
me indent. It could be a simple way to fix the problem. I think Andrei 
suggested this in the bug report quoted before. Essentially, the 
compiler could insert an else: whenever it encounters a static if 
without an else, where all paths exit the outer scope for that compilation.

But the compiler should be smart enough to just not report errors in 
these cases.


 
 Well my background is C and maybe there is a reason for the use of 
 "else" with "static if".


Yes, if you consider the AST rewriting that static if does, it makes 
sense, because the linter doesn't know how the AST came about.

-Steve
Apr 30 2020
prev sibling parent Steven Schveighoffer <schveiguy gmail.com> writes:
On 4/30/20 5:10 AM, Timon Gehr wrote:

 On 30.04.20 08:00, Walter Bright wrote:

 Evidently, your example must be different from this. Please post a 
 complete example.


 
 You need -W. -W also notices the dangling elses, the intended code is this:


I didn't even know that, I was just building phobos, which probably 
includes the -W.

Thanks.

-Steve
Apr 30 2020