Skip to content

[container.requirements] Simplify Returns specification for try_emplace #7892

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 1 commit into from
Jun 2, 2025

Conversation

Eisenwave
Copy link
Member

Fixes #7881.

@jensmaurer
Copy link
Member

@jwakely , since we already say we do as-if emplace in the "Effects", this change seems to be a nice clarification. What do you think?

@jwakely
Copy link
Member

jwakely commented May 26, 2025

Why not just Equivalent to: Returns ... except that ... ?

@Eisenwave Eisenwave force-pushed the emplace-hint-returns branch from cb056ae to 806be71 Compare May 26, 2025 18:29
@Eisenwave
Copy link
Member Author

@jwakely that seems better, yes.

I've also deleted a bunch of the adjacent specifications because those are inherited when "Equivalent to" is specified. Let me know if that's correct.

@Eisenwave Eisenwave force-pushed the emplace-hint-returns branch from 806be71 to 132931e Compare May 26, 2025 18:34
Copy link
Member

@jwakely jwakely left a comment

Choose a reason for hiding this comment

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

I think the \result elements should remain, those are still useful (and [structure.specifications] p4 does not say that Effects: Equivalent to implies the Result: element).

I prefer the original form of "is a hint ... Implementations are permitted to ignore the hint." rather than "is an ignorable hint..."

@Eisenwave Eisenwave force-pushed the emplace-hint-returns branch from 132931e to 4c7ff44 Compare May 28, 2025 04:16
@Eisenwave Eisenwave requested a review from jwakely May 28, 2025 04:16
@@ -3233,15 +3233,10 @@

\pnum
\effects
Equivalent to \tcode{a.emplace(std::forward<Args>(args)...)},
Equivalent to: \tcode{return a.emplace(std::forward<Args>(args)...).first},
Copy link
Member

Choose a reason for hiding this comment

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

I've just realised that this doesn't work, because a_uniq.emplace and a_eq.emplace return different things. It's not a pair for a_eq, so you can't do .first

So maybe what you had before was better, sorry.

Copy link
Member Author

Choose a reason for hiding this comment

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

I must not have been paying attention either; my bad. The latest commit should resolve everything.

@Eisenwave Eisenwave force-pushed the emplace-hint-returns branch from 4c7ff44 to a4e485e Compare May 30, 2025 11:51
@jensmaurer jensmaurer requested a review from jwakely June 2, 2025 20:01
@jensmaurer jensmaurer merged commit a3b9cb5 into cplusplus:main Jun 2, 2025
0 of 2 checks passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[container.requirements] Confusing "Returns" paragraph under a.emplace_hint(p, args)
3 participants