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

[associative.reqmts.general] introduces r with two different meanings #5134

Closed
StephanTLavavej opened this issue Dec 3, 2021 · 3 comments · Fixed by #5138
Closed

[associative.reqmts.general] introduces r with two different meanings #5134

StephanTLavavej opened this issue Dec 3, 2021 · 3 comments · Fixed by #5138
Assignees
@jensmaurer

Comments

@StephanTLavavej
Copy link
Contributor

N4901 [associative.reqmts.general]/8.13:

— r denotes a valid dereferenceable iterator to a,

But then /8.19 through /8.22 introduce r with a totally different meaning:

— kl is a value such that a is partitioned (25.8) with respect to c(r, kl), with r the key value of e and e in a;
— ku is a value such that a is partitioned with respect to !c(ku, r);
— ke is a value such that a is partitioned with respect to c(r, ke) and !c(ke, r), with c(r, ke) implying !c(ke, r);
— kx is a value such that
— a is partitioned with respect to c(r, rx) and !c(kx, r), with c(r, kx) implying !c(kx, r), and
— kx is not convertible to either iterator or const_iterator; and

This is confusing; a different name should be introduced.
@jensmaurer
Copy link
Member

Are you saying that all mentions of "r" in p8.19 through p8.22 mean "r" as in "r the key value of e and e in a"?
If so, it's seriously confusing that we introduce "r" only in p8.19 and not with a larger scope.
However, it's obvious that "r" can't have the "iterator" meaning here, because it needs to be of the key_type.

@StephanTLavavej
Copy link
Contributor Author

Yep! It is indeed seriously confusing.

@jensmaurer jensmaurer self-assigned this Dec 4, 2021
@jensmaurer jensmaurer mentioned this issue Dec 4, 2021
Merged
@jwakely
Copy link
Member

jwakely commented Dec 14, 2021

The first r in (8.13) was introduced by f03534f for LWG 2059. It should have been a different name.

It would be a smaller change to rename that one, and the single use of it in the table, a.erase(r).

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

@jensmaurer jensmaurer

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[associative.reqmts.general] Fix confusing local use of 'r' jensmaurer/draft
3 participants
@jwakely @StephanTLavavej @jensmaurer