D issues are now tracked on GitHub. This Bugzilla instance remains as a read-only archive.
Issue 1931 - dmd doesn't enforce users to use assert(0) for noreturn func
Summary: dmd doesn't enforce users to use assert(0) for noreturn func
Status: RESOLVED FIXED
Alias: None
Product: D
Classification: Unclassified
Component: dmd (show other issues)
Version: D1 (retired)
Hardware: x86 Windows
: P2 enhancement
Assignee: Walter Bright
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-03-18 21:59 UTC by david
Modified: 2014-02-24 15:30 UTC (History)
3 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this issue.
Description david 2008-03-18 21:59:03 UTC
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.
Comment 1 david 2008-03-18 22:01:28 UTC
this should be possibly a regression. And it's a very serious flow analysis problem.
Comment 2 Walter Bright 2008-04-23 02:32:12 UTC
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.
Comment 3 david 2008-04-23 05:47:18 UTC
why not issue a compile error?
Comment 4 Bill Baxter 2008-04-23 06:10:47 UTC
(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".
Comment 5 david 2008-04-24 02:20:16 UTC
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.
Comment 6 torhu 2008-04-24 09:52:38 UTC
(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
Comment 7 david 2008-04-25 01:14:52 UTC
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!!
}
Comment 8 Janice Caron 2008-04-25 01:40:25 UTC
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.

Comment 9 david 2008-04-25 02:47:29 UTC
umm the bug have no longer got the meaning of the old title and keyword. let me fix it.
Comment 10 Stewart Gordon 2008-11-20 20:55:01 UTC
Technically, this _is_ an enhancement request.  A major enhancement request IYO maybe, but still an enhancement request.  The other severities are intended for bugs.
Comment 11 Don 2010-01-15 04:46:41 UTC
This was implemented in DMD2.031.