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

Jump to bottom

[range.chunk.by.view] Add missing exposition only comment #5498

Closed
wants to merge 1 commit into from
Closed

[range.chunk.by.view] Add missing exposition only comment #5498

wants to merge 1 commit into from

Conversation

hewillk
Copy link
Contributor

@hewillk hewillk commented May 27, 2022

No description provided.

CaseyCarter
CaseyCarter reviewed May 27, 2022
View reviewed changes
Copy link
Contributor

@CaseyCarter CaseyCarter left a comment

Choose a reason for hiding this comment

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

I came to say "this isn't a thing we do in the library specification", but I see there's at least one inconsistency. That said, I don't think we want to get in the business of spamming // exposition only comments on every use of every exposition-only name; we've been operating under the belief that tagging the first occurrence suffices, especially so in [ranges] where exposition-only class members are all rendered in kebab-case.

I'd rather see an editorial change that removes the few existing occurrences of redundant // exposition only comments in itemdecls than one that adds more of them.

@jensmaurer
Copy link
Member

Agreed. Patches welcome.

@hewillk
Copy link
Contributor Author

hewillk commented May 28, 2022

Agreed. Patches welcome.

I am glad to. However, I still have a question about this, taking the helper concept compatible-joinable-ranges as an example, which first appeared in [ranges.syn] and already has the //exposition only comment

// [range.join.with], join with view
template<class R, class P>
  concept compatible-joinable-ranges = see below; // exposition only

template<input_­range V, forward_­range Pattern>
  requires view<V> && input_­range<range_reference_t<V>>
        && view<Pattern>
        && compatible-joinable-ranges<range_reference_t<V>, Pattern>
class join_with_view;

So in [range.join.with.view], does this reappearance of // exposition only comment need to be removed?

namespace std::ranges {
  template<class R, class P>
  concept compatible-joinable-ranges =            // exposition only
      common_­with<range_value_t<R>, range_value_t<P>> &&
      common_­reference_­with<range_reference_t<R>, range_reference_t<P>> &&
      common_­reference_­with<range_rvalue_reference_t<R>, range_rvalue_reference_t<P>>;

@jwakely
Copy link
Member

jwakely commented May 28, 2022

IMHO it makes sense to keep the comment on declarations in a synopsis. Just not in a later \itemdecl.

@hewillk hewillk mentioned this pull request May 28, 2022
Merged
@hewillk hewillk closed this Jul 6, 2022
@hewillk hewillk deleted the main-lazy-split branch August 16, 2022 02:39
@tkoeppe tkoeppe mentioned this pull request Nov 8, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@CaseyCarter CaseyCarter CaseyCarter left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@hewillk @jensmaurer @jwakely @CaseyCarter