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

P2767R0 §3 as seen by LWG in Varna #6274

Merged
merged 8 commits into from
Jun 10, 2024

Conversation

Quuxplusone
Copy link
Contributor

@Quuxplusone Quuxplusone commented Jun 16, 2023

LWG reviewed this wording in Varna, approved it as suitably editorial (poll 7–0–1), and we agreed that I should now submit it as a PR for @jwakely to look at.

This PR right now is structured as eight commits: two commits per container adaptor. For each adaptor, the first commit is literally the wording published in P2767R0 §3 and reviewed live in Varna. The second commit is amendments requested by LWG, to put the constructor overload sets in the traditional order (e.g. default constructor at the top). These second commits are editorial, and they're improvements over the status quo, and they were explicitly requested by LWG, but they have not been published in a P-paper, and have not been reviewed yet. Please see if they match your expectation!

Also, of course, I may have made typos in my transcription of even the first commits, so someone (@jwakely presumably) should still look through this and not just merge it blindly. Duh. :)

I expect this PR will be squash-merged with an updated commit message of some sort, and am happy to let whoever merges it figure that part out. Please let me know if you want me on my side to squash it and/or reword the commit message.

@Quuxplusone
Copy link
Contributor Author

@jwakely : Belated ping! I bet it's too late now to get this into the draft before I submit P2767R1 based on top of it, but if I could have remembered to ping you last week, I should have...

Quuxplusone added a commit to Quuxplusone/draft that referenced this pull request Jul 13, 2023
This should not be merged yet; it should be rebased after cplusplus#6274.
@Quuxplusone
Copy link
Contributor Author

I just noticed that my original upload had missed adding the text

+\pnum
+The constructors in this subclause shall not participate in overload resolution
+unless \tcode{uses_allocator_v<key_container_type, Alloc>} is \tcode{true}
+and \tcode{uses_allocator_v<mapped_container_type, Alloc>} is \tcode{true}.

at the top of [flat.multimap.cons.alloc]. This is now fixed.

@Quuxplusone
Copy link
Contributor Author

Rebased to pick up the additions in 03da5ba (LWG3884). @jwakely ping!

@frederick-vs-ja
Copy link
Contributor

@jensmaurer Why is this PR considered able to close cplusplus/papers#1539? I think P2767 is largely non-editorial.

@Quuxplusone
Copy link
Contributor Author

@jensmaurer Why is this PR considered able to close cplusplus/papers#1539? I think P2767 is largely non-editorial.

@frederick-vs-ja You're correct, this PR absolutely would not "close" cplusplus/papers#1539, since it encompasses only one section (§3) of that whole paper. You're also correct that all the other sections are non-editorial (although some are at least non-design-level).

@Quuxplusone
Copy link
Contributor Author

Rebased to pick up aa8a531 (typos in [flat.multiset]). @jwakely ping!

@jensmaurer
Copy link
Member

Why is this PR considered able to close cplusplus/papers#1539? I think P2767 is largely non-editorial.

Apologies, removed.

@Quuxplusone
Copy link
Contributor Author

@jwakely ping! (Remember we reviewed this wording in Varna?)

@Quuxplusone
Copy link
Contributor Author

@jwakely ping, please!

source/containers.tex Show resolved Hide resolved
source/containers.tex Show resolved Hide resolved
source/containers.tex Outdated Show resolved Hide resolved
source/containers.tex Outdated Show resolved Hide resolved
Modified to resolve the merge conflict with LWG3884 (03da5ba).
Swap the default ctor to the top.
Follow each non-`key_compare` overload by its `key_compare` counterpart.
Then, follow each non-`sorted_unique` pair of overloads by its `sorted_unique` counterparts.
Modified to resolve the merge conflict with LWG3884 (03da5ba).
Swap the default ctor to the top.
Follow each non-`key_compare` overload by its `key_compare` counterpart.
Then, follow each non-`sorted_unique` pair of overloads by its `sorted_unique` counterparts.
Modified to resolve the merge conflict with LWG3884 (03da5ba).
Swap the default ctor to the top.
Follow each non-`key_compare` overload by its `key_compare` counterpart.
Then, follow each non-`sorted_unique` pair of overloads by its `sorted_unique` counterparts.
Modified to resolve the merge conflict with LWG3884 (03da5ba).
Swap the default ctor to the top.
Follow each non-`key_compare` overload by its `key_compare` counterpart.
Then, follow each non-`sorted_unique` pair of overloads by its `sorted_unique` counterparts.
@jensmaurer jensmaurer merged commit dbf1752 into cplusplus:main Jun 10, 2024
2 checks passed
jensmaurer pushed a commit to jensmaurer/draft that referenced this pull request Jul 28, 2024
…lus#6274)

Canonicalize the ordering of the constructors for the flat_* adaptors:
 - default constructor is first
 - each overload without `key_compare` is followed by the corresponding one with `key_compare`
 - each pair of overloads without `sorted_unique` is followed by the corresponding pair with `sorted_unique`
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.

4 participants