Bug 24827 – maxElement does not correctly handle types with opAssign

Strictly speaking, I'm pretty sure that the problem here is that Rebindable2 doesn't use emplaceCopy but instead uses =, but maxElement is the public symbol which exposes this issue, an example being --- void main() { import std.algorithm.searching : maxElement; auto arr = [S(19), S(2), S(145), S(7)]; assert(maxElement(arr) == S(145)); } struct S { int i; bool destroyed; this(int i) { this.i = i; } ~this() { destroyed = true; } bool opEquals()(auto ref S rhs) { return this.i == rhs.i; } int opCmp()(auto ref S rhs) { if(this.i < rhs.i) return -1; return this.i == rhs.i ? 0 : 1; } invariant { assert(!destroyed); } } --- This results is the assertion in the invariant failing a bunch of times.

@jmdavis created dlang/phobos pull request #9067 "Fix Bugzilla issue 24827: maxElement does not handle opAssign correctly." fixing this issue: - Fix Bugzilla issue 24827: maxElement does not handle opAssign correctly. Rebindable2 did not handle types with opAssign correctly, which affected both minElement and maxElement. Namely, Rebindable2 assigned to memory which was not properly initialized when the correct solution in such a situation is to use copyEmplace. Assignment works when assignment is just a memcpy, but in the general case, opAssign needs to have a properly initialized object in order to work correctly. copyEmplace instead copies the object and then places the copy into the unitialized memory, so it avoids assigning to uninitialized memory. This commit also adds additional tests for types with destructors (which do get opAssign automatically) and types with postblit constructors or copy constructors to try to ensure that the code is doing the correct thing in those cases with regards to copying, assignment, and destruction. https://issues.dlang.org/show_bug.cgi?id=24829 was found in the process, and this does not fix that. Namely, types which cannot be assigned to and which also have a postblit constructor or copy constructor do not get copied correctly. So, among the tests added here are commented out tests for that case, since they're an altered version of some of the enabled tests. However, fixing that issue would be involved enough that I'm not attempting to fix it at this time. https://github.com/dlang/phobos/pull/9067

dlang/phobos pull request #9067 "Fix Bugzilla issue 24827: maxElement does not handle opAssign correctly." was merged into stable: - 33fde0d1bbbd3585c574c52a7cfb0cafaa63a010 by Jonathan M Davis: Fix Bugzilla issue 24827: maxElement does not handle opAssign correctly. Rebindable2 did not handle types with opAssign correctly, which affected both minElement and maxElement. Namely, Rebindable2 assigned to memory which was not properly initialized when the correct solution in such a situation is to use copyEmplace. Assignment works when assignment is just a memcpy, but in the general case, opAssign needs to have a properly initialized object in order to work correctly. copyEmplace instead copies the object and then places the copy into the unitialized memory, so it avoids assigning to uninitialized memory. This commit also adds additional tests for types with destructors (which do get opAssign automatically) and types with postblit constructors or copy constructors to try to ensure that the code is doing the correct thing in those cases with regards to copying, assignment, and destruction. https://issues.dlang.org/show_bug.cgi?id=24829 was found in the process, and this does not fix that. Namely, types which cannot be assigned to and which also have a postblit constructor or copy constructor do not get copied correctly. So, among the tests added here are commented out tests for that case, since they're an altered version of some of the enabled tests. However, fixing that issue would be involved enough that I'm not attempting to fix it at this time. https://github.com/dlang/phobos/pull/9067

dlang/phobos pull request #9086 "Merge stable" was merged into master: - f0c3e4a66b68d766c7601d9571aa1bf65a5c371e by Jonathan M Davis: Fix Bugzilla issue 24827: maxElement does not handle opAssign correctly. (#9067) Rebindable2 did not handle types with opAssign correctly, which affected both minElement and maxElement. Namely, Rebindable2 assigned to memory which was not properly initialized when the correct solution in such a situation is to use copyEmplace. Assignment works when assignment is just a memcpy, but in the general case, opAssign needs to have a properly initialized object in order to work correctly. copyEmplace instead copies the object and then places the copy into the unitialized memory, so it avoids assigning to uninitialized memory. This commit also adds additional tests for types with destructors (which do get opAssign automatically) and types with postblit constructors or copy constructors to try to ensure that the code is doing the correct thing in those cases with regards to copying, assignment, and destruction. https://issues.dlang.org/show_bug.cgi?id=24829 was found in the process, and this does not fix that. Namely, types which cannot be assigned to and which also have a postblit constructor or copy constructor do not get copied correctly. So, among the tests added here are commented out tests for that case, since they're an altered version of some of the enabled tests. However, fixing that issue would be involved enough that I'm not attempting to fix it at this time. https://github.com/dlang/phobos/pull/9086