rust-lang / rust-clippy

A bunch of lints to catch common mistakes and improve your Rust code. Book: https://doc.rust-lang.org/clippy/
https://rust-lang.github.io/rust-clippy/
Other
11.24k stars 1.51k forks source link

`declare_interior_mutable_const` now fires for `arcstr::ArcStr` #12951

Closed kpreid closed 2 weeks ago

kpreid commented 2 months ago

Summary

The arcstr::ArcStr type is interior mutable for its reference count, but provides a macro which can safely construct const instances. As of 1.81.0-nightly (59e2c01c2 2024-06-17), presumably due to #12691, there is now a warning emitted even though the macro tries to suppress it — all relevant components of the macro expansion have #[allow(clippy::declare_interior_mutable_const)].

I imagine that this might be considered “working as intended”, with the solution being to configure ignore-interior-mutability much as is done with bytes. Still, this is a regression in out-of-the-box usability, so I figured I'd report it for consideration.

Lint Name

declare_interior_mutable_const

Reproducer

I tried this code:

// arcstr = { version = "1.2.0" }

const FOO: arcstr::ArcStr = arcstr::literal!("hello");

I saw this happen:

warning: a `const` item should not be interior mutable
 --> src/lib.rs:1:1
  |
1 | const FOO: arcstr::ArcStr = arcstr::literal!("hello");
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

I expected to see this happen: No warning

Version

rustc 1.81.0-nightly (59e2c01c2 2024-06-17)
binary: rustc
commit-hash: 59e2c01c2217a01546222e4d9ff4e6695ee8a1db
commit-date: 2024-06-17
host: x86_64-apple-darwin
release: 1.81.0-nightly
LLVM version: 18.1.7

Additional Labels

No response

kpreid commented 1 month ago

This bug has now reached the 1.80.0 stable pre-release :(

Alexendoo commented 1 month ago

ArcStr stores a NonNull pointer to an interior mutable type, previously these weren't linted but after https://github.com/rust-lang/rust-clippy/pull/12691 the lint now follows references/pointers to interior mutable types

Since it's linting based on the type this also triggers without macros being used:

const S: ArcStr = ArcStr::new();

Nominating for discussion for how we want to handle this in an ongoing fashion, we could continue adding exceptions to the list as they pop up or make the lint more conservative

flip1995 commented 1 month ago

https://github.com/rust-lang/rust-clippy/pull/13207 This PR might fix this issue, without having to add arcstr to our default config. So we decided to wait for that and then re-evaluate.

thomcc commented 1 month ago

(author of arcstr here) I tried #[allow]ing the lint inside the macros, and because the user's const is outside the macro it didn't do the trick. That said, it might help some cases (like macro use that's not directly assigning a const) so I might publish an update with it.

Alexendoo commented 2 weeks ago

We spoke about the issue in the Clippy meeting and decided to backport a small fix (#13290) while waiting for the full #13207