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