Skip to content

Commit

Permalink
rewrite the test and fix a minor fp
Browse files Browse the repository at this point in the history 
* rewrite the test for `declare_interior_mutable_const from scratch`

* fix a minor false positive where `Cell<"const T>` gets linted twice
  • Loading branch information
@rail-rain
rail-rain committed Sep 17, 2020
1 parent d655c0a commit 2fc9064
Show file tree
Hide file tree
Showing 3 changed files with 166 additions and 75 deletions.
24 changes: 13 additions & 11 deletions clippy_lints/src/non_copy_const.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,7 @@ use rustc_hir::{Expr, ExprKind, ImplItem, ImplItemKind, Item, ItemKind, Node, Tr
use rustc_infer::traits::specialization_graph;
use rustc_lint::{LateContext, LateLintPass, Lint};
use rustc_middle::ty::adjustment::Adjust;
use rustc_middle::ty::fold::TypeFoldable as _;
use rustc_middle::ty::{AssocKind, Ty, TypeFlags};
use rustc_middle::ty::{AssocKind, Ty};
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::{InnerSpan, Span, DUMMY_SP};
use rustc_typeck::hir_ty_to_ty;
Expand Down Expand Up @@ -178,15 +177,18 @@ impl<'tcx> LateLintPass<'tcx> for NonCopyConst {
if let Some(of_trait_def_id) = of_trait_ref.trait_def_id();
if let Some(of_assoc_item) = specialization_graph::Node::Trait(of_trait_def_id)
.item(cx.tcx, impl_item.ident, AssocKind::Const, of_trait_def_id);
if cx.tcx
// Normalize assoc types because ones originated from generic params
// bounded other traits could have their bound at the trait defs;
// and, in that case, the definition is *not* generic.
.normalize_erasing_regions(
cx.tcx.param_env(of_trait_def_id),
cx.tcx.type_of(of_assoc_item.def_id),
)
.has_type_flags(TypeFlags::HAS_PROJECTION | TypeFlags::HAS_TY_PARAM);
if cx
.tcx
.layout_of(cx.tcx.param_env(of_trait_def_id).and(
// Normalize assoc types because ones originated from generic params
// bounded other traits could have their bound at the trait defs;
// and, in that case, the definition is *not* generic.
cx.tcx.normalize_erasing_regions(
cx.tcx.param_env(of_trait_def_id),
cx.tcx.type_of(of_assoc_item.def_id),
),
))
.is_err();
then {
let ty = hir_ty_to_ty(cx.tcx, hir_ty);
let normalized = cx.tcx.normalize_erasing_regions(cx.param_env, ty);
Expand Down
159 changes: 115 additions & 44 deletions tests/ui/declare_interior_mutable_const.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,64 +34,135 @@ static STATIC_TUPLE: (AtomicUsize, String) = (ATOMIC, STRING);
#[allow(clippy::declare_interior_mutable_const)]
const ONCE_INIT: Once = Once::new();

struct Wrapper<T>(T);

trait Trait<T: Trait2<AssocType5 = AtomicUsize>> {
type AssocType;
type AssocType2;
type AssocType3;

// a constant whose type is a concrete type should be linted at the definition site.
trait ConcreteTypes {
const ATOMIC: AtomicUsize; //~ ERROR interior mutable
const INTEGER: u64;
const STRING: String;
const SELF: Self;
const INPUT: T;
const INPUT_ASSOC: T::AssocType4;
const INPUT_ASSOC_2: T::AssocType5; //~ ERROR interior mutable
const ASSOC: Self::AssocType;
const ASSOC_2: Self::AssocType2;
const WRAPPED_ASSOC_2: Wrapper<Self::AssocType2>;
const WRAPPED_ASSOC_3: Wrapper<Self::AssocType3>;

const AN_INPUT: T = Self::INPUT;
declare_const!(ANOTHER_INPUT: T = Self::INPUT);
declare_const!(ANOTHER_ATOMIC: AtomicUsize = Self::ATOMIC); //~ ERROR interior mutable
}

trait Trait2 {
type AssocType4;
type AssocType5;
impl ConcreteTypes for u64 {
const ATOMIC: AtomicUsize = AtomicUsize::new(9);
const INTEGER: u64 = 10;
const STRING: String = String::new();
}

const SELF_2: Self;
const ASSOC_4: Self::AssocType4;
// a helper trait used below
trait ConstDefault {
const DEFAULT: Self;
}

impl<T: Trait2<AssocType5 = AtomicUsize>> Trait<T> for u64 {
type AssocType = u16;
type AssocType2 = AtomicUsize;
type AssocType3 = T;
// a constant whose type is a generic type should be linted at the implementation site.
trait GenericTypes<T, U> {
const TO_REMAIN_GENERIC: T;
const TO_BE_CONCRETE: U;

const ATOMIC: AtomicUsize = AtomicUsize::new(9);
const INTEGER: u64 = 10;
const STRING: String = String::new();
const SELF: Self = 11;
const INPUT: T = T::SELF_2;
const INPUT_ASSOC: T::AssocType4 = T::ASSOC_4;
const INPUT_ASSOC_2: T::AssocType5 = AtomicUsize::new(16);
const ASSOC: Self::AssocType = 13;
const ASSOC_2: Self::AssocType2 = AtomicUsize::new(15); //~ ERROR interior mutable
const WRAPPED_ASSOC_2: Wrapper<Self::AssocType2> = Wrapper(AtomicUsize::new(16)); //~ ERROR interior mutable
const WRAPPED_ASSOC_3: Wrapper<Self::AssocType3> = Wrapper(T::SELF_2);
const HAVING_DEFAULT: T = Self::TO_REMAIN_GENERIC;
declare_const!(IN_MACRO: T = Self::TO_REMAIN_GENERIC);
}

impl<T: ConstDefault> GenericTypes<T, AtomicUsize> for u64 {
const TO_REMAIN_GENERIC: T = T::DEFAULT;
const TO_BE_CONCRETE: AtomicUsize = AtomicUsize::new(11); //~ ERROR interior mutable
}

// a helper type used below
struct Wrapper<T>(T);

// a constant whose type is an associated type should be linted at the implementation site, too.
trait AssocTypes {
type ToBeFrozen;
type ToBeUnfrozen;
type ToBeGenericParam;

const TO_BE_FROZEN: Self::ToBeFrozen;
const TO_BE_UNFROZEN: Self::ToBeUnfrozen;
const WRAPPED_TO_BE_UNFROZEN: Wrapper<Self::ToBeUnfrozen>;
// to ensure it can handle things when a generic type remains after normalization.
const WRAPPED_TO_BE_GENERIC_PARAM: Wrapper<Self::ToBeGenericParam>;
}

impl<T: ConstDefault> AssocTypes for Vec<T> {
type ToBeFrozen = u16;
type ToBeUnfrozen = AtomicUsize;
type ToBeGenericParam = T;

const TO_BE_FROZEN: Self::ToBeFrozen = 12;
const TO_BE_UNFROZEN: Self::ToBeUnfrozen = AtomicUsize::new(13); //~ ERROR interior mutable
const WRAPPED_TO_BE_UNFROZEN: Wrapper<Self::ToBeUnfrozen> = Wrapper(AtomicUsize::new(14)); //~ ERROR interior mutable
const WRAPPED_TO_BE_GENERIC_PARAM: Wrapper<Self::ToBeGenericParam> = Wrapper(T::DEFAULT);
}

struct Local<T, U>(T, U);
// a helper trait used below
trait AssocTypesHelper {
type NotToBeBounded;
type ToBeBounded;

impl<T: Trait<U>, U: Trait2<AssocType5 = AtomicUsize>> Local<T, U> {
const ASSOC_5: AtomicUsize = AtomicUsize::new(14); //~ ERROR interior mutable
const NOT_TO_BE_BOUNDED: Self::NotToBeBounded;
}

// a constant whose type is an assoc type originated from a generic param bounded at the definition
// site should be linted at there.
trait AssocTypesFromGenericParam<T>
where
T: AssocTypesHelper<ToBeBounded = AtomicUsize>,
{
const NOT_BOUNDED: T::NotToBeBounded;
const BOUNDED: T::ToBeBounded; //~ ERROR interior mutable
}

impl<T> AssocTypesFromGenericParam<T> for u64
where
T: AssocTypesHelper<ToBeBounded = AtomicUsize>,
{
// an associated type could remain unknown in a trait impl.
const NOT_BOUNDED: T::NotToBeBounded = T::NOT_TO_BE_BOUNDED;
const BOUNDED: T::ToBeBounded = AtomicUsize::new(15);
}

trait SelfType {
const SELF: Self;
}

impl SelfType for u64 {
const SELF: Self = 16;
}

impl SelfType for AtomicUsize {
// this (interior mutable `Self` const) exists in `parking_lot`.
// `const_trait_impl` will replace it in the future, hopefully.
const SELF: Self = AtomicUsize::new(17); //~ ERROR interior mutable
}

// Even though a constant contains a generic type, if it also have a interior mutable type,
// it should be linted at the definition site.
trait BothOfCellAndGeneric<T> {
// this is a false negative in the current implementation.
const DIRECT: Cell<T>;
const INDIRECT: Cell<*const T>; //~ ERROR interior mutable
}

impl<T: ConstDefault> BothOfCellAndGeneric<T> for u64 {
const DIRECT: Cell<T> = Cell::new(T::DEFAULT);
const INDIRECT: Cell<*const T> = Cell::new(std::ptr::null());
}

struct Local<T>(T);

// a constant in an inherent impl are essentially the same as a normal const item
// except there can be a generic or associated type.
impl<T> Local<T>
where
T: ConstDefault + AssocTypesHelper<ToBeBounded = AtomicUsize>,
{
const ATOMIC: AtomicUsize = AtomicUsize::new(18); //~ ERROR interior mutable
const COW: Cow<'static, str> = Cow::Borrowed("tuvwxy");
const U_SELF: U = U::SELF_2;
const T_ASSOC: T::AssocType = T::ASSOC;
const U_ASSOC: U::AssocType5 = AtomicUsize::new(17); //~ ERROR interior mutable

const GENERIC_TYPE: T = T::DEFAULT;

const ASSOC_TYPE: T::NotToBeBounded = T::NOT_TO_BE_BOUNDED;
const BOUNDED_ASSOC_TYPE: T::ToBeBounded = AtomicUsize::new(19); //~ ERROR interior mutable
}

fn main() {}
58 changes: 38 additions & 20 deletions tests/ui/declare_interior_mutable_const.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,17 +36,11 @@ LL | declare_const!(_ONCE: Once = Once::new()); //~ ERROR interior mutable
= note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)

error: a `const` item should never be interior mutable
--> $DIR/declare_interior_mutable_const.rs:44:5
--> $DIR/declare_interior_mutable_const.rs:39:5
|
LL | const ATOMIC: AtomicUsize; //~ ERROR interior mutable
| ^^^^^^^^^^^^^^^^^^^^^^^^^^

error: a `const` item should never be interior mutable
--> $DIR/declare_interior_mutable_const.rs:50:5
|
LL | const INPUT_ASSOC_2: T::AssocType5; //~ ERROR interior mutable
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: a `const` item should never be interior mutable
--> $DIR/declare_interior_mutable_const.rs:16:9
|
Expand All @@ -59,28 +53,52 @@ LL | declare_const!(ANOTHER_ATOMIC: AtomicUsize = Self::ATOMIC); //~ ERROR i
= note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)

error: a `const` item should never be interior mutable
--> $DIR/declare_interior_mutable_const.rs:82:5
--> $DIR/declare_interior_mutable_const.rs:67:5
|
LL | const TO_BE_CONCRETE: AtomicUsize = AtomicUsize::new(11); //~ ERROR interior mutable
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: a `const` item should never be interior mutable
--> $DIR/declare_interior_mutable_const.rs:92:5
|
LL | const TO_BE_UNFROZEN: Self::ToBeUnfrozen = AtomicUsize::new(13); //~ ERROR interior mutable
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: a `const` item should never be interior mutable
--> $DIR/declare_interior_mutable_const.rs:93:5
|
LL | const WRAPPED_TO_BE_UNFROZEN: Wrapper<Self::ToBeUnfrozen> = Wrapper(AtomicUsize::new(14)); //~ ERROR interior mutable
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: a `const` item should never be interior mutable
--> $DIR/declare_interior_mutable_const.rs:112:5
|
LL | const BOUNDED: T::ToBeBounded; //~ ERROR interior mutable
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: a `const` item should never be interior mutable
--> $DIR/declare_interior_mutable_const.rs:135:5
|
LL | const ASSOC_2: Self::AssocType2 = AtomicUsize::new(15); //~ ERROR interior mutable
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
LL | const SELF: Self = AtomicUsize::new(17); //~ ERROR interior mutable
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: a `const` item should never be interior mutable
--> $DIR/declare_interior_mutable_const.rs:83:5
--> $DIR/declare_interior_mutable_const.rs:143:5
|
LL | const WRAPPED_ASSOC_2: Wrapper<Self::AssocType2> = Wrapper(AtomicUsize::new(16)); //~ ERROR interior mutable
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
LL | const INDIRECT: Cell<*const T>; //~ ERROR interior mutable
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: a `const` item should never be interior mutable
--> $DIR/declare_interior_mutable_const.rs:90:5
--> $DIR/declare_interior_mutable_const.rs:159:5
|
LL | const ASSOC_5: AtomicUsize = AtomicUsize::new(14); //~ ERROR interior mutable
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
LL | const ATOMIC: AtomicUsize = AtomicUsize::new(18); //~ ERROR interior mutable
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: a `const` item should never be interior mutable
--> $DIR/declare_interior_mutable_const.rs:94:5
--> $DIR/declare_interior_mutable_const.rs:165:5
|
LL | const U_ASSOC: U::AssocType5 = AtomicUsize::new(17); //~ ERROR interior mutable
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
LL | const BOUNDED_ASSOC_TYPE: T::ToBeBounded = AtomicUsize::new(19); //~ ERROR interior mutable
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to 11 previous errors
error: aborting due to 14 previous errors

0 comments on commit 2fc9064

Please sign in to comment.