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

Is "must_use" really needed for Option::insert ? #130594

Open
hardfist opened this issue Sep 20, 2024 · 2 comments
Open

Is "must_use" really needed for Option::insert ? #130594

hardfist opened this issue Sep 20, 2024 · 2 comments
Labels
C-discussion Category: Discussion or questions that doesn't represent real issues. T-lang Relevant to the language team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@hardfist
Copy link

Related to #87196

I personally think insert is a more succinct and readable way than = Some(x) and this seems should be a style preference and don't get why it's marked as must_use.
what's more weird is Option::replace is not marked as must use, so the syntax is kind of inconsistent to me.

@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Sep 20, 2024
@jieyouxu jieyouxu added T-lang Relevant to the language team, which will review and decide on the PR/issue. C-discussion Category: Discussion or questions that doesn't represent real issues. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Sep 20, 2024
@lolbinarycat
Copy link

the original PR seems like it was merged before anyone's concerns were addressed. there were two positions expressed (all functions of this type should be annotated, or none), and neither were satisfied. usually i'm all for incremental PRs, but when it's a single trivial insertion, that seems a bit much.

my two cents is this should be a clippy pedantic (or even restriction) lint, nothing more.

@jieyouxu
Copy link
Member

Hm, would removing #[must_use] on Option::insert be a breaking change if someone #[expect(unused_must_use)] for Option::insert?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-discussion Category: Discussion or questions that doesn't represent real issues. T-lang Relevant to the language team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants