D issues are now tracked on GitHub. This Bugzilla instance remains as a read-only archive.
Issue 6074 - Assert expressions shouldn't have side effects
Summary: Assert expressions shouldn't have side effects
Status: RESOLVED INVALID
Alias: None
Product: D
Classification: Unclassified
Component: dmd (show other issues)
Version: D2
Hardware: Other All
: P2 normal
Assignee: No Owner
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-05-29 08:01 UTC by simendsjo
Modified: 2011-05-30 01:34 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 simendsjo 2011-05-29 08:01:34 UTC
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 kennytm 2011-05-29 08:33:31 UTC
In the current stage, allowing only 'pure' function inside an 'assert' is impractical, e.g. you can't use std.algorithm.equal.
Comment 2 simendsjo 2011-05-29 08:44:37 UTC
(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 kennytm 2011-05-29 09:40:03 UTC
(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 Walter Bright 2011-05-29 11:53:22 UTC
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 bearophile_hugs 2011-05-29 13:17:29 UTC
(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 kennytm 2011-05-29 14:01:56 UTC
(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 Jonathan M Davis 2011-05-29 15:09:28 UTC
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 Walter Bright 2011-05-30 01:34:52 UTC
(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.