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.

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

[range.cartesian.view] Fix _cartesian-product-is-common_ #5735

Closed
JohelEGP opened this issue Aug 17, 2022 · 6 comments · Fixed by #5750
Closed

[range.cartesian.view] Fix _cartesian-product-is-common_ #5735

JohelEGP opened this issue Aug 17, 2022 · 6 comments · Fixed by #5750

Comments

@JohelEGP
Copy link
Contributor

What is Const? Why is Vs... unused? The refined concept only takes 1 argument.

Presumably, Const came from a copy-pasted maybe-const<Const, First>. That'd explain the extra closing >.

Considering the name of the concept and how it's used, this should be:

    (@\exposconcept{cartesian-product-common-arg}@<First> && ... &&
     @\exposconcept{cartesian-product-common-arg}@<Vs>);

Originally posted by @JohelEGP in #5647 (comment)

@tkoeppe
Copy link
Contributor

tkoeppe commented Aug 19, 2022

I think you're right, will send a PR.

@hewillk
Copy link
Contributor

hewillk commented Aug 22, 2022

Is this really correct? From the original paper:

cartesian_product_view can be a common range if the first range either is common, or is sized and random access. This paper reflects this.

In my opinion this should be

template <class First/*, class... Vs*/>
  concept cartesian-product-is-common = // exposition only
    cartesian-product-common-arg<First>;

@tkoeppe @JohelEGP

@JohelEGP
Copy link
Contributor Author

Then I'd also question why the end() overloads are constrained with @\exposconcept{cartesian-product-is-common}@<First, Vs...> rather than cartesian-product-common-arg<First> directly.

@hewillk
Copy link
Contributor

hewillk commented Aug 22, 2022

cartesian-product-common-arg might not be a good constraint name for end, cartesian-product-is-common seems to be additionally defined for consistency with cartesian-product-is-sized.

@tkoeppe
Copy link
Contributor

tkoeppe commented Aug 22, 2022

@griwes: ^^^ (This was the original issue.)

@griwes
Copy link

griwes commented Aug 22, 2022

The reason why common-arg isn't used directly in the constraint is that it's a "lower level" helper concept that's also used elsewhere, whereas is-common is a "higher level" helper concept that's used specifically for answering the question, "is a cartesian product of all of these ranges a common range?". That's also why it takes all the ranges as arguments, instead of just the first one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants