Bug 15708 – std.range.choose assumes hasElaborateCopyConstructor means "has __postblit"

Status
RESOLVED
Resolution
FIXED
Severity
normal
Priority
P3
Component
phobos
Product
D
Version
D2
Platform
x86_64
OS
Linux
Creation time
2016-02-20T14:32:57Z
Last change time
2024-03-03T23:44:06Z
Keywords
pull
Assigned to
No Owner
Creator
Ali Cehreli

Comments

Comment #0 by acehreli — 2016-02-20T14:32:57Z
The following program fails compilation: import std.stdio : stdin; import std.range: choose; void main() { ubyte[1][] secondRange; choose(true, stdin.byChunk(1), secondRange); } /usr/include/dmd/phobos/std/range/package.d(1296): Error: no property '__postblit' for type 'ByChunk', did you mean '__xpostblit'? Forum thread: http://forum.dlang.org/thread/[email protected] The reason is, std.range.choose has the following post-blit: static if (hasElaborateCopyConstructor!R1 || hasElaborateCopyConstructor!R2) this(this) { if (condition) { static if (hasElaborateCopyConstructor!R1) r1.__postblit(); } else { static if (hasElaborateCopyConstructor!R2) r2.__postblit(); } } However, hasElaborateCopyConstructor may be true even if there is no __postblit: template hasElaborateCopyConstructor(S) { static if(isStaticArray!S && S.length) { enum bool hasElaborateCopyConstructor = hasElaborateCopyConstructor!(typeof(S.init[0])); } else static if(is(S == struct)) { enum hasElaborateCopyConstructor = hasMember!(S, "__postblit") || anySatisfy!(.hasElaborateCopyConstructor, FieldTypeTuple!S); } else { enum bool hasElaborateCopyConstructor = false; } } Ali
Comment #1 by johannes.loher — 2017-06-11T10:57:41Z
After playing around a bit, I found out that the postblit defined in choose does not result in correct behaviour anyways. Consider the following example: import std.stdio : writeln; import std.range : choose, repeat; struct StructWithPostblit { this(this) { writeln("StructWithPostblit.__postblit() called"); } } struct MyRange { enum empty = false; enum front = 1; void popFront() {} StructWithPostblit structWithPostblit; this(this) { writeln("MyRange.__postblit() called"); } } void main() { MyRange r1; auto r2 = choose(true, r1, repeat(1)); writeln(); auto r3 = r2; } The programs output: StructWithPostblit.__postblit() called MyRange.__postblit() called StructWithPostblit.__postblit() called MyRange.__postblit() called StructWithPostblit.__postblit() called MyRange.__postblit() called MyRange.__postblit() called As you can see, when copying a std.range.choose.Result, the postblit of the chosen underlying range (if any) gets called, but postblits of members of that underlying range don't (they should!). I see 2 possible solutions to this (and the original issue): 1. Similarly, too how hasElaborateCopyconstructor works, recursively check for all members of of the underlying range, if they define __postblit(). If they do, call it. 2. Simply make std.range.choose.Result have 2 members R1 and R2 instead of only a buffer large enough to fit the larger one of those two. I am actually in favor of the second apporach. I suppose the reason for using a buffer was to save a little memory. But in my opinion, this is only a really smallbenefit and makes the constructor, the postblit (as we can see it's buggy right now..) and the destructor much more complicated for only very little benefit (saving the memory, that the smaller one of the 2 ranges would occupy). Using the second approach would probably also solve https://issues.dlang.org/show_bug.cgi?id=14660, as that one is also caused by the use of the void[] buffer. Any opinions on that?
Comment #2 by default_357-line — 2023-04-20T08:27:36Z
Just ran into this. Makes `choose` pretty unusable.
Comment #3 by dlang-bot — 2024-03-03T23:00:13Z
@pbackus created dlang/phobos pull request #8935 "std.range.choose: call payload postblit correctly" fixing this issue: - std.range.choose: call payload postblit correctly Fixes bugzilla issue 15708 https://github.com/dlang/phobos/pull/8935
Comment #4 by dlang-bot — 2024-03-03T23:44:06Z
dlang/phobos pull request #8935 "std.range.choose: call payload postblit correctly" was merged into master: - 32efa560184eedb7e384b42638ebc33d0553c964 by Paul Backus: std.range.choose: call payload postblit correctly Fixes bugzilla issue 15708 https://github.com/dlang/phobos/pull/8935