Skip to content

[flat.map, flat.multimap, flat.set, flat.multiset] Exposition-only formatting for c, comp, compare, and key-equiv #6404

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
Dec 17, 2024

Conversation

frederick-vs-ja
Copy link
Contributor

@frederick-vs-ja frederick-vs-ja commented Jul 23, 2023

Also changes key_equiv to key-equiv.

I think it's OK to keep the member comp of key_equiv not marked \exposid, since the identifier comp is already reserved by priority_queue::comp. Implementations can use the name comp as-is or use another name.

key_equiv and value_compare's exposition-only constructors are less than ideal to me. But I think they should be handled in an LWG issue because additional copy of comparator seems to be observable. Edit: now this is LWG3959.

Follows up #5914. Fixes #5812. Fixes #7380.

@tkoeppe
Copy link
Contributor

tkoeppe commented Nov 10, 2023

Thank you, this would be quite nice to land. @Dani-Hub, @CaseyCarter, would you perhaps be able to lend another pair of eyes?

@tkoeppe
Copy link
Contributor

tkoeppe commented Nov 10, 2023

@frederick-vs-ja Could you perhaps retrigger the precommit checks so we can get a downloadable PDF?

@Dani-Hub
Copy link
Member

Thank you, this would be quite nice to land. @Dani-Hub, @CaseyCarter, would you perhaps be able to lend another pair of eyes?

I would happily do so, but I'm away during from an 30 minutes from now until Tuesday and I'm unable to do that right now.

@frederick-vs-ja
Copy link
Contributor Author

@frederick-vs-ja Could you perhaps retrigger the precommit checks so we can get a downloadable PDF?

I've rebased and force-pushed, not sure whether this is sufficent...

@Dani-Hub
Copy link
Member

The changes look good to me, but I'd like to point out that similar fixes in e.g. for c and compare in [flat.multiset.modifiers] have not been applied. The issue says that it does only touch [flat.map] and [flat.multimap], so this is formally OK.

@tkoeppe
Copy link
Contributor

tkoeppe commented Nov 14, 2023

@Dani-Hub Thanks! Yes, good point, we should fix those cases, too.

@frederick-vs-ja
Copy link
Contributor Author

@Dani-Hub @tkoeppe Sorry for noticing the comments so late. Now the changes are also applied to [flat.multiset.modifiers].

@Dani-Hub
Copy link
Member

@Dani-Hub @tkoeppe Sorry for noticing the comments so late. Now the changes are also applied to [flat.multiset.modifiers].

Looks good to me - Thanks!

@frederick-vs-ja frederick-vs-ja changed the title [flat.map][flat.multimap] Exposition-only formatting for c, comp, compare, and key-equiv [flat.map, flat.multimap, flat.set, flat.multiset] Exposition-only formatting for c, comp, compare, and key-equiv Sep 27, 2024
@frederick-vs-ja
Copy link
Contributor Author

Rebased & force-pushed to resolve some conflicts.

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.

Feels like an improvement.

@jwakely , what do you think?

@tkoeppe
Copy link
Contributor

tkoeppe commented Dec 17, 2024

Yes, I like it, this has been a long-standing issue. Thanks!

@tkoeppe tkoeppe merged commit daae8f9 into cplusplus:main Dec 17, 2024
2 checks passed
@frederick-vs-ja frederick-vs-ja deleted the flatmap-expos-2 branch December 17, 2024 23:33
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.

flat maps could use \exposid for private members Style exposition-only members of flat_map, flat_multimap, flat_set, flat_multiset consistently
5 participants