Bug 15862 – Functions that return types with mutable indirections should be weakly pure, not strongly pure

dmd thinks functions are strongly pure despite mutable indirections in the return type ---- int* p() pure nothrow {return new int;} int[] a() pure nothrow {return [0];} Object o() pure nothrow {return new Object;} void main() { int* p1 = p(); int* p2 = p(); if (p1 is p2) throw new Exception("pointers same"); int[] a1 = a(); int[] a2 = a(); if (a1 is a2) throw new Exception("arrays same"); Object o1 = o(); Object o2 = o(); if (o1 is o2) throw new Exception("objects same"); } ---- No exceptions should be thrown. All is fine when compiled without optimization flags. When compiled with `dmd -O -release`, the exceptions are thrown. ldc does not seem to have the same issue.

This is very, very bad... Tried some things. Still fails: int[] a1 = a(); immutable(int)[] a2 = a(); if(a1 is a2) throw ...; changing a2 to immutable int[] seems to work. Goes back at least as far as 2.064 (my earliest installed compiler). I find it hard to believe we don't have widespread failures in Phobos due to this...

This is as designed and intended. 1. Mutable indirections in the return type do not affect purity determination. Mutability of the parameters does. 2. An early design decision was that calling new() inside a pure function is an acceptable special case. (Otherwise pure functions would be nearly useless.)

(In reply to Walter Bright from comment #2) > This is as designed and intended. > > 1. Mutable indirections in the return type do not affect purity > determination. Mutability of the parameters does. Sorry, but this is ridiculous. Reopening. Please explain how on earth the shown behavior is supposedly acceptable. > 2. An early design decision was that calling new() inside a pure function is > an acceptable special case. (Otherwise pure functions would be nearly > useless.) This is fine. It follows that the compiler must not consider the result reusable when a function has a mutable indirection in the return type, even when it's otherwise pure. A strongly pure function (with const parameters) can only return an int* by newly allocating it. And then reusing it is disastrous.

Proper test case: int[] a=f(); int[] b=f(); a[0]=0; b[0]=1; assert(a[0]==0 && b[0]==1);

(In reply to anonymous4 from comment #4) > Proper test case: > > int[] a=f(); > int[] b=f(); > a[0]=0; > b[0]=1; > assert(a[0]==0 && b[0]==1); You can't use asserts (except for assert(false)) to test this. The bug needs -release to occur, and -release disables asserts.

(In reply to Walter Bright from comment #2) > This is as designed and intended. > > 1. Mutable indirections in the return type do not affect purity > determination. Mutability of the parameters does. The function can still be strong-pure, in that the result will always be the same for the same parameters. You can elide the call, but the result must be copied -- it cannot be identical. That is, it is OK for the compiler to optimize by changing OP's code to this: int* p1 = p(); int* p2 = new int(*p1); if (p1 is p2) throw new Exception("pointers same"); int[] a1 = a(); int[] a2 = a1.dup; if (a1 is a2) throw new Exception("arrays same"); Object o1 = o(); Object o2 = deepCopy(o); if (o1 is o2) throw new Exception("objects same"); > 2. An early design decision was that calling new() inside a pure function is > an acceptable special case. (Otherwise pure functions would be nearly > useless.) Right, but all pure memory allocations should not return the same piece of memory!

Yeah, my mistake. The compiler should consider @nogc when marking a pure function as a common subexpression.

Hm, I liked the previous title better because "allocating storage" is not defined at language level (could come from malloc, specialized allocators etc) whereas "mutable indirections" are part of the language.

(In reply to Walter Bright from comment #7) > Yeah, my mistake. The compiler should consider @nogc when marking a pure > function as a common subexpression. Nononononono, @nogc has nothing to do with it. Think malloc and allocators. The compiler should consider functions that return data with mutable indirections weakly pure, regardless. Then they won't be subject to CSE and everything will work well.

(In reply to Walter Bright from comment #7) > Yeah, my mistake. The compiler should consider @nogc when marking a pure > function as a common subexpression. I think approaching this with @nogc would be a mistake. Considering mutable indirections in the return type seems superior to me. You can have a function that allocates via the GC but doesn't return any mutable indirections. Either it allocates for internal use only, or it casts to immutable when returning. You can reuse the result of such a function for another identical call. I'm only rehashing David Nadlinger's article on this matter, of course. http://klickverbot.at/blog/2012/05/purity-in-d/ Considering the indirections in the return type would also leave the door open for other allocators to be used in `pure` code. Tying this to @nogc would only add to the arbitrary divide between the GC and others. As far as I see, the GC isn't actually more pure than other allocators, it's just recognized by the language which makes it seem natural to special case it.

(In reply to Walter Bright from comment #11) > https://github.com/dlang/dmd/pull/6197 I did this before the other comments were posted. Have to think about this some more based on those comments.

(In reply to Walter Bright from comment #12) > (In reply to Walter Bright from comment #11) > > https://github.com/dlang/dmd/pull/6197 > > I did this before the other comments were posted. Have to think about this > some more based on those comments. Thanks for working on this. It seems we're getting close.

Commits pushed to master at https://github.com/dlang/dmd https://github.com/dlang/dmd/commit/4dfc9e9252220ef724412a74b3a8caf862d0a95e fix Issue 15862 - allocating storage in pure functions should not result in caching return values of them https://github.com/dlang/dmd/commit/ab9d712ad6b6a2f59e1e4427bf1627968fc65c8c Merge pull request #6197 from WalterBright/fix15862 fix Issue 15862 - allocating storage in pure functions should not res…

Commits pushed to scope at https://github.com/dlang/dmd https://github.com/dlang/dmd/commit/4dfc9e9252220ef724412a74b3a8caf862d0a95e fix Issue 15862 - allocating storage in pure functions should not result in caching return values of them https://github.com/dlang/dmd/commit/ab9d712ad6b6a2f59e1e4427bf1627968fc65c8c Merge pull request #6197 from WalterBright/fix15862

Commits pushed to newCTFE at https://github.com/dlang/dmd https://github.com/dlang/dmd/commit/4dfc9e9252220ef724412a74b3a8caf862d0a95e fix Issue 15862 - allocating storage in pure functions should not result in caching return values of them https://github.com/dlang/dmd/commit/ab9d712ad6b6a2f59e1e4427bf1627968fc65c8c Merge pull request #6197 from WalterBright/fix15862

Commits pushed to stable at https://github.com/dlang/dmd https://github.com/dlang/dmd/commit/4dfc9e9252220ef724412a74b3a8caf862d0a95e fix Issue 15862 - allocating storage in pure functions should not result in caching return values of them https://github.com/dlang/dmd/commit/ab9d712ad6b6a2f59e1e4427bf1627968fc65c8c Merge pull request #6197 from WalterBright/fix15862

dlang/phobos pull request #8509 "Remove uses of 'in' on extern(C) functions" was merged into master: - 501a3ab35a7b6f826fbf08f3f5eb2bcc96b71202 by Geod24: std.typecons: Remove workaround for fixed issue 15862 https://github.com/dlang/phobos/pull/8509