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

Avoid overflow in cartesian_product size() impl #168

Merged
merged 2 commits into from
Feb 3, 2024
Merged

Avoid overflow in cartesian_product size() impl #168

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1158ed3
Avoid overflow in cartesian_product size() impl
tcbrindle Feb 1, 2024
b4c244d
Improve cartesian_product size fix
tcbrindle Feb 2, 2024
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
6 changes: 4 additions & 2 deletions include/flux/op/cartesian_base.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,8 +221,10 @@ struct cartesian_traits_base_impl {
requires (CartesianKind == cartesian_kind::product
&& (sized_sequence<Bases> && ...))
{
return std::apply([](auto&... base) {
return (flux::size(base) * ...);
return std::apply([](auto& base0, auto&... bases) {
distance_t sz = flux::size(base0);
((sz = num::checked_mul(sz, flux::size(bases))), ...);
return sz;
}, self.bases_);
}

Expand Down
13 changes: 13 additions & 0 deletions test/test_cartesian_product.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -498,9 +498,22 @@ constexpr bool test_cartesian_product()
}
static_assert(test_cartesian_product());

// https://github.com/tcbrindle/flux/issues/167
void issue_167()
{
// Check that overflowing size() is correctly caught
auto ints = flux::ints(0, std::numeric_limits<flux::distance_t>::max());

auto prod = flux::cartesian_product(ints, ints, ints);

REQUIRE_THROWS_AS(flux::size(prod), flux::unrecoverable_error);
}

}

TEST_CASE("cartesian_product")
{
REQUIRE(test_cartesian_product());

issue_167();
}
59 changes: 35 additions & 24 deletions test/test_cartesian_product_map.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ constexpr bool test_cartesian_product_map()
std::array arr1{100, 200};
std::array arr2{1, 2, 3, 4, 5};

auto cart = flux::cartesian_product_map(sum, flux::ref(arr1), flux::ref(arr2));
auto cart =
flux::cartesian_product_map(sum, flux::ref(arr1), flux::ref(arr2));
using C = decltype(cart);

static_assert(flux::sequence<C>);
Expand All @@ -27,14 +28,12 @@ constexpr bool test_cartesian_product_map()

STATIC_CHECK(flux::size(cart) == 2 * 5);

STATIC_CHECK(check_equal(cart, { 101, 102, 103, 104, 105,
201, 202, 203, 204, 205
}));
STATIC_CHECK(check_equal(
cart, {101, 102, 103, 104, 105, 201, 202, 203, 204, 205}));

STATIC_CHECK(check_equal(flux::reverse(flux::ref(cart)),
{ 205, 204, 203, 202, 201,
105, 104, 103, 102, 101
}));
STATIC_CHECK(
check_equal(flux::reverse(flux::ref(cart)),
{205, 204, 203, 202, 201, 105, 104, 103, 102, 101}));

// Random access checks
STATIC_CHECK(flux::distance(cart, cart.first(), cart.last()) == 2 * 5);
Expand All @@ -52,7 +51,8 @@ constexpr bool test_cartesian_product_map()
std::array arr2{10, 20, 30};
std::array arr3{1, 2, 3, 4};

auto cart = flux::cartesian_product_map(sum, flux::ref(arr1), flux::ref(arr2), flux::ref(arr3));
auto cart = flux::cartesian_product_map(
sum, flux::ref(arr1), flux::ref(arr2), flux::ref(arr3));
using C = decltype(cart);

static_assert(flux::sequence<C>);
Expand All @@ -64,13 +64,10 @@ constexpr bool test_cartesian_product_map()

STATIC_CHECK(flux::size(cart) == 2 * 3 * 4);

STATIC_CHECK(check_equal(cart, { 111, 112, 113, 114,
121, 122, 123, 124,
131, 132, 133, 134,
211, 212, 213, 214,
221, 222, 223, 224,
231, 232, 233, 234 }
));
STATIC_CHECK(
check_equal(cart, {111, 112, 113, 114, 121, 122, 123, 124,
131, 132, 133, 134, 211, 212, 213, 214,
221, 222, 223, 224, 231, 232, 233, 234}));

{
auto cur = flux::next(cart, cart.first(), 7);
Expand All @@ -84,7 +81,8 @@ constexpr bool test_cartesian_product_map()

{
auto seq0 = single_pass_only(flux::from(std::array{100, 200}));
auto cart = flux::cartesian_product_map(sum, std::move(seq0), std::array{1, 2, 3});
auto cart = flux::cartesian_product_map(sum, std::move(seq0),
std::array{1, 2, 3});
using C = decltype(cart);

static_assert(flux::sequence<C>);
Expand All @@ -100,7 +98,8 @@ constexpr bool test_cartesian_product_map()
auto arr = std::array{1, 2, 3, 4, 5};
auto emp = flux::empty<int>;

auto cart = flux::cartesian_product_map(sum, flux::ref(arr), std::move(emp));
auto cart =
flux::cartesian_product_map(sum, flux::ref(arr), std::move(emp));

static_assert(flux::bidirectional_sequence<decltype(cart)>);

Expand All @@ -116,26 +115,38 @@ constexpr bool test_cartesian_product_map()
double vals[3][3] = {};
auto get = [&vals](auto i, auto j) -> double& { return vals[i][j]; };

auto seq = flux::cartesian_product_map(get, flux::iota(0, 3), flux::iota(0, 3));
auto seq = flux::cartesian_product_map(get, flux::iota(0, 3),
flux::iota(0, 3));

static_assert(std::same_as<flux::element_t<decltype(seq)>, double&>);

seq.fill(100.0);

for (int i = 0; i < 3; i++) {
for (int j = 0; j < 3; j++) {
STATIC_CHECK(vals[i][j] == 100.0);
}
for (int j = 0; j < 3; j++) { STATIC_CHECK(vals[i][j] == 100.0); }
}
}

return true;
}
static_assert(test_cartesian_product_map());

// https://github.com/tcbrindle/flux/issues/167
void issue_167()
{
// Check that overflowing size() is correctly caught
auto ints = flux::ints(0, std::numeric_limits<flux::distance_t>::max());

auto prod = flux::cartesian_product_map([](auto...) { return 0; }, ints, ints, ints);

REQUIRE_THROWS_AS(flux::size(prod), flux::unrecoverable_error);
}

}

TEST_CASE("cartesian_product_map")
{
REQUIRE(test_cartesian_product_map());
}

}
issue_167();
}
Loading