www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.learn - Solution to "statement is not reachable" depending on template

reply Johan Engelen <j j.nl> writes:
Hi all,
   I've found discussions, but not an actual "recommended" 
solution for the problem of "statement is not reachable" warnings 
in templates with early returns, e.g.:
```
bool nobool(T...)() {
     foreach (i, U; T) {
         static if (is(U == bool)) {
             return false;
         }
     }
     return true;  // emits "Warning: statement is not reachable"
}

bool nobool_nowarning(T...)() {
     bool retval = true;
     foreach (i, U; T) {
         static if (is(U == bool)) {
             retval = false;
         }
     }
     return retval;
}

void main() {
     static assert ( nobool_nowarning!(int,bool)() == false );
     static assert ( nobool_nowarning!(int,int)() == true );

     static assert ( nobool!(int,bool)() == false );
     static assert ( nobool!(int,int)() == true );
}
```
(I have heavily simplified the real-world code, please don't 
discuss alternative solutions to the "is(U==bool)" in particular. 
For sake of argument, assume that the predicate is a complicated 
beast.)

The `nobool` template prevents compilation with `-w`. Is 
`nobool_nowarning`, with the early return eradicated, the only 
acceptable solution? (with the hope that there will not be a 
"dead store" warning in the future...)
What if early returns cannot be avoided?

Thanks,
   Johan
Mar 16 2016
next sibling parent reply rikki cattermole <rikki cattermole.co.nz> writes:
On 16/03/16 11:18 PM, Johan Engelen wrote:

 Hi all,
    I've found discussions, but not an actual "recommended" solution for
 the problem of "statement is not reachable" warnings in templates with
 early returns, e.g.:
 ```
 bool nobool(T...)() {
      foreach (i, U; T) {
          static if (is(U == bool)) {
              return false;
          }
      }
      return true;  // emits "Warning: statement is not reachable"
 }

 bool nobool_nowarning(T...)() {
      bool retval = true;
      foreach (i, U; T) {
          static if (is(U == bool)) {
              retval = false;
          }
      }
      return retval;
 }

 void main() {
      static assert ( nobool_nowarning!(int,bool)() == false );
      static assert ( nobool_nowarning!(int,int)() == true );

      static assert ( nobool!(int,bool)() == false );
      static assert ( nobool!(int,int)() == true );
 }
 ```
 (I have heavily simplified the real-world code, please don't discuss
 alternative solutions to the "is(U==bool)" in particular. For sake of
 argument, assume that the predicate is a complicated beast.)

 The `nobool` template prevents compilation with `-w`. Is
 `nobool_nowarning`, with the early return eradicated, the only
 acceptable solution? (with the hope that there will not be a "dead
 store" warning in the future...)
 What if early returns cannot be avoided?

 Thanks,
    Johan


Change those static if's to just plain old ifs.

Imagine those foreach loops being unrolled and what that happens there 
isn't just one return there, imagine many many many of them and all of 
them valid.

---
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus
Mar 16 2016
next sibling parent reply Johan Engelen <j j.nl> writes:
On Wednesday, 16 March 2016 at 11:22:02 UTC, rikki cattermole 
wrote:

 Change those static if's to just plain old ifs.


But then this wouldn't compile, would it?
```
static if(__traits(compiles, __traits(getMember, a, "b"))) {
    return a.b;
}
```
(real code, I am not making this up)


 Imagine those foreach loops being unrolled and what that 
 happens there isn't just one return there, imagine many many 
 many of them and all of them valid.


Yep :-)

Perhaps the only solution is to be able to annotate the code to 
"shut up" DMD about it.
Mar 16 2016
parent rikki cattermole <rikki cattermole.co.nz> writes:
On 17/03/16 5:05 AM, Johan Engelen wrote:

 On Wednesday, 16 March 2016 at 11:22:02 UTC, rikki cattermole wrote:

 Change those static if's to just plain old ifs.


 But then this wouldn't compile, would it?
 ```
 static if(__traits(compiles, __traits(getMember, a, "b"))) {
     return a.b;
 }
 ```
 (real code, I am not making this up)


Hmm no.
If that's in a foreach as well, you'll need to solve it some other way.

---
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus
Mar 16 2016
prev sibling parent reply tsbockman <thomas.bockman gmail.com> writes:
On Wednesday, 16 March 2016 at 11:22:02 UTC, rikki cattermole 
wrote:

 Change those static if's to just plain old ifs.


This only works (sometimes) because D's value range propagation 
doesn't understand comparisons or normal if statements very well. 
This will hopefully be fixed sooner or later:
     https://github.com/D-Programming-Language/dmd/pull/1913
     https://github.com/D-Programming-Language/dmd/pull/5229

The only future-proof way to fix the "statement is not reachable" 
warning, is to guard the potentially unreachable code with a 
`static if` whose predicate precisely describes the circumstances 
in which it becomes unreachable...

...Which itself is a terrible solution that doesn't scale well at 
all to complex generic code and violates the "DRY" principle.

We really ought to just remove the warning. It just doesn't mesh 
well with D's super-powered meta-programming features.
Mar 16 2016
parent reply Steven Schveighoffer <schveiguy yahoo.com> writes:
On 3/16/16 9:48 PM, tsbockman wrote:

 On Wednesday, 16 March 2016 at 11:22:02 UTC, rikki cattermole wrote:

 Change those static if's to just plain old ifs.


 This only works (sometimes) because D's value range propagation doesn't
 understand comparisons or normal if statements very well. This will
 hopefully be fixed sooner or later:
      https://github.com/D-Programming-Language/dmd/pull/1913
      https://github.com/D-Programming-Language/dmd/pull/5229

 The only future-proof way to fix the "statement is not reachable"
 warning, is to guard the potentially unreachable code with a `static if`
 whose predicate precisely describes the circumstances in which it
 becomes unreachable...

 ....Which itself is a terrible solution that doesn't scale well at all
 to complex generic code and violates the "DRY" principle.

 We really ought to just remove the warning. It just doesn't mesh well
 with D's super-powered meta-programming features.


Yes. I agree. The way I look at it is that the code *is* reached in some 
cases, so it should compile (and just remove that section in that instance).

IMO any time a template value is used for branching, it should turn that 
warning off.

-Steve
Mar 17 2016
parent tsbockman <thomas.bockman gmail.com> writes:
On Thursday, 17 March 2016 at 17:12:07 UTC, Steven Schveighoffer 
wrote:

 Yes. I agree. The way I look at it is that the code *is* 
 reached in some cases, so it should compile (and just remove 
 that section in that instance).

 IMO any time a template value is used for branching, it should 
 turn that warning off.

 -Steve


That's what I think it should do, also. However, when we 
discussed it before, Daniel Murphy pretty much told me there is 
no practical way to actually implement that behavior in the 
compiler.

So, the next best thing is to just remove the warning entirely.
Mar 17 2016
prev sibling next sibling parent reply QAston <qaston gmail.com> writes:
On Wednesday, 16 March 2016 at 11:18:36 UTC, Johan Engelen wrote:

 Hi all,
   I've found discussions, but not an actual "recommended" 
 solution for the problem of "statement is not reachable" 
 warnings in templates with early returns, e.g.:
 ```
 bool nobool(T...)() {
     foreach (i, U; T) {
         static if (is(U == bool)) {
             return false;
         }
     }
     return true;  // emits "Warning: statement is not reachable"
 }

 [...]


On Wednesday, 16 March 2016 at 11:18:36 UTC, Johan Engelen wrote:
import std.meta;
template isBool(U)() = is(U == bool);
static if (!allSatisfy!(isBool, T)) {
     return true;  // no longer emits a warning
}

Something like this should work.
Mar 16 2016
parent reply Johan Engelen <j j.nl> writes:
On Wednesday, 16 March 2016 at 11:47:35 UTC, QAston wrote:

 
 import std.meta;
 template isBool(U)() = is(U == bool);
 static if (!allSatisfy!(isBool, T)) {
     return true;  // no longer emits a warning
 }

 Something like this should work.


Thanks, but:

On Wednesday, 16 March 2016 at 11:18:36 UTC, Johan Engelen wrote:

 (I have heavily simplified the real-world code, please don't 
 discuss alternative solutions to the "is(U==bool)" in 
 particular. For sake of argument, assume that the predicate is 
 a complicated beast.)
Mar 16 2016
parent QAston <qaston gmail.com> writes:
On Wednesday, 16 March 2016 at 17:08:20 UTC, Johan Engelen wrote:

 On Wednesday, 16 March 2016 at 11:47:35 UTC, QAston wrote:

 
 import std.meta;
 template isBool(U)() = is(U == bool);
 static if (!allSatisfy!(isBool, T)) {
     return true;  // no longer emits a warning
 }

 Something like this should work.


 Thanks, but:

 On Wednesday, 16 March 2016 at 11:18:36 UTC, Johan Engelen 
 wrote:

 (I have heavily simplified the real-world code, please don't 
 discuss alternative solutions to the "is(U==bool)" in 
 particular. For sake of argument, assume that the predicate is 
 a complicated beast.)




This method will work regarless of the predicate, just check if 
the predicate isn't matched for the whole array using 
allSatisfy/anySatisfy.
Mar 19 2016
prev sibling next sibling parent reply Steven Schveighoffer <schveiguy yahoo.com> writes:
On 3/16/16 7:18 AM, Johan Engelen wrote:

 Hi all,
    I've found discussions, but not an actual "recommended" solution for
 the problem of "statement is not reachable" warnings in templates with
 early returns, e.g.:
 ```
 bool nobool(T...)() {
      foreach (i, U; T) {
          static if (is(U == bool)) {
              return false;
          }
      }
      return true;  // emits "Warning: statement is not reachable"
 }


Instead of foreach, you could use recursive mechanism. Not ideal, but it 
would work.

Another possibility:

foreach(i, U; T) {
    static if(is(U == bool)) {
        return false;
    } else static if(i + 1 == T.length) {
        return true;
    }
}

-Steve
Mar 16 2016
parent Johan Engelen <j j.nl> writes:
On Wednesday, 16 March 2016 at 17:34:13 UTC, Steven Schveighoffer 
wrote:

 On 3/16/16 7:18 AM, Johan Engelen wrote:

 Hi all,
    I've found discussions, but not an actual "recommended" 
 solution for
 the problem of "statement is not reachable" warnings in 
 templates with
 early returns, e.g.:
 ```
 bool nobool(T...)() {
      foreach (i, U; T) {
          static if (is(U == bool)) {
              return false;
          }
      }
      return true;  // emits "Warning: statement is not 
 reachable"
 }


 Instead of foreach, you could use recursive mechanism. Not 
 ideal, but it would work.

 Another possibility:

 foreach(i, U; T) {
    static if(is(U == bool)) {
        return false;
    } else static if(i + 1 == T.length) {
        return true;
    }
 }


I like your second solution, it's clever :)
Mar 16 2016
prev sibling next sibling parent tsbockman <thomas.bockman gmail.com> writes:
On Wednesday, 16 March 2016 at 11:18:36 UTC, Johan Engelen wrote:

 Hi all,
   I've found discussions, but not an actual "recommended" 
 solution for the problem of "statement is not reachable" 
 warnings in templates with early returns...


"statement is not reachable" is fundamentally broken in D for 
generic code:
     https://issues.dlang.org/show_bug.cgi?id=14835

It's mostly not too big of a deal right now, only because the D 
front end's value range propagation capabilities are very weak.

Lionello Lunesu made a pull request a while back that majorly 
improves VRP in D as a part of his efforts to fix DMD issue 259. 
I decided to port it to DDMD a few months ago, and in the process 
discovered that with decent VRP, issue 14835 becomes 10-100x 
worse.

There was a forum discussion a while back as to what should be 
done about this, but unfortunately the conclusion was that it 
probably cannot really be fixed with any reasonable effort:
     
http://forum.dlang.org/thread/bnzfuekewfguvnwdznfk forum.dlang.org

As far as I can tell, the only reasonable way forward is to 
simply remove the warning entirely (or ban meaningful upgrades to 
VRP).
Mar 16 2016
prev sibling parent reply Walter Bright <newshound2 digitalmars.com> writes:
On 3/16/2016 4:18 AM, Johan Engelen wrote:

    I've found discussions, but not an actual "recommended" solution for the
 problem of "statement is not reachable" warnings in templates with early
 returns, e.g.:
 ```
 bool nobool(T...)() {
      foreach (i, U; T) {
          static if (is(U == bool)) {
              return false;
          }
      }
      return true;  // emits "Warning: statement is not reachable"
 }



bool nobool(T...)() {
      bool result = true;
      foreach (i, U; T) {
          static if (is(U == bool)) {
              result = false;
              break;
          }
          else
          {
              ...
          }
      }
      return result;  // emits "Warning: statement is not reachable"
}

Note that the optimizer will remove the dead loads.
Mar 31 2016
next sibling parent Johan Engelen <j j.nl> writes:
On Friday, 1 April 2016 at 01:21:32 UTC, Walter Bright wrote:

 On 3/16/2016 4:18 AM, Johan Engelen wrote:

    I've found discussions, but not an actual "recommended" 
 solution for the
 problem of "statement is not reachable" warnings in templates 
 with early
 returns, e.g.:
 ```
 bool nobool(T...)() {
      foreach (i, U; T) {
          static if (is(U == bool)) {
              return false;
          }
      }
      return true;  // emits "Warning: statement is not 
 reachable"
 }



 bool nobool(T...)() {
      bool result = true;
      foreach (i, U; T) {
          static if (is(U == bool)) {
              result = false;
              break;
          }
          else
          {
              ...
          }
      }
      return result;  // emits "Warning: statement is not 
 reachable"
 }


As you can see from my first post, that is the solution I used 
first  (without the "break;" addition), but it works until...


 Note that the optimizer will remove the dead loads.


... until someone adds a (imho useful) diagnostic of dead 
stores/loads. :-)
In that case, I guess inverting all bools and then inverting the 
return value at the end will work (using default initialization 
of result); but this is not possible in a slightly more 
complicated case with non-bool return types.

In another piece of code, the return type is a struct with an 
immutable member. That's when I gave up on working around the 
warning and just disabled it.
Simplified, something like this:

struct IMM {
     immutable bool result;
}
IMM immut(T...)() {
     IMM retval = IMM(true);
     foreach (i, U; T) {
         static if (is(U == bool)) {
             retval = IMM(false);  //  "Error: cannot modify 
struct retval IMM with immutable member".
         }
     }
     return retval;
}
Apr 01 2016
prev sibling parent reply ZombineDev <petar.p.kirov gmail.com> writes:
On Friday, 1 April 2016 at 01:21:32 UTC, Walter Bright wrote:

 On 3/16/2016 4:18 AM, Johan Engelen wrote:

    I've found discussions, but not an actual "recommended" 
 solution for the
 problem of "statement is not reachable" warnings in templates 
 with early
 returns, e.g.:
 ```
 bool nobool(T...)() {
      foreach (i, U; T) {
          static if (is(U == bool)) {
              return false;
          }
      }
      return true;  // emits "Warning: statement is not 
 reachable"
 }



 bool nobool(T...)() {
      bool result = true;
      foreach (i, U; T) {
          static if (is(U == bool)) {
              result = false;
              break;
          }
          else
          {
              ...
          }
      }
      return result;  // emits "Warning: statement is not 
 reachable"
 }

 Note that the optimizer will remove the dead loads.


Actually, I would expect every call of this fuction to be 
replaced by a boolean constant, since the result depends only on 
compile-time information.

What happened to "obvious code should be correct"? Can we try to 
find a solution that doesn't look like a hack? I don't think that 
we should ask users of the language to uglify their code just to 
workaround a deficiency in the compiler. Instead, we should 
improve the lowering of the static foreach construct so the 
information that it is syntactically a loop is preserved after 
loop unrolling.
Apr 01 2016
parent reply Johan Engelen <j j.nl> writes:
Reviving this thread to see whether anything has changed on the 
topic.

I now have this monster:
```
struct FMT {
   // has immutable members. FMT cannot be assigned to.
}

FMT monsterThatCompilerAccepts(T)(){
     alias TP = Tuple!(__traits(getAttributes, T));
     foreach(i, att; TP){
         static if( ... ) {
             return FMT( ... );
         }

         // Make sure we return a default value in the last 
iteration.
         // Prevents "statement not reachable" warning when placed 
here instead of outside the foreach.
         else static if (i + 1 == TP.length) {
             return FMT( ... );
         }
     }
     static if (TP.length == 0) {
         return FMT( ... );
     }
}

FMT codeThatIsUnderstandableButNotAccepted(T)(){
     alias TP = Tuple!(__traits(getAttributes, T));
     foreach(i, att; TP){
         static if( ... ) {
             return FMT( ... );
         }
     }
     return FMT( ... );
}
```

Thanks,
   Johan
Jun 18 2017
parent reply Steven Schveighoffer <schveiguy yahoo.com> writes:
On Sunday, 18 June 2017 at 09:28:57 UTC, Johan Engelen wrote:

 Reviving this thread to see whether anything has changed on the 
 topic.


If Timon gets static for each into the language, it can look a 
little better.

-Steve
Jun 18 2017
parent reply Johan Engelen <j j.nl> writes:
On Sunday, 18 June 2017 at 09:56:50 UTC, Steven Schveighoffer 
wrote:

 On Sunday, 18 June 2017 at 09:28:57 UTC, Johan Engelen wrote:

 Reviving this thread to see whether anything has changed on 
 the topic.


 If Timon gets static for each into the language, it can look a 
 little better.


Can you help me understand what you mean? How will it improve 
things? (static foreach would disable the "statement not 
reachable" analysis?)

-Johan
Jun 18 2017
next sibling parent Moritz Maxeiner <moritz ucworks.org> writes:
On Sunday, 18 June 2017 at 11:11:59 UTC, Johan Engelen wrote:

 On Sunday, 18 June 2017 at 09:56:50 UTC, Steven Schveighoffer 
 wrote:

 On Sunday, 18 June 2017 at 09:28:57 UTC, Johan Engelen wrote:

 Reviving this thread to see whether anything has changed on 
 the topic.


 If Timon gets static for each into the language, it can look a 
 little better.


 Can you help me understand what you mean? How will it improve 
 things? (static foreach would disable the "statement not 
 reachable" analysis?)


The new static foreach is AFAIK supposed to execute the loop 
instead of unrolling it, i.e. the expansion of

---
foreach (i, U; T) {
     static if (is(U == bool)) {
         return false;
     }
}
return true;
---

to

---
     static if (is(U1 == bool)) {
         return false;
     }
     static if (is(U2 == bool)) {
         return false;
     }
     ...
return true;
---

, which triggers that "statement is not reachable"  if at least 
one of U1,U2,... = T is bool, will not occur if you instead use 
`static foreach (i, U; T)`.
Jun 18 2017
prev sibling parent Steven Schveighoffer <schveiguy yahoo.com> writes:
On Sunday, 18 June 2017 at 11:11:59 UTC, Johan Engelen wrote:

 On Sunday, 18 June 2017 at 09:56:50 UTC, Steven Schveighoffer 
 wrote:

 On Sunday, 18 June 2017 at 09:28:57 UTC, Johan Engelen wrote:

 Reviving this thread to see whether anything has changed on 
 the topic.


 If Timon gets static for each into the language, it can look a 
 little better.


 Can you help me understand what you mean? How will it improve 
 things? (static foreach would disable the "statement not 
 reachable" analysis?)

 -Johan


On my phone so can't be too detailed, but you could do:

static foreach(i; 0..seq.length+1)
   static if(seq.length == i)
     return true;
   else static if(is(seq[i] == bool))
     return false;

Maybe looks a little better than the first workaround.

-Steve
Jun 18 2017