Thread overview
[Issue 15708] std.range.choose assumes hasElaborateCopyConstructor means "has __postblit"
Jun 11, 2017
Johannes Loher
Jun 23, 2017
ZombineDev
Dec 17, 2022
Iain Buclaw
Apr 20, 2023
FeepingCreature
Mar 03
Dlang Bot
Mar 03
Dlang Bot
June 11, 2017
https://issues.dlang.org/show_bug.cgi?id=15708

Johannes Loher <johannes.loher@fg4f.de> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |johannes.loher@fg4f.de

--- Comment #1 from Johannes Loher <johannes.loher@fg4f.de> ---
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?

--
June 23, 2017
https://issues.dlang.org/show_bug.cgi?id=15708

ZombineDev <petar.p.kirov@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |petar.p.kirov@gmail.com

--
December 17, 2022
https://issues.dlang.org/show_bug.cgi?id=15708

Iain Buclaw <ibuclaw@gdcproject.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P1                          |P3

--
April 20, 2023
https://issues.dlang.org/show_bug.cgi?id=15708

FeepingCreature <default_357-line@yahoo.de> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |default_357-line@yahoo.de

--- Comment #2 from FeepingCreature <default_357-line@yahoo.de> ---
Just ran into this. Makes `choose` pretty unusable.

--
March 03
https://issues.dlang.org/show_bug.cgi?id=15708

Dlang Bot <dlang-bot@dlang.rocks> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |pull

--- Comment #3 from Dlang Bot <dlang-bot@dlang.rocks> ---
@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

--
March 03
https://issues.dlang.org/show_bug.cgi?id=15708

Dlang Bot <dlang-bot@dlang.rocks> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--- Comment #4 from Dlang Bot <dlang-bot@dlang.rocks> ---
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

--