D issues are now tracked on GitHub. This Bugzilla instance remains as a read-only archive.
Issue 6205 - Strongly-pure nothrow functions with ignored return value are entirely stripped even if it contains a failing 'assert'.
Summary: Strongly-pure nothrow functions with ignored return value are entirely stripp...
Status: RESOLVED FIXED
Alias: None
Product: D
Classification: Unclassified
Component: dmd (show other issues)
Version: D2
Hardware: Other Mac OS X
: P2 normal
Assignee: No Owner
URL:
Keywords: patch, wrong-code
: 6827 (view as issue list)
Depends on:
Blocks:
 
Reported: 2011-06-24 01:05 UTC by kennytm
Modified: 2012-01-09 02:07 UTC (History)
4 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this issue.
Description kennytm 2011-06-24 01:05:51 UTC
Test case:

---------------------------
int x() pure nothrow {
    assert(false, "1");
}

void main() {
    x();
}
---------------------------

This should throw an AssertError, but instead the generated program does nothing. The AssertError will be thrown if the return value of 'x' is assigned to some variable though. 

This bug (?) causes 'runnable/test41.d' to fail since commit 4c9661f as nothrow inference is also implemented, making 'imports.test41a.func' a strongly-pure nothrow function, and the 'assert' inside fails to run.

https://github.com/D-Programming-Language/dmd/commit/4c9661fa9fbd427909a334133dfc7f3869e47c31
Comment 1 Jonathan M Davis 2011-06-24 01:29:19 UTC
I'm not sure that this is a bug. It's a strongly pure function. It _is_ nothrow, which means that it won't throw any Exception, and its return value isn't used. assert is more of a debugging tool than anything. Sure, assert(false) sticks around in release mode, but still. Based on the purity and nothrow rules, this function can be optimized out of existance. I really don't see a problem with this. Now, assuming that is indeed the correct behavior, the obviously runnable/test41.d needs to be fixed, but it looks to me like having the call to x optimized out of existance makes perfect sense. And if the assert doesn't get hit, then it doesn't get hit. Asserts are intended primarily for debugging purposes. Yes, it's an assert(false) and not a normal assert, but still, if we start worrying about whether an assert would have killed a function or not, then we won't be able to optimize out functions like this, which wouldn't be good IMHO. Now, assuming that it's really only an issue when you have a strongly pure function where you throw away its return value, then maybe that's not a big deal, because that's bad code on the part of the programmer anyway, but I'm still inclined to think that it makes sense for x to never be called in this code (at least if optimizations are turned on).
Comment 2 kennytm 2011-06-24 02:06:12 UTC
Pull request for test41 if this is considered INVALID:

https://github.com/D-Programming-Language/dmd/pull/162
Comment 3 Don 2011-06-24 02:30:58 UTC
The bug is that the compiler's behaviour isn't consistent.
It's reasonable to optimize the function away in this case -- but then, it should generate an 'expression has no effect' warning. See bug 3882.

This test case is an excellent justification for treating 3882 as a bug, rather than an enhancement.
Comment 4 Kenji Hara 2012-01-04 03:07:56 UTC
I think this is 'too early optimization' bug.

Mechanism:
1. The calling of a function that is strong-pure and nothrow is 'no side effect'.
   Then dmd marks it in IR level.

   https://github.com/D-Programming-Language/dmd/blob/master/src/e2ir.c#L286

2. OPucallns and OPcallns are completely removed in backend optimizer level.

    https://github.com/D-Programming-Language/dmd/blob/master/src/backend/cgelem.c#L4088

    https://github.com/D-Programming-Language/dmd/blob/master/src/backend/cgelem.c#L4385

The mistaken is in #1. assert() has 'implicit side effect' (throw AssertError, or halt), so all of function call with enabling assertion should disable 'remove no side effect calling' optimization.

At least, a compilation without -O switch should not remove such calls.
Comment 5 Kenji Hara 2012-01-04 04:24:59 UTC
https://github.com/D-Programming-Language/dmd/pull/607

Only `-O -release` specification ignite the optimization.
Comment 7 Kenji Hara 2012-01-09 02:07:12 UTC
*** Issue 6827 has been marked as a duplicate of this issue. ***