search

rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
97.4k stars 12.59k forks source link

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

Open hardfist opened 2 weeks ago

hardfist commented 2 weeks ago

Related to https://github.com/rust-lang/rust/pull/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.

lolbinarycat commented 2 weeks ago

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 commented 2 weeks ago

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

lolbinarycat commented 2 weeks ago

by the same logic, isn't adding a new lint a breaking change, as someone could be using #[deny(all)]? i think that generally lint changes are never considered breaking changes, as there have been several cases of deny-by-default lints being added without being edition-gated.