Skip to content

[coro.generator] Rename the generator's template parameter V to Val #7129

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

Merged
merged 1 commit into from
Jul 29, 2024

Conversation

hewillk
Copy link
Contributor

@hewillk hewillk commented Jul 10, 2024

Fixes #6451.

@jensmaurer
Copy link
Member

Names of template parameters rarely convey lots of meaning, it seems to me. So, this change feels rather undermotivated to me.

@jwakely ?

Copy link
Member

@jwakely jwakely left a comment

Choose a reason for hiding this comment

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

While I agree that template parameter names aren't always meaningful, for generator we have Ref and V not just T and U or T1 and T2, so there's already some attempt at making them meaningful.

And I think there's also scope for confusion due to class V = void and is_void_v<V> making it seem like V is for void.

I think this change does give some positive value (no pun intended).

@jwakely
Copy link
Member

jwakely commented Jul 18, 2024

And I think there's also scope for confusion due to class V = void and is_void_v<V> making it seem like V is for void.

Ha, just realised I've repeated the argument given in #6451, so I guess that means I agree with it.

Copy link
Member

@jensmaurer jensmaurer left a comment

Choose a reason for hiding this comment

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

Let's do that, then.

@tkoeppe
Copy link
Contributor

tkoeppe commented Jul 29, 2024

@hewillk In the future, can you please write commit messages that describe the change?

@tkoeppe tkoeppe merged commit d9bff4a into cplusplus:main Jul 29, 2024
2 checks passed
@hewillk hewillk deleted the main-coro branch July 29, 2024 15:35
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.

[coro.generator.class] Name the generator's second template parameter Val?
4 participants