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

Not fix, just an other way - instead of final `return false;` (not tested):

static if(!condA || !condB || !NEW_CONDITION) return false;

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.

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.

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();

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.

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

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

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.

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