Skip to content

Comments

<ranges>: Fix cartesian_product_view::size in debug mode#3733

Merged
StephanTLavavej merged 3 commits intomicrosoft:mainfrom
JMazurkiewicz:cartprod-fix-size-debug
May 31, 2023
Merged

<ranges>: Fix cartesian_product_view::size in debug mode#3733
StephanTLavavej merged 3 commits intomicrosoft:mainfrom
JMazurkiewicz:cartprod-fix-size-debug

Conversation

@JMazurkiewicz
Copy link
Contributor

@JMazurkiewicz JMazurkiewicz commented May 25, 2023 •
edited
Loading

Per [range.cartesian.view]/9-10:

Let p be the product of the sizes of all the ranges in bases_.
Preconditions: p can be represented by the return type.

Currently cartesian_product_view::size may fail when p can be represented by return type:

#include <ranges>

using namespace std;

int main() {
  const auto r1 = views::repeat(0, (numeric_limits<ptrdiff_t>::max)());
  const auto r2 = views::repeat(1, 0);
  (void)views::cartesian_product(r1, r1, r1, r2).size(); // FAIL!
}

Obviously 0 can be represented by any unsigned integral type. This PR fixes this bug.

@JMazurkiewicz JMazurkiewicz marked this pull request as ready for review May 25, 2023 22:40
@JMazurkiewicz JMazurkiewicz requested a review from a team as a code owner May 25, 2023 22:40
@JMazurkiewicz

This comment was marked as resolved.

Sign in to view
CaseyCarter
CaseyCarter suggested changes May 25, 2023
View reviewed changes
@CaseyCarter CaseyCarter added ranges C++20/23 ranges bug Something isn't working labels May 25, 2023
@StephanTLavavej StephanTLavavej self-assigned this May 26, 2023
@StephanTLavavej
Copy link
Member

StephanTLavavej commented May 26, 2023

I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed.

@StephanTLavavej StephanTLavavej merged commit 0d47d98 into microsoft:main May 31, 2023
@StephanTLavavej
Copy link
Member

StephanTLavavej commented May 31, 2023

Thanks for fixing this corner case! 0️⃣ ✖️ ♾️

@JMazurkiewicz JMazurkiewicz deleted the cartprod-fix-size-debug branch May 31, 2023 00:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@StephanTLavavej StephanTLavavej StephanTLavavej approved these changes

+1 more reviewer

@CaseyCarter CaseyCarter CaseyCarter approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@StephanTLavavej StephanTLavavej

Labels

bug Something isn't working ranges C++20/23 ranges

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@JMazurkiewicz @StephanTLavavej @CaseyCarter