2423 – Erroneous unreachable statement warning

Consider the following function:
void foo() {
 do {
 if (false)
 return 1;
 } while (true);
}
Compiling with -w, results in
warning - whiletrue.d(6): Error: statement is not reachable
Minimized from a module in Tango, meaning Tango does not compile with warnings on.
This regression was introduced in DMD 1.032.

There is also a related issue that will show in the same function if that has a return value:
int foo() {
 do {
 return 1;
 } while (true);
}
warning - whiletrue.d(4): Error: statement is not reachable
warning - whiletrue.d(1): function whiletrue.foo no return at end of function
I understand that the "no return" is a semantic challenge, but the rule is fairly simple: if there is a while(true), then all code after it is dead code unless there is also a break. This is also a regression.
Note that whereas the first is an obvious bug and impossible to workaround, this one is possible to workaround, but still a question about quality of implementation. The "no return" bug does also affect/break Tango when using warnings.

(In reply to comment #1)
> There is also a related issue that will show in the same function if that has a
> return value:
> 
> int foo() {
> do {
> return 1;
> } while (true);
> }
> 
> warning - whiletrue.d(4): Error: statement is not reachable
> warning - whiletrue.d(1): function whiletrue.foo no return at end of function
> 
> I understand that the "no return" is a semantic challenge, but the rule is
> fairly simple: if there is a while(true), then all code after it is dead code
> unless there is also a break. This is also a regression.
> 
> Note that whereas the first is an obvious bug and impossible to workaround,
> this one is possible to workaround, but still a question about quality of
> implementation. The "no return" bug does also affect/break Tango when using
> warnings.
In this case there's no doubt a simple flow analysis will take care of things. The challenge is only when conditions are complex; in this case that doesn't matter. The code could as well be:
do {
 return 1;
} while (P == NP);
My explanation for the bug is that Walter's front-end rewrites loops with terminal test as loops with initial test with a jump:
do stmt while (cond); 
==>
goto __label; while (cond) __label: stmt
The rewritten form makes it a tad more difficult to figure out what's going on.
Andrei

(In reply to comment #0)
> Consider the following function:
> 
> void foo() {
> 
> do {
> if (false)
> return 1;
> } while (true);
> }
> 
> Compiling with -w, results in
> 
> warning - whiletrue.d(6): Error: statement is not reachable
> 
> Minimized from a module in Tango, meaning Tango does not compile with warnings
> on.
> 
> This regression was introduced in DMD 1.032.
> 
Erm, actually, I wonder if this is even valid. do-while loops in D do not require a semicolon at the end. The "unreachable statement" is simply the empty statement that follows the "while(true)". The following code:
int foo() {
 do {
 return 1;
 } while (true)
}
gives no warnings.

(In reply to comment #3)
> (In reply to comment #0)
> > Consider the following function:
> > 
> > void foo() {
> > 
> > do {
> > if (false)
> > return 1;
> > } while (true);
> > }
> > 
> > Compiling with -w, results in
> > 
> > warning - whiletrue.d(6): Error: statement is not reachable
> > 
> > Minimized from a module in Tango, meaning Tango does not compile with warnings
> > on.
> > 
> > This regression was introduced in DMD 1.032.
> > 
> 
> Erm, actually, I wonder if this is even valid. do-while loops in D do not
> require a semicolon at the end. The "unreachable statement" is simply the
> empty statement that follows the "while(true)". The following code:
> 
> int foo() {
> do {
> return 1;
> } while (true)
> }
> 
> gives no warnings.
> 
You are right. I won't decide whether there is still a bug in the compiler, but it is no longer a problem for Tango.

(In reply to comment #3)
> Erm, actually, I wonder if this is even valid. do-while loops in D do not
> require a semicolon at the end. The "unreachable statement" is simply the
> empty statement that follows the "while(true)". The following code:
It's valid, it just throws a warning. ISTM not having the semicolon at the end of DoStatement was a bad design decision - if you stumble upon
 }
 while (whatever)
 {
in the middle of some code, you have to look through possibly screenfuls of code to determine whether the while applies to the preceding block (and the following one just opens a new scope for whatever reason) or the following block.
Meanwhile, the conditions under which "statement is not reachable" is thrown ought to be changed to exclude empty statements.

To remove the warning from empty statements:
PATCH:
statement.c, line 564 (DMD2)
-	 if (!(result & BEfallthru) && !s->comeFrom())
+	 if (!(result & BEfallthru) && !s->comeFrom() && !s->isEmpty())
	 {
		s->warning("statement is not reachable");
	 }
And then add this line to ExpStatement, in statement.h line 140:
 virtual int isEmpty() { return exp==NULL; }
Side effect: This will make {;;;;;} an empty statement; at the moment, it isn't. The patch below makes the code below compile (into return 2;). Currently it won't compile, but works if try{;} is changed into try{}.
nothrow int main() {
 int x= 2;
 try { ; } catch(Exception e) { x=4; throw new Exception("xxx"); } 
 return x;
}

Fixed DMD1.050 and DMD2.035.