Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Issue 24541 - cartesianProduct should have a length for finite ranges #10657

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
+80 −13
Open

Fix Issue 24541 - cartesianProduct should have a length for finite ranges #10657

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c6a0f97
Add length for cartesianProduct with two finite forward ranges
nicolo-mn Mar 2, 2025
d07e7a0
Add length for cartesianProduct with one forward range and one finite…
nicolo-mn Mar 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
93 changes: 80 additions & 13 deletions std/algorithm/setops.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,14 +115,20 @@ if (!allSatisfy!(isForwardRange, R1, R2) ||
else static if (isInputRange!R1 && isForwardRange!R2 && !isInfinite!R2)
{
import std.range : zip, repeat;
return joiner(map!((ElementType!R1 a) => zip(repeat(a), range2.save))
(range1));
import std.array : join;
auto pairs = map!((ElementType!R1 a) => zip(repeat(a), range2.save))
(range1);
static if (!isInfinite!R1) return join(pairs);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This appears to turn cartesianProduct from a lazy range into an eager range. Surely we should check if hasLength!R1 && hasLength!R2 (after taking care of infinite ranges) and compute that.

@CyberShadow do you have a better solution?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking of the same solution. Maybe we can use takeExactly to set the length if we can know it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A hypothetical assumeLength range wrapper might be even better (like assumeSorted).

else return joiner(pairs);
}
else static if (isInputRange!R2 && isForwardRange!R1 && !isInfinite!R1)
{
import std.range : zip, repeat;
return joiner(map!((ElementType!R2 a) => zip(range1.save, repeat(a)))
(range2));
import std.array : join;
auto pairs = map!((ElementType!R2 a) => zip(range1.save, repeat(a)))
(range2);
static if (!isInfinite!R2) return join(pairs);
else return joiner(pairs);
}
else static assert(0, "cartesianProduct involving finite ranges must "~
"have at least one finite forward range");
Expand Down Expand Up @@ -160,6 +166,22 @@ if (!allSatisfy!(isForwardRange, R1, R2) ||
{
assert(canFind(BC, tuple(n[0], n[1])));
}
assert(BC.length == 9);
}

@safe unittest
{
auto A = [ 1, 2, 3 ];
auto B = [ 4, 5, 6 ];
auto AB = cartesianProduct(A, B);
auto len = AB.length;

assert(len == 9);
for (auto i = 0; i < len; i++)
{
assert(AB.length == len - i);
AB.popFront();
}
}

@safe unittest
Expand Down Expand Up @@ -250,6 +272,7 @@ if (!allSatisfy!(isForwardRange, R1, R2) ||
}

// And therefore, by set comprehension, XY == Expected
assert(XY.length == Expected.length);
}

@safe unittest
Expand Down Expand Up @@ -335,6 +358,32 @@ if (!allSatisfy!(isForwardRange, R1, R2) ||
assert(equal(map!"[a[0],a[1]]"(NB.take(10)), [[0, 1], [0, 2], [0, 3],
[1, 1], [1, 2], [1, 3], [2, 1], [2, 2], [2, 3], [3, 1]]));

// Test fifth case: (finite) input range with finite forward range
static struct InpFiniteRangeWrapper
{
int[] data;
@property int front() { return data[0]; }
@property bool empty() { return data.length == 0; }
@property void popFront() { data = data[1 .. $]; }
}

auto X = InpFiniteRangeWrapper([0, 1, 2]);
auto XB = cartesianProduct(X, B);
foreach (n; [[0, 1], [0, 2], [0, 3], [1, 1],
[1, 2], [1, 3], [2, 1], [2, 2], [2, 3]])
{
assert(canFind(XB, tuple(n[0], n[1])));
}
assert(XB.length == 9);
auto Y = InpFiniteRangeWrapper([0, 1, 2]);
auto BY = cartesianProduct(B, Y);
foreach (n; [[1, 0], [2, 0], [3, 0], [1, 1],
[2, 1], [3, 1], [1, 2], [2, 2], [3, 2]])
{
assert(canFind(BY, tuple(n[0], n[1])));
}
assert(BY.length == 9);

// General finite range case
auto C = [ 4, 5, 6 ];
auto BC = cartesianProduct(B, C);
Expand All @@ -344,6 +393,7 @@ if (!allSatisfy!(isForwardRange, R1, R2) ||
{
assert(canFind(BC, tuple(n[0], n[1])));
}
assert(BC.length == 9);
}

// https://issues.dlang.org/show_bug.cgi?id=13091
Expand Down Expand Up @@ -371,17 +421,26 @@ if (ranges.length >= 2 &&
{
RR ranges;
RR current;
bool empty = true;
size_t length = 0;

this(RR _ranges)
{
length = 1;
ranges = _ranges;
empty = false;
foreach (i, r; ranges)
{
current[i] = r.save;
if (current[i].empty)
empty = true;
static if (__traits(hasMember, current[i], "length"))
{
length *= current[i].length;
}
else
{
size_t count = 0;
for (auto tmp = r.save; !tmp.empty; tmp.popFront())
++count;
length *= count;
}
}
}
@property auto front()
Expand All @@ -398,11 +457,10 @@ if (ranges.length >= 2 &&
r.popFront();
if (!r.empty) break;

static if (i == 0)
empty = true;
else
static if (i != 0)
r = ranges[i].save; // rollover
}
length--;
}
@property Result save() return scope
{
Expand All @@ -414,6 +472,10 @@ if (ranges.length >= 2 &&
}
return copy;
}
@property bool empty() return scope
{
return length == 0;
}
}
static assert(isForwardRange!Result, Result.stringof ~ " must be a forward"
~ " range");
Expand All @@ -428,13 +490,15 @@ if (ranges.length >= 2 &&
int[] a, b, c, d, e;
auto cprod = cartesianProduct(a,b,c,d,e);
assert(cprod.empty);
assert(cprod.length == 0);
foreach (_; cprod) {} // should not crash

// Test case where only one of the ranges is empty: the result should still
// be empty.
int[] p=[1], q=[];
auto cprod2 = cartesianProduct(p,p,p,q,p);
assert(cprod2.empty);
assert(cprod2.length == 0);
foreach (_; cprod2) {} // should not crash
}

Expand All @@ -443,6 +507,7 @@ if (ranges.length >= 2 &&
// .init value of cartesianProduct should be empty
auto cprod = cartesianProduct([0,0], [1,1], [2,2]);
assert(!cprod.empty);
assert(cprod.length == 8);
assert(cprod.init.empty);
}

Expand Down Expand Up @@ -519,6 +584,7 @@ if (!allSatisfy!(isForwardRange, R1, R2, RR) ||
auto C = [ "x", "y", "z" ];
auto ABC = cartesianProduct(A, B, C);

assert(ABC.length == 27);
assert(ABC.equal([
tuple(1, 'a', "x"), tuple(1, 'a', "y"), tuple(1, 'a', "z"),
tuple(1, 'b', "x"), tuple(1, 'b', "y"), tuple(1, 'b', "z"),
Expand Down Expand Up @@ -584,8 +650,9 @@ pure @safe nothrow @nogc unittest
}
}

assert(SystemRange([1, 2]).cartesianProduct(SystemRange([3, 4]))
.equal([tuple(1, 3), tuple(1, 4), tuple(2, 3), tuple(2, 4)]));
auto cprod = SystemRange([1, 2]).cartesianProduct(SystemRange([3, 4]));
assert(cprod.length == 4);
assert(cprod.equal([tuple(1, 3), tuple(1, 4), tuple(2, 3), tuple(2, 4)]));
}

// largestPartialIntersection
Expand Down
Loading