Partially solve issue 18161. splitWhen and chunkBy when using forward ranges is now safe

This commit is contained in:
Ate Eskola 2021-08-23 22:34:35 +03:00
parent f96f8046d9
commit 41355cd4e3

View file

@ -2029,23 +2029,37 @@ private struct ChunkByGroup(alias eq, Range, bool eqEquivalenceAssured)
}
private Range current;
private RefCounted!(OuterRange) mothership;
// using union prevents RefCounted destructor from propagating @system to
// user code
union { private RefCounted!(OuterRange) mothership; }
private @trusted ref cargo() { return mothership.refCountedPayload; }
this(RefCounted!(OuterRange) origin)
private this(ref RefCounted!(OuterRange) origin)
{
groupNum = origin.groupNum;
current = origin.current.save;
() @trusted { mothership = origin; }();
groupNum = cargo.groupNum;
current = cargo.current.save;
assert(!current.empty, "Passed range 'r' must not be empty");
static if (eqEquivalenceAssured)
{
start = origin.current.save;
start = cargo.current.save;
// Check for reflexivity.
assert(eq(start.front, current.front),
"predicate is not reflexive");
}
}
mothership = origin;
// Cannot be a copy constructor due to issue 22239
this(this) @trusted
{
import core.lifetime : emplace;
auto temp = mothership;
mothership = temp;
// prevent decrementing reference count with brute force
() @trusted { emplace(&temp); }();
}
@property bool empty() { return groupNum == size_t.max; }
@ -2073,14 +2087,14 @@ private struct ChunkByGroup(alias eq, Range, bool eqEquivalenceAssured)
if (nowEmpty)
{
if (groupNum == mothership.groupNum)
if (groupNum == cargo.groupNum)
{
// If parent range hasn't moved on yet, help it along by
// saving location of start of next Group.
mothership.next = current.save;
cargo.next = current.save;
static if (!eqEquivalenceAssured)
{
mothership.nextUpdated = true;
cargo.nextUpdated = true;
}
}
@ -2094,6 +2108,11 @@ private struct ChunkByGroup(alias eq, Range, bool eqEquivalenceAssured)
copy.current = current.save;
return copy;
}
@trusted ~this()
{
mothership.destroy;
}
}
private enum GroupingOpType{binaryEquivalent, binaryAny, unary}
@ -2110,23 +2129,46 @@ if (isForwardRange!Range)
static assert(isForwardRange!InnerRange);
private RefCounted!OuterRange impl;
// using union prevents RefCounted destructor from propagating @system to
// user code
union { private RefCounted!OuterRange _impl; }
private @trusted ref impl() { return _impl; }
private @trusted ref implPL() { return _impl.refCountedPayload; }
this(Range r)
{
static if (eqEquivalenceAssured)
import core.lifetime : move;
auto savedR = r.save;
static if (eqEquivalenceAssured) () @trusted
{
impl = RefCounted!OuterRange(0, r, r.save);
}
else impl = RefCounted!OuterRange(0, r, r.save, false);
_impl = RefCounted!OuterRange(0, r, savedR.move);
}();
else () @trusted
{
_impl = RefCounted!OuterRange(0, r, savedR.move, false);
}();
}
@property bool empty() { return impl.current.empty; }
// Cannot be a copy constructor due to issue 22239
this(this) @trusted
{
import core.lifetime : emplace;
auto temp = _impl;
_impl = temp;
// prevent decrementing reference count with brute force
emplace(&temp);
}
@property bool empty() { return implPL.current.empty; }
static if (opType == GroupingOpType.unary) @property auto front()
{
import std.typecons : tuple;
return tuple(unaryFun!pred(impl.current.front), InnerRange(impl));
return tuple(unaryFun!pred(implPL.current.front), InnerRange(impl));
}
else @property auto front()
{
@ -2138,48 +2180,53 @@ if (isForwardRange!Range)
// Scan for next group. If we're lucky, one of our Groups would have
// already set .next to the start of the next group, in which case the
// loop is skipped.
while (!impl.next.empty && eq(impl.current.front, impl.next.front))
while (!implPL.next.empty && eq(implPL.current.front, implPL.next.front))
{
impl.next.popFront();
implPL.next.popFront();
}
impl.current = impl.next.save;
implPL.current = implPL.next.save;
// Indicate to any remaining Groups that we have moved on.
impl.groupNum++;
implPL.groupNum++;
}
else void popFront()
{
if (impl.nextUpdated)
if (implPL.nextUpdated)
{
impl.current = impl.next.save;
implPL.current = implPL.next.save;
}
else while (true)
{
auto prevElement = impl.current.front;
impl.current.popFront();
if (impl.current.empty) break;
if (!eq(prevElement, impl.current.front)) break;
auto prevElement = implPL.current.front;
implPL.current.popFront();
if (implPL.current.empty) break;
if (!eq(prevElement, implPL.current.front)) break;
}
impl.nextUpdated = false;
implPL.nextUpdated = false;
// Indicate to any remaining Groups that we have moved on.
impl.groupNum++;
implPL.groupNum++;
}
@property auto save()
{
// Note: the new copy of the range will be detached from any existing
// satellite Groups, and will not benefit from the .next acceleration.
return typeof(this)(impl.current.save);
return typeof(this)(implPL.current.save);
}
static assert(isForwardRange!(typeof(this)), typeof(this).stringof
~ " must be a forward range");
@trusted ~this()
{
_impl.destroy;
}
}
//Test for https://issues.dlang.org/show_bug.cgi?id=14909
@system unittest
@safe unittest
{
import std.algorithm.comparison : equal;
import std.typecons : tuple;
@ -2192,8 +2239,36 @@ if (isForwardRange!Range)
assert(u.equal!equal([[1],[2],[3]]));
}
//Testing inferring @system correctly
@safe unittest
{
struct DeadlySave
{
int front;
@safe void popFront(){front++;}
@safe bool empty(){return front >= 5;}
@system auto save(){return this;}
}
auto test1()
{
DeadlySave src;
return src.walkLength;
}
auto test2()
{
DeadlySave src;
return src.chunkBy!((a,b) => a % 2 == b % 2).walkLength;
}
static assert(isSafe!test1);
static assert(!isSafe!test2);
}
//Test for https://issues.dlang.org/show_bug.cgi?id=18751
@system unittest
@safe unittest
{
import std.algorithm.comparison : equal;
@ -2205,7 +2280,7 @@ if (isForwardRange!Range)
}
//Additional test for fix for issues 14909 and 18751
@system unittest
@safe unittest
{
import std.algorithm.comparison : equal;
auto v = [2,4,8,3,6,9,1,5,7];
@ -2329,7 +2404,7 @@ if (isInputRange!Range)
}
/// Showing usage with binary predicate:
/*FIXME: @safe*/ @system unittest
@safe unittest
{
import std.algorithm.comparison : equal;
@ -2356,7 +2431,7 @@ if (isInputRange!Range)
}
/// Showing usage with unary predicate:
/* FIXME: pure @safe nothrow*/ @system unittest
/* FIXME: pure nothrow*/ @safe unittest
{
import std.algorithm.comparison : equal;
import std.range.primitives;
@ -2755,7 +2830,7 @@ if (isInputRange!Range)
// https://issues.dlang.org/show_bug.cgi?id=13805
@system unittest
@safe unittest
{
[""].map!((s) => s).chunkBy!((x, y) => true);
}
@ -2790,9 +2865,8 @@ if (isForwardRange!Range)
return ChunkByImpl!(not!pred, not!pred, GroupingOpType.binaryAny, Range)(r);
}
//FIXME: these should be @safe
///
nothrow pure @system unittest
nothrow pure @safe unittest
{
import std.algorithm.comparison : equal;
import std.range : dropExactly;
@ -2816,7 +2890,7 @@ nothrow pure @system unittest
}
//ensure we don't iterate the underlying range twice
nothrow @system unittest
nothrow @safe unittest
{
import std.algorithm.comparison : equal;
import std.math.algebraic : abs;
@ -2827,7 +2901,7 @@ nothrow @system unittest
static int popfrontsSoFar;
auto front(){return elements[0];}
nothrow void popFront()
nothrow @safe void popFront()
{ popfrontsSoFar++;
elements = elements[1 .. $];
}
@ -2851,7 +2925,7 @@ nothrow @system unittest
}
// Issue 13595
@system unittest
@safe unittest
{
import std.algorithm.comparison : equal;
auto r = [1, 2, 3, 4, 5, 6, 7, 8, 9].splitWhen!((x, y) => ((x*y) % 3) > 0);
@ -2863,7 +2937,7 @@ nothrow @system unittest
]));
}
nothrow pure @system unittest
nothrow pure @safe unittest
{
// Grouping by maximum adjacent difference:
import std.math.algebraic : abs;