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.filter.sentinel] filter_view::sentinel's single-parameter constructor seems wrong #2880

Closed
cpplearner opened this issue May 11, 2019 · 3 comments · Fixed by #2937
Closed

[range.filter.sentinel] filter_view::sentinel's single-parameter constructor seems wrong #2880

cpplearner opened this issue May 11, 2019 · 3 comments · Fixed by #2937
Assignees
@CaseyCarter

Comments

@cpplearner
Copy link
Contributor

[range.filter.sentinel]:

constexpr explicit sentinel(filter_view& parent);

Effects: Initializes end_ with ranges::end(parent).

  1. ranges::end(parent) should probably be ranges::end(parent.base_).

  2. The design of sentinel's constructor seems to be unnecessarily inconsistent among range adaptors.

  • filter_view::sentinel ([range.filter.sentinel]) and join_view::sentinel ([range.join.sentinel]) have constructors that accept a reference to parent type, and call ranges::end inside that constructor. The parents simply pass *this as sentinel constructor argument in end().
  • transform_view::sentinel ([range.transform.sentinel]) and take_view::sentinel ([range.take.sentinel]) have constructors that accept a sentinel_t<Base>, the parents pass ranges::end(base_) as sentinel constructor argument in end().
@cpplearner cpplearner changed the title filter_view::sentinel's single-parameter constructor seems wrong [range.filter.sentinel]filter_view::sentinel's single-parameter constructor seems wrong May 13, 2019
@jensmaurer
Copy link
Member

"The design of ..." catapults this right out of editorial scope.

@CaseyCarter , do you have an opinion on the first item above?

@jensmaurer jensmaurer changed the title [range.filter.sentinel]filter_view::sentinel's single-parameter constructor seems wrong [range.filter.sentinel] filter_view::sentinel's single-parameter constructor seems wrong May 24, 2019
@cpplearner
Copy link
Contributor Author

Nevermind 2. I think I understand now. The difference is because the iterators of filter_view and join_view use ranges::end(parent_->base_), which only makes sense when the sentinels hold the same value.

CaseyCarter added a commit to CaseyCarter/draft that referenced this issue Jun 20, 2019
@CaseyCarter
[range.filter.sentinel] Correct typo in constructor Effects
1f116b4 
Resolves cplusplus#2880.
@CaseyCarter CaseyCarter mentioned this issue Jun 20, 2019
Merged
@CaseyCarter
Copy link
Contributor

@CaseyCarter , do you have an opinion on the first item above?

Yes, the spec is completely bogus: either the specified initialization is ill-formed, or the resulting program will overflow its stack recursing between this constructor and filter_view::end.

I've submitted a PR with the editorial-ish fix; feel free to assign this issue to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@CaseyCarter CaseyCarter

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[range.filter.sentinel] Correct typo in constructor Effects CaseyCarter/draft
3 participants
@CaseyCarter @cpplearner @jensmaurer