fix a FP in interior_mutable_const

fix a false positive in two `interior_mutable_const` lints where a constant with enums gets linted
even if it uses a clearly unfrozen variant. Note that the code uses the MIR interpreter, which
the author of #3962 thought unlikely to be a solution. This might be over-engineering;
but, I think it's important to be able to work with the 'http' crate (#3825).
This commit is contained in:
rail 2020-10-01 10:51:49 +13:00
parent be8160878a
commit f58a1695a6
6 changed files with 539 additions and 35 deletions

View file

@ -0,0 +1,16 @@
// this file solely exists to test constants defined in foreign crates.
// As the most common case is the `http` crate, it replicates `http::HeadewrName`'s structure.
#![allow(clippy::declare_interior_mutable_const)]
use std::sync::atomic::AtomicUsize;
enum Private<T> {
ToBeUnfrozen(T),
Frozen(usize),
}
pub struct Wrapper(Private<AtomicUsize>);
pub const WRAPPED_PRIVATE_UNFROZEN_VARIANT: Wrapper = Wrapper(Private::ToBeUnfrozen(AtomicUsize::new(6)));
pub const WRAPPED_PRIVATE_FROZEN_VARIANT: Wrapper = Wrapper(Private::Frozen(7));

View file

@ -0,0 +1,101 @@
// aux-build:helper.rs
#![warn(clippy::borrow_interior_mutable_const)]
#![allow(clippy::declare_interior_mutable_const)]
// this file (mostly) replicates its `declare` counterpart. Please see it for more discussions.
extern crate helper;
use std::cell::Cell;
use std::sync::atomic::AtomicUsize;
enum OptionalCell {
Unfrozen(Cell<bool>),
Frozen,
}
const UNFROZEN_VARIANT: OptionalCell = OptionalCell::Unfrozen(Cell::new(true));
const FROZEN_VARIANT: OptionalCell = OptionalCell::Frozen;
fn borrow_optional_cell() {
let _ = &UNFROZEN_VARIANT; //~ ERROR interior mutability
let _ = &FROZEN_VARIANT;
}
trait AssocConsts {
const TO_BE_UNFROZEN_VARIANT: OptionalCell;
const TO_BE_FROZEN_VARIANT: OptionalCell;
const DEFAULTED_ON_UNFROZEN_VARIANT: OptionalCell = OptionalCell::Unfrozen(Cell::new(false));
const DEFAULTED_ON_FROZEN_VARIANT: OptionalCell = OptionalCell::Frozen;
fn function() {
// This is the "suboptimal behavior" mentioned in `is_value_unfrozen`
// caused by a similar reason to unfrozen types without any default values
// get linted even if it has frozen variants'.
let _ = &Self::TO_BE_FROZEN_VARIANT; //~ ERROR interior mutable
// The lint ignores default values because an impl of this trait can set
// an unfrozen variant to `DEFAULTED_ON_FROZEN_VARIANT` and use the default impl for `function`.
let _ = &Self::DEFAULTED_ON_FROZEN_VARIANT; //~ ERROR interior mutable
}
}
impl AssocConsts for u64 {
const TO_BE_UNFROZEN_VARIANT: OptionalCell = OptionalCell::Unfrozen(Cell::new(false));
const TO_BE_FROZEN_VARIANT: OptionalCell = OptionalCell::Frozen;
fn function() {
let _ = &<Self as AssocConsts>::TO_BE_UNFROZEN_VARIANT; //~ ERROR interior mutable
let _ = &<Self as AssocConsts>::TO_BE_FROZEN_VARIANT;
let _ = &Self::DEFAULTED_ON_UNFROZEN_VARIANT; //~ ERROR interior mutable
let _ = &Self::DEFAULTED_ON_FROZEN_VARIANT;
}
}
trait AssocTypes {
type ToBeUnfrozen;
const TO_BE_UNFROZEN_VARIANT: Option<Self::ToBeUnfrozen>;
const TO_BE_FROZEN_VARIANT: Option<Self::ToBeUnfrozen>;
// there's no need to test here because it's the exactly same as `trait::AssocTypes`
fn function();
}
impl AssocTypes for u64 {
type ToBeUnfrozen = AtomicUsize;
const TO_BE_UNFROZEN_VARIANT: Option<Self::ToBeUnfrozen> = Some(Self::ToBeUnfrozen::new(4)); //~ ERROR interior mutable
const TO_BE_FROZEN_VARIANT: Option<Self::ToBeUnfrozen> = None;
fn function() {
let _ = &<Self as AssocTypes>::TO_BE_UNFROZEN_VARIANT; //~ ERROR interior mutable
let _ = &<Self as AssocTypes>::TO_BE_FROZEN_VARIANT;
}
}
enum BothOfCellAndGeneric<T> {
Unfrozen(Cell<*const T>),
Generic(*const T),
Frozen(usize),
}
impl<T> BothOfCellAndGeneric<T> {
const UNFROZEN_VARIANT: BothOfCellAndGeneric<T> = BothOfCellAndGeneric::Unfrozen(Cell::new(std::ptr::null())); //~ ERROR interior mutable
const GENERIC_VARIANT: BothOfCellAndGeneric<T> = BothOfCellAndGeneric::Generic(std::ptr::null()); //~ ERROR interior mutable
const FROZEN_VARIANT: BothOfCellAndGeneric<T> = BothOfCellAndGeneric::Frozen(5);
fn function() {
let _ = &Self::UNFROZEN_VARIANT; //~ ERROR interior mutability
let _ = &Self::GENERIC_VARIANT; //~ ERROR interior mutability
let _ = &Self::FROZEN_VARIANT;
}
}
fn main() {
// constants defined in foreign crates
let _ = &helper::WRAPPED_PRIVATE_UNFROZEN_VARIANT; //~ ERROR interior mutability
let _ = &helper::WRAPPED_PRIVATE_FROZEN_VARIANT;
}

View file

@ -0,0 +1,75 @@
error: a `const` item with interior mutability should not be borrowed
--> $DIR/enums.rs:22:14
|
LL | let _ = &UNFROZEN_VARIANT; //~ ERROR interior mutability
| ^^^^^^^^^^^^^^^^
|
= note: `-D clippy::borrow-interior-mutable-const` implied by `-D warnings`
= help: assign this const to a local or static variable, and use the variable here
error: a `const` item with interior mutability should not be borrowed
--> $DIR/enums.rs:37:18
|
LL | let _ = &Self::TO_BE_FROZEN_VARIANT; //~ ERROR interior mutable
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: assign this const to a local or static variable, and use the variable here
error: a `const` item with interior mutability should not be borrowed
--> $DIR/enums.rs:41:18
|
LL | let _ = &Self::DEFAULTED_ON_FROZEN_VARIANT; //~ ERROR interior mutable
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: assign this const to a local or static variable, and use the variable here
error: a `const` item with interior mutability should not be borrowed
--> $DIR/enums.rs:50:18
|
LL | let _ = &<Self as AssocConsts>::TO_BE_UNFROZEN_VARIANT; //~ ERROR interior mutable
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: assign this const to a local or static variable, and use the variable here
error: a `const` item with interior mutability should not be borrowed
--> $DIR/enums.rs:52:18
|
LL | let _ = &Self::DEFAULTED_ON_UNFROZEN_VARIANT; //~ ERROR interior mutable
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: assign this const to a local or static variable, and use the variable here
error: a `const` item with interior mutability should not be borrowed
--> $DIR/enums.rs:74:18
|
LL | let _ = &<Self as AssocTypes>::TO_BE_UNFROZEN_VARIANT; //~ ERROR interior mutable
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: assign this const to a local or static variable, and use the variable here
error: a `const` item with interior mutability should not be borrowed
--> $DIR/enums.rs:91:18
|
LL | let _ = &Self::UNFROZEN_VARIANT; //~ ERROR interior mutability
| ^^^^^^^^^^^^^^^^^^^^^^
|
= help: assign this const to a local or static variable, and use the variable here
error: a `const` item with interior mutability should not be borrowed
--> $DIR/enums.rs:92:18
|
LL | let _ = &Self::GENERIC_VARIANT; //~ ERROR interior mutability
| ^^^^^^^^^^^^^^^^^^^^^
|
= help: assign this const to a local or static variable, and use the variable here
error: a `const` item with interior mutability should not be borrowed
--> $DIR/enums.rs:99:14
|
LL | let _ = &helper::WRAPPED_PRIVATE_UNFROZEN_VARIANT; //~ ERROR interior mutability
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: assign this const to a local or static variable, and use the variable here
error: aborting due to 9 previous errors

View file

@ -0,0 +1,123 @@
#![warn(clippy::declare_interior_mutable_const)]
use std::cell::Cell;
use std::sync::atomic::AtomicUsize;
enum OptionalCell {
Unfrozen(Cell<bool>),
Frozen,
}
// a constant with enums should be linted only when the used variant is unfrozen (#3962).
const UNFROZEN_VARIANT: OptionalCell = OptionalCell::Unfrozen(Cell::new(true)); //~ ERROR interior mutable
const FROZEN_VARIANT: OptionalCell = OptionalCell::Frozen;
const fn unfrozen_variant() -> OptionalCell {
OptionalCell::Unfrozen(Cell::new(false))
}
const fn frozen_variant() -> OptionalCell {
OptionalCell::Frozen
}
const UNFROZEN_VARIANT_FROM_FN: OptionalCell = unfrozen_variant(); //~ ERROR interior mutable
const FROZEN_VARIANT_FROM_FN: OptionalCell = frozen_variant();
enum NestedInnermost {
Unfrozen(AtomicUsize),
Frozen,
}
struct NestedInner {
inner: NestedInnermost,
}
enum NestedOuter {
NestedInner(NestedInner),
NotNested(usize),
}
struct NestedOutermost {
outer: NestedOuter,
}
// a constant with enums should be linted according to its value, no matter how structs involve.
const NESTED_UNFROZEN_VARIANT: NestedOutermost = NestedOutermost {
outer: NestedOuter::NestedInner(NestedInner {
inner: NestedInnermost::Unfrozen(AtomicUsize::new(2)),
}),
}; //~ ERROR interior mutable
const NESTED_FROZEN_VARIANT: NestedOutermost = NestedOutermost {
outer: NestedOuter::NestedInner(NestedInner {
inner: NestedInnermost::Frozen,
}),
};
trait AssocConsts {
// When there's no default value, lint it only according to its type.
// Further details are on the corresponding code (`NonCopyConst::check_trait_item`).
const TO_BE_UNFROZEN_VARIANT: OptionalCell; //~ ERROR interior mutable
const TO_BE_FROZEN_VARIANT: OptionalCell; //~ ERROR interior mutable
// Lint default values accordingly.
const DEFAULTED_ON_UNFROZEN_VARIANT: OptionalCell = OptionalCell::Unfrozen(Cell::new(false)); //~ ERROR interior mutable
const DEFAULTED_ON_FROZEN_VARIANT: OptionalCell = OptionalCell::Frozen;
}
// The lint doesn't trigger for an assoc constant in a trait impl with an unfrozen type even if it
// has enums. Further details are on the corresponding code in 'NonCopyConst::check_impl_item'.
impl AssocConsts for u64 {
const TO_BE_UNFROZEN_VARIANT: OptionalCell = OptionalCell::Unfrozen(Cell::new(false));
const TO_BE_FROZEN_VARIANT: OptionalCell = OptionalCell::Frozen;
// even if this sets an unfrozen variant, the lint ignores it.
const DEFAULTED_ON_FROZEN_VARIANT: OptionalCell = OptionalCell::Unfrozen(Cell::new(false));
}
// At first, I thought I'd need to check every patterns in `trait.rs`; but, what matters
// here are values; and I think substituted generics at definitions won't appear in MIR.
trait AssocTypes {
type ToBeUnfrozen;
const TO_BE_UNFROZEN_VARIANT: Option<Self::ToBeUnfrozen>;
const TO_BE_FROZEN_VARIANT: Option<Self::ToBeUnfrozen>;
}
impl AssocTypes for u64 {
type ToBeUnfrozen = AtomicUsize;
const TO_BE_UNFROZEN_VARIANT: Option<Self::ToBeUnfrozen> = Some(Self::ToBeUnfrozen::new(4)); //~ ERROR interior mutable
const TO_BE_FROZEN_VARIANT: Option<Self::ToBeUnfrozen> = None;
}
// Use raw pointers since direct generics have a false negative at the type level.
enum BothOfCellAndGeneric<T> {
Unfrozen(Cell<*const T>),
Generic(*const T),
Frozen(usize),
}
impl<T> BothOfCellAndGeneric<T> {
const UNFROZEN_VARIANT: BothOfCellAndGeneric<T> = BothOfCellAndGeneric::Unfrozen(Cell::new(std::ptr::null())); //~ ERROR interior mutable
// This is a false positive. The argument about this is on `is_value_unfrozen_raw`
const GENERIC_VARIANT: BothOfCellAndGeneric<T> = BothOfCellAndGeneric::Generic(std::ptr::null()); //~ ERROR interior mutable
const FROZEN_VARIANT: BothOfCellAndGeneric<T> = BothOfCellAndGeneric::Frozen(5);
// This is what is likely to be a false negative when one tries to fix
// the `GENERIC_VARIANT` false positive.
const NO_ENUM: Cell<*const T> = Cell::new(std::ptr::null()); //~ ERROR interior mutable
}
// associated types here is basically the same as the one above.
trait BothOfCellAndGenericWithAssocType {
type AssocType;
const UNFROZEN_VARIANT: BothOfCellAndGeneric<Self::AssocType> =
BothOfCellAndGeneric::Unfrozen(Cell::new(std::ptr::null())); //~ ERROR interior mutable
const GENERIC_VARIANT: BothOfCellAndGeneric<Self::AssocType> = BothOfCellAndGeneric::Generic(std::ptr::null()); //~ ERROR interior mutable
const FROZEN_VARIANT: BothOfCellAndGeneric<Self::AssocType> = BothOfCellAndGeneric::Frozen(5);
}
fn main() {}

View file

@ -0,0 +1,89 @@
error: a `const` item should never be interior mutable
--> $DIR/enums.rs:12:1
|
LL | const UNFROZEN_VARIANT: OptionalCell = OptionalCell::Unfrozen(Cell::new(true)); //~ ERROR interior mutable
| -----^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| |
| make this a static item (maybe with lazy_static)
|
= note: `-D clippy::declare-interior-mutable-const` implied by `-D warnings`
error: a `const` item should never be interior mutable
--> $DIR/enums.rs:23:1
|
LL | const UNFROZEN_VARIANT_FROM_FN: OptionalCell = unfrozen_variant(); //~ ERROR interior mutable
| -----^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| |
| make this a static item (maybe with lazy_static)
error: a `const` item should never be interior mutable
--> $DIR/enums.rs:45:1
|
LL | const NESTED_UNFROZEN_VARIANT: NestedOutermost = NestedOutermost {
| ^----
| |
| _make this a static item (maybe with lazy_static)
| |
LL | | outer: NestedOuter::NestedInner(NestedInner {
LL | | inner: NestedInnermost::Unfrozen(AtomicUsize::new(2)),
LL | | }),
LL | | }; //~ ERROR interior mutable
| |__^
error: a `const` item should never be interior mutable
--> $DIR/enums.rs:59:5
|
LL | const TO_BE_UNFROZEN_VARIANT: OptionalCell; //~ ERROR interior mutable
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: a `const` item should never be interior mutable
--> $DIR/enums.rs:60:5
|
LL | const TO_BE_FROZEN_VARIANT: OptionalCell; //~ ERROR interior mutable
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: a `const` item should never be interior mutable
--> $DIR/enums.rs:63:5
|
LL | const DEFAULTED_ON_UNFROZEN_VARIANT: OptionalCell = OptionalCell::Unfrozen(Cell::new(false)); //~ ERROR interior mutable
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: a `const` item should never be interior mutable
--> $DIR/enums.rs:89:5
|
LL | const TO_BE_UNFROZEN_VARIANT: Option<Self::ToBeUnfrozen> = Some(Self::ToBeUnfrozen::new(4)); //~ ERROR interior mutable
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: a `const` item should never be interior mutable
--> $DIR/enums.rs:101:5
|
LL | const UNFROZEN_VARIANT: BothOfCellAndGeneric<T> = BothOfCellAndGeneric::Unfrozen(Cell::new(std::ptr::null())); //~ ERROR interior mut...
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: a `const` item should never be interior mutable
--> $DIR/enums.rs:104:5
|
LL | const GENERIC_VARIANT: BothOfCellAndGeneric<T> = BothOfCellAndGeneric::Generic(std::ptr::null()); //~ ERROR interior mutable
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: a `const` item should never be interior mutable
--> $DIR/enums.rs:110:5
|
LL | const NO_ENUM: Cell<*const T> = Cell::new(std::ptr::null()); //~ ERROR interior mutable
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: a `const` item should never be interior mutable
--> $DIR/enums.rs:117:5
|
LL | / const UNFROZEN_VARIANT: BothOfCellAndGeneric<Self::AssocType> =
LL | | BothOfCellAndGeneric::Unfrozen(Cell::new(std::ptr::null())); //~ ERROR interior mutable
| |____________________________________________________________________^
error: a `const` item should never be interior mutable
--> $DIR/enums.rs:119:5
|
LL | const GENERIC_VARIANT: BothOfCellAndGeneric<Self::AssocType> = BothOfCellAndGeneric::Generic(std::ptr::null()); //~ ERROR interior mu...
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: aborting due to 12 previous errors