http://d.puremagic.com/issues/show_bug.cgi?id=1931

           Summary: dmd flow analysis error
           Product: D
           Version: 1.026
          Platform: PC
        OS/Version: Windows
            Status: NEW
          Keywords: accepts-invalid
          Severity: normal
          Priority: P2
         Component: DMD
        AssignedTo: bugzilla@digitalmars.com
        ReportedBy: davidl@126.com


int k;
bool setsomething()
{
    if (k==4)
        return false;

}
void main()
{
}

func setsomething should be banned because it not always returns bool in all flow.


-- 

http://d.puremagic.com/issues/show_bug.cgi?id=1931


davidl@126.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|normal                      |regression




------- Comment #1 from davidl@126.com  2008-03-18 22:01 -------
this should be possibly a regression. And it's a very serious flow analysis problem.


-- 

http://d.puremagic.com/issues/show_bug.cgi?id=1931


bugzilla@digitalmars.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |INVALID




------- Comment #2 from bugzilla@digitalmars.com  2008-04-23 02:32 -------
Actually, the compiler inserts a HALT instruction at the other return, so you'll never see the function "fall off the end" and return garbage. This is as intended.


-- 

http://d.puremagic.com/issues/show_bug.cgi?id=1931


davidl@126.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |REOPENED
         Resolution|INVALID                     |




------- Comment #3 from davidl@126.com  2008-04-23 05:47 -------
why not issue a compile error?


-- 

http://d.puremagic.com/issues/show_bug.cgi?id=1931


wbaxter@gmail.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|regression                  |enhancement




------- Comment #4 from wbaxter@gmail.com  2008-04-23 06:10 -------
(In reply to comment #3)
> why not issue a compile error?

It does in very simple cases, like if there is only one code path.

: noreturn.d(18): function noreturn.func expected to return a value of type int

So what you are asking for is to improve the escape analysis to cover more cases.  I think that's a hard problem in general.

Also "regression" means something used to work in a previous release but now doesn't.  But I don't think that's the case here.  Did this ever work?  If not this should technically be marked as "enhancement".


-- 

http://d.puremagic.com/issues/show_bug.cgi?id=1931





------- Comment #5 from davidl@126.com  2008-04-24 02:20 -------
I really thought it were working :D

I think I realize the problem now.

sometimes people would intend to have a func without an extra return statement when all paths return. Barking with an error makes people frustrated.

I hope if there's a way to specify this by some func qualifier.
like:
__built_in_no_return_check
it shouldn't look clean.. if people really want the func to have that return
statement optimized.


-- 

http://d.puremagic.com/issues/show_bug.cgi?id=1931


torhu@yahoo.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |torhu@yahoo.com




------- Comment #6 from torhu@yahoo.com  2008-04-24 09:52 -------
(In reply to comment #5)

Doesn't assert(0) have this effect?  You can put it in place of 'missing' return statements to tell the compiler that you know that control flow will never reach that location.

http://www.digitalmars.com/d/2.0/expression.html#AssertExpression http://www.digitalmars.com/d/2.0/statement.html#ReturnStatement


-- 

http://d.puremagic.com/issues/show_bug.cgi?id=1931


davidl@126.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|enhancement                 |major




------- Comment #7 from davidl@126.com  2008-04-25 01:14 -------
Since we have assert(0); then we should deprecate the current compiler
behavior.

Every such piece code *should be* written as following:
bool setsomething()
{
    if (k==4)
        return false;
    assert(0);        // developer should never ignore this line!!
}


-- 

On 25/04/2008, d-bugmail@puremagic.com <d-bugmail@puremagic.com> wrote:
>  Every such piece code *should be* written as following:
>  bool setsomething()
>  {
>     if (k==4)
>         return false;
>     assert(0);        // developer should never ignore this line!!
>  }

Usefully, that's exactly what happens if you use the -w command line switch.

I would certainly support raising this from a warning to an error. After being clobbered by this nasty crashing behavior once too often, I now /always/ compile with -w. But it pains me that every single D user* has to learn this lesson the hard way.

----
*or at least, every single /Windows/ D user. I'm told that falling off the end of a function on Linux is less crashy and more diagnostic than doing so on Windows, but not being a Linux user, I can't confirm that.

http://d.puremagic.com/issues/show_bug.cgi?id=1931


davidl@126.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|accepts-invalid             |
            Summary|dmd flow analysis error     |dmd doesn't enforce users to
                   |                            |use assert(0) for noreturn
                   |                            |func




------- Comment #9 from davidl@126.com  2008-04-25 02:47 -------
umm the bug have no longer got the meaning of the old title and keyword. let me fix it.


--