Bug 24021 – Issue a warning on assert with side effects

Status
REOPENED
Severity
enhancement
Priority
P1
Component
dmd
Product
D
Version
D2
Platform
All
OS
All
Creation time
2023-06-28T12:29:03Z
Last change time
2024-12-13T19:30:00Z
Assigned to
No Owner
Creator
Grim Maple
Moved to GitHub: dmd#20307 →

Comments

Comment #0 by grimmaple95 — 2023-06-28T12:29:03Z
Consider this code: ``` bool fn(out int a) { a = 10; return true; } void main() { int a; assert(fn(a)); writeln(a); } ``` This code will behave differently in Release and Debug builds. I suggest issuing a warning for the user, when caling functions with side-effects inside an assertion. I'm aware of 6074, and I'm not talking about changing how asserts work, but it would be great to have a warning about this.
Comment #1 by razvan.nitu1305 — 2023-06-28T13:03:58Z
Note: Walter is against issuing warnings. The compiler should either accept or reject your code. A warning is something vague that programmers usually ignore (against what best practices advice). In the last 7 years there were exactly 0 warning added to the compiler. With that being said, there is no way such code is ever going to be an error. That would mean that any function that receives an out parameter (or a pointer/ref parameter) would have to issue an error if used in an assert. That is very restrictive. Even if it were issued as a measly warning, I don't really see the usefulness of it. Asserts have the purpose to test for a specific condition, in this particular case, that a function returns a specific value. Why would a warning be issued if a function receives a pointer parameter? What's the actionable item for this message? Stop using the assert?
Comment #2 by razvan.nitu1305 — 2023-06-28T13:20:52Z
This could be implemented by a third party tool using dmd as a library. But I don't think it should be implemented in the compiler.
Comment #3 by nick — 2023-06-28T16:39:20Z
Also asserts are used in unit tests where having side effects is often deliberate.
Comment #4 by maxhaton — 2023-06-28T18:21:12Z
Pretty sure warnings have been added.
Comment #5 by tim.dlang — 2023-06-28T19:54:40Z
This seems to be a duplicate of issue 12028.
Comment #6 by b2.temp — 2023-06-29T08:21:45Z
*** This issue has been marked as a duplicate of issue 12028 ***
Comment #7 by grimmaple95 — 2023-06-29T13:27:27Z
With all due respect, this isn't a _duplicate_ of 12028. I'm not proposing to disallow side effects, I'm proposing to notify user that their code was not compiled. I think this warning should be issued in release builds only, as they are the only one affected. This will not affect unittests or bring other inconveniences. Besides, maybe, rewriting code from `assert(foo())` to `auto tmp = foo(); assert(tmp);`
Comment #8 by b2.temp — 2023-06-29T17:03:40Z
indeed, subtle difference.
Comment #9 by robert.schadek — 2024-12-13T19:30:00Z
THIS ISSUE HAS BEEN MOVED TO GITHUB https://github.com/dlang/dmd/issues/20307 DO NOT COMMENT HERE ANYMORE, NOBODY WILL SEE IT, THIS ISSUE HAS BEEN MOVED TO GITHUB