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
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
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
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
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
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
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
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
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
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