Bug 4789 – std.algorithm.sort bug

Status
RESOLVED
Resolution
DUPLICATE
Severity
normal
Priority
P2
Component
dmd
Product
D
Version
D2
Platform
Other
OS
Windows
Creation time
2010-09-02T19:39:00Z
Last change time
2012-08-05T15:38:10Z
Assigned to
nobody
Creator
soarowl

Comments

Comment #0 by soarowl — 2010-09-02T19:39:54Z
private import std.algorithm, std.stdio; void main() { int[2][] a; a ~= [1, 2]; a ~= [2, 3]; a ~= [3, 4]; a ~= [1, 1]; std.algorithm.sort(a); std.stdio.writefln("%s", a); } When I compile without any arguments, the runtime has an "object.Exception: overlapping array copy" exception; But when I compile with -release or -release -inline arguments, the result is correct.
Comment #1 by dsimcha — 2010-09-02T20:52:24Z
Here's the swap algorithm used in the int[] instantiation in std.algorithm: auto t = a; a = b; b = t; This function is broken for static arrays if a and b are the same array, and in your example, swap() gets called with the rhs and lhs being the same array. This is probably related to using optimized memcpy() routines under the hood for copying static arrays. The language should arguably do the right thing when a static array is assigned to itself instead of this kind of crazy behavior. In the mean time, I've inserted a trivial check into swap() that only gets compiled in for static arrays. It's a kludge, but it's a trivial, well-encapsulated kludge and can be removed when the deeper issue gets solved. http://dsource.org/projects/phobos/changeset/1948
Comment #2 by code — 2011-01-11T05:44:43Z
import std.traits : hasElaborateAssign; import std.algorithm : swap; struct Elem { ~this() {} } static assert(hasElaborateAssign!Elem); void main() { auto elem = Elem(); swap(elem, elem); } ---- This bug still exists for structs with elaborate assign. It throws an "object.Exception: overlapping array copy". Before changeset 1948 the implementation used memcpy which would have also led to undefined behavior on some platforms. Now swap uses _d_arraycopy which throws the overlapping exception. Proposed fix is to add an "if (lhs !is rhs)" around the struct copy code.
Comment #3 by code — 2011-01-11T05:50:48Z
> Before changeset 1948 the implementation used memcpy which would have also led > to undefined behavior on some platforms. This was changeset 2180 that changed the implementation. But as stated is essentially broken in both versions.
Comment #4 by code — 2011-06-18T20:07:44Z
*** This issue has been marked as a duplicate of issue 5705 ***
Comment #5 by github-bugzilla — 2012-08-05T15:38:10Z
Commits pushed to master at https://github.com/D-Programming-Language/phobos https://github.com/D-Programming-Language/phobos/commit/8881a31541e05a494703ed276777afe0fedf2096 Workaround for bug# 4789 removed. The "workaround" for bug# 4789 was correct, because it _is_ illegal to assign overlapping arrays, which assigning a static array to itself does. https://github.com/D-Programming-Language/phobos/commit/8cddf7d1769b1dbcf8f646a7cb4c9e19157cad83 Merge pull request #682 from jmdavis/swap Workaround for bug# 4789 removed.