Bug 6074 – Assert expressions shouldn't have side effects

Status
RESOLVED
Resolution
INVALID
Severity
normal
Priority
P2
Component
dmd
Product
D
Version
D2
Platform
Other
OS
All
Creation time
2011-05-29T08:01:00Z
Last change time
2011-05-30T01:34:52Z
Assigned to
nobody
Creator
simendsjo

Comments

Comment #0 by simendsjo — 2011-05-29T08:01:34Z
http://www.digitalmars.com/d/2.0/expression.html#AssertExpression says "It is an error if the expression contains any side effects that the program depends on" This is not enforced by the compiler though: module assert_sideeffect; bool b; bool f() { b = !b; return b; } void main() { assert(f()); // oops.. changes b in debug mode if(!b) { // true only in release assert(0); } } dmd -g -w -wi -debug -run assert_sideeffect // no output dmd -g -w -wi -release -run assert_sideeffect object.Error: assert(0) or HLT instruction Bearophile suggested: "In D there are pure functions, so I think it's not too much hard for it to tell apart when the contents of an assert() are pure or not. My opinion is that the D compiler has to enforce purity inside assert(), to avoid bugs. "
Comment #1 by kennytm — 2011-05-29T08:33:31Z
In the current stage, allowing only 'pure' function inside an 'assert' is impractical, e.g. you can't use std.algorithm.equal.
Comment #2 by simendsjo — 2011-05-29T08:44:37Z
(In reply to comment #1) > In the current stage, allowing only 'pure' function inside an 'assert' is > impractical, e.g. you can't use std.algorithm.equal. Can't equal be pure when not using a closure? I would think having side effects in asserts is always bad. If it's difficult to implement, at least the documentation should be changed.
Comment #3 by kennytm — 2011-05-29T09:40:03Z
(In reply to comment #2) > (In reply to comment #1) > > In the current stage, allowing only 'pure' function inside an 'assert' is > > impractical, e.g. you can't use std.algorithm.equal. > > Can't equal be pure when not using a closure? I would think having side effects > in asserts is always bad. If it's difficult to implement, at least the > documentation should be changed. Since 'pure' is transitive, if 'equal' needs to be pure, all range primitives (.front, .popFront, .empty) it depends on needs to be pure as well, as then the requirement propagates to all other ranges (map, filter, iota, zip, ...). This is a very huge change. This proposal is practical only when there is a working 'auto pure' implementation, which I don't think will be included in D2 as the syntax is pretty much frozen.
Comment #4 by bugzilla — 2011-05-29T11:53:22Z
This is as designed. Sometimes, checking code may have side effects, but it is up to the user to ensure that they do not affect the program. Forcing the assert expression to be pure is too restrictive. Not a bug.
Comment #5 by bearophile_hugs — 2011-05-29T13:17:29Z
(In reply to comment #4) > This is as designed. Sometimes, checking code may have side effects, but it is > up to the user to ensure that they do not affect the program. Forcing the > assert expression to be pure is too restrictive. > > Not a bug. A reminder: forbidding side effects in asserts is useful for static analyzability of the asserts. Languages that take Contracts seriously don't allow generic code in Contracts right to allow a simpler analyzability. They even define a specific expression language for this purpose.
Comment #6 by kennytm — 2011-05-29T14:01:56Z
(In reply to comment #5) > (In reply to comment #4) > > This is as designed. Sometimes, checking code may have side effects, but it is > > up to the user to ensure that they do not affect the program. Forcing the > > assert expression to be pure is too restrictive. > > > > Not a bug. > > A reminder: forbidding side effects in asserts is useful for static > analyzability of the asserts. Languages that take Contracts seriously don't > allow generic code in Contracts right to allow a simpler analyzability. They > even define a specific expression language for this purpose. Purity is not necessary nor sufficient (in the current stage) for 'static analyzability' (CTFE) in D. Also, unit tests are used much more than DbC, where accepting an impure predicate in assert is perfectly acceptable (e.g. testing a mmap module). It's possible to enforce 'assert' purity only in 'in', 'out' and 'invariant' blocks, but that create a special case. ;)
Comment #7 by issues.dlang — 2011-05-29T15:09:28Z
Requiring purity in asserts would be completely unacceptable in unit tests. You would have to constantly save the results of expressions and then tests them rather than testing them directly. You could end up doubly the length of a typical unit tests. In some cases, it would likely even be highly annoying in normal assertions in normal code. It's _far_ too easy for a function to not be able to be pure for requiring purity in assertions to be practical. Even if/when we had/have conditional purity, there's still plenty of stuff which doesn't really have side effects which can't be pure due to making a system call or doing something else which just can't quite be pure in spite of the lack of side effects. The documentation on the site should be fixed to so that it doesn't claim that it's illegal to have an expression with a side effect in an assertion rather than "fixing" assertions so that they can't have side effects. Warning the programmer about the risk of doing so is wise, but making it so that they can't is not.
Comment #8 by bugzilla — 2011-05-30T01:34:52Z
(In reply to comment #5) > A reminder: forbidding side effects in asserts is useful for static > analyzability of the asserts. Languages that take Contracts seriously don't > allow generic code in Contracts right to allow a simpler analyzability. They > even define a specific expression language for this purpose. It is not necessary to disallow impure asserts to do static analysis. Nor is it an issue of taking asserts "seriously" or not. BTW, the optimizer already does quite a bit of static analysis. That's what optimizers do. Of course an optimizer doesn't require everything to be pure. It would be a fairly useless one if it did.