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] Remove unused template parameter #6177

Merged
merged 1 commit into from Nov 13, 2023

Conversation

hewillk
Copy link
Contributor

@hewillk hewillk commented Mar 13, 2023

Is it reasonable to do so?

@JohelEGP
Copy link
Contributor

Part of #5735. I don't see why not.

@CaseyCarter
Copy link
Contributor

I think I'd rather see the unused template parameter pack removed from the exposition-only concept; there are only four uses to clean up.

@hewillk
Copy link
Contributor Author

hewillk commented Mar 13, 2023

I think I'd rather see the unused template parameter pack removed from the exposition-only concept; there are only four uses to clean up.

#5735 (comment): "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.

Although it's clearly the intended design. I think it's more straightforward to just remove the template parameter pack, and I don't see how it would cause confusion, so I prefer removing it.

@tkoeppe
Copy link
Contributor

tkoeppe commented Nov 10, 2023

@CaseyCarter Would that be editorial, or should we open an LWG issue to remove the pack?

@CaseyCarter
Copy link
Contributor

@CaseyCarter Would that be editorial, or should we open an LWG issue to remove the pack?

I think the change in the PR is both clearly editorial, and a minor readability improvement. Nothing says "yes, I really intend to ignore this parameter" as explicitly as does leaving it unnamed.

Any further simplifications - Should we remove the parameter pack? Should we replace uses of cartesian-product-is-common<T, Us...> with cartesian-product-common-arg<T> and remove the former altogether? - probably ought to go through LWG. The current structure was put in place for readability, and while we may not all agree that it's ideal, we should give plenty of eyes the chance to comment on any larger structural change.

@tkoeppe
Copy link
Contributor

tkoeppe commented Nov 13, 2023

@CaseyCarter Thanks!

@hewillk Please file an LWG issue if you want to remove the template parameter.

@tkoeppe tkoeppe merged commit 9799839 into cplusplus:main Nov 13, 2023
2 checks passed
@hewillk hewillk deleted the mian-cart branch November 15, 2023 09:13
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 this pull request may close these issues.

None yet

4 participants