D issues are now tracked on GitHub. This Bugzilla instance remains as a read-only archive.
Issue 2423 - Erroneous unreachable statement warning
Summary: Erroneous unreachable statement warning
Status: RESOLVED FIXED
Alias: None
Product: D
Classification: Unclassified
Component: dmd (show other issues)
Version: D1 (retired)
Hardware: Other All
: P2 regression
Assignee: No Owner
URL:
Keywords: diagnostic, patch
Depends on:
Blocks:
 
Reported: 2008-10-20 02:55 UTC by Lars Ivar Igesund
Modified: 2014-03-01 00:37 UTC (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this issue.
Description Lars Ivar Igesund 2008-10-20 02:55:02 UTC
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.
Comment 1 Lars Ivar Igesund 2008-10-26 11:40:11 UTC
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.
Comment 2 Andrei Alexandrescu 2008-10-26 12:27:48 UTC
(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
Comment 3 Jarrett Billingsley 2008-10-26 12:52:46 UTC
(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.
Comment 4 Lars Ivar Igesund 2008-10-26 13:35:51 UTC
(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.
Comment 5 Stewart Gordon 2008-11-19 18:38:08 UTC
(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.
Comment 6 Don 2009-08-07 08:35:26 UTC
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;
}
Comment 7 Don 2009-10-21 06:39:20 UTC
Fixed DMD1.050 and DMD2.035.