diff --git a/CHANGELOG.md b/CHANGELOG.md index c8d41e3b31d2..33d7b0974ec0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1366,6 +1366,7 @@ Released 2018-09-13 [`new_without_default`]: https://rust-lang.github.io/rust-clippy/master/index.html#new_without_default [`no_effect`]: https://rust-lang.github.io/rust-clippy/master/index.html#no_effect [`non_ascii_literal`]: https://rust-lang.github.io/rust-clippy/master/index.html#non_ascii_literal +[`non_scalar_const`]: https://rust-lang.github.io/rust-clippy/master/index.html#non_scalar_const [`nonminimal_bool`]: https://rust-lang.github.io/rust-clippy/master/index.html#nonminimal_bool [`nonsensical_open_options`]: https://rust-lang.github.io/rust-clippy/master/index.html#nonsensical_open_options [`not_unsafe_ptr_arg_deref`]: https://rust-lang.github.io/rust-clippy/master/index.html#not_unsafe_ptr_arg_deref diff --git a/README.md b/README.md index 2181c296e9b2..7d6fcbc90986 100644 --- a/README.md +++ b/README.md @@ -5,7 +5,7 @@ A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code. -[There are 361 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) +[There are 362 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) We have a bunch of lint categories to allow you to choose how much Clippy is supposed to ~~annoy~~ help you: diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 791307ba43f4..921c23c7a6e7 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -270,6 +270,7 @@ pub mod new_without_default; pub mod no_effect; pub mod non_copy_const; pub mod non_expressive_names; +pub mod non_scalar_const; pub mod open_options; pub mod option_env_unwrap; pub mod overflow_check_conditional; @@ -722,6 +723,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: &non_expressive_names::JUST_UNDERSCORES_AND_DIGITS, &non_expressive_names::MANY_SINGLE_CHAR_NAMES, &non_expressive_names::SIMILAR_NAMES, + &non_scalar_const::NON_SCALAR_CONST, &open_options::NONSENSICAL_OPEN_OPTIONS, &option_env_unwrap::OPTION_ENV_UNWRAP, &overflow_check_conditional::OVERFLOW_CHECK_CONDITIONAL, @@ -1007,6 +1009,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|| box to_digit_is_some::ToDigitIsSome); let array_size_threshold = conf.array_size_threshold; store.register_late_pass(move || box large_stack_arrays::LargeStackArrays::new(array_size_threshold)); + store.register_late_pass(move || box non_scalar_const::NonScalarConst::new(array_size_threshold)); store.register_late_pass(move || box floating_point_arithmetic::FloatingPointArithmetic); store.register_early_pass(|| box as_conversions::AsConversions); store.register_early_pass(|| box utils::internal_lints::ProduceIce); @@ -1305,6 +1308,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&non_copy_const::DECLARE_INTERIOR_MUTABLE_CONST), LintId::of(&non_expressive_names::JUST_UNDERSCORES_AND_DIGITS), LintId::of(&non_expressive_names::MANY_SINGLE_CHAR_NAMES), + LintId::of(&non_scalar_const::NON_SCALAR_CONST), LintId::of(&open_options::NONSENSICAL_OPEN_OPTIONS), LintId::of(&option_env_unwrap::OPTION_ENV_UNWRAP), LintId::of(&overflow_check_conditional::OVERFLOW_CHECK_CONDITIONAL), @@ -1646,6 +1650,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&methods::SINGLE_CHAR_PATTERN), LintId::of(&misc::CMP_OWNED), LintId::of(&mutex_atomic::MUTEX_ATOMIC), + LintId::of(&non_scalar_const::NON_SCALAR_CONST), LintId::of(&redundant_clone::REDUNDANT_CLONE), LintId::of(&slow_vector_initialization::SLOW_VECTOR_INITIALIZATION), LintId::of(&trivially_copy_pass_by_ref::TRIVIALLY_COPY_PASS_BY_REF), diff --git a/clippy_lints/src/non_scalar_const.rs b/clippy_lints/src/non_scalar_const.rs new file mode 100644 index 000000000000..6b0c670b7af4 --- /dev/null +++ b/clippy_lints/src/non_scalar_const.rs @@ -0,0 +1,85 @@ +use crate::rustc_target::abi::LayoutOf; +use crate::utils::span_lint_and_then; +use if_chain::if_chain; +use rustc::mir::interpret::ConstValue; +use rustc::ty::{self, ConstKind}; +use rustc_errors::Applicability; +use rustc_hir::{Item, ItemKind}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_session::{declare_tool_lint, impl_lint_pass}; +use rustc_span::{BytePos, Pos, Span}; +use rustc_typeck::hir_ty_to_ty; + +declare_clippy_lint! { + /// **What it does:** Checks for large `const` non-scalar types (ie: array) that should + /// be defined as `static` instead. + /// + /// **Why is this bad?** Performance: const variables are inlined upon use. + /// Static items result in only one instance and has a fixed location in memory. + /// + /// **Known problems:** None. + /// + /// **Example:** + /// ```rust,ignore + /// // Bad + /// pub const a = [0u32; 1_000_000]; + /// + /// // Good + /// pub static a = [0u32; 1_000_000]; + /// ``` + pub NON_SCALAR_CONST, + perf, + "large non-scalar const variable may cause performance overhead" +} + +pub struct NonScalarConst { + maximum_allowed_size: u64, +} + +impl NonScalarConst { + #[must_use] + pub fn new(maximum_allowed_size: u64) -> Self { + Self { maximum_allowed_size } + } +} + +impl_lint_pass!(NonScalarConst => [NON_SCALAR_CONST]); + +impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NonScalarConst { + fn check_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx Item<'_>) { + if_chain! { + if !item.span.from_expansion(); + if let ItemKind::Const(hir_ty, _) = &item.kind; + let ty = hir_ty_to_ty(cx.tcx, hir_ty); + if let ty::Array(element_type, cst) = ty.kind; + if let ConstKind::Value(val) = cst.val; + if let ConstValue::Scalar(element_count) = val; + if let Ok(element_count) = element_count.to_machine_usize(&cx.tcx); + if let Ok(element_size) = cx.layout_of(element_type).map(|l| l.size.bytes()); + if self.maximum_allowed_size < element_count * element_size; + + then { + let hi_pos = item.ident.span.lo() - BytePos::from_usize(1); + let sugg_span = Span::new( + hi_pos - BytePos::from_usize("const".len()), + hi_pos, + item.span.ctxt(), + ); + span_lint_and_then( + cx, + NON_SCALAR_CONST, + item.span, + "large array defined as const", + |db| { + db.span_suggestion( + sugg_span, + "make this a static item", + "static".to_string(), + Applicability::MachineApplicable, + ); + } + ); + } + } + } +} diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs index fd948953ea23..d20a3b93ace8 100644 --- a/src/lintlist/mod.rs +++ b/src/lintlist/mod.rs @@ -6,7 +6,7 @@ pub use lint::Lint; pub use lint::LINT_LEVELS; // begin lint list, do not remove this comment, it’s used in `update_lints` -pub const ALL_LINTS: [Lint; 361] = [ +pub const ALL_LINTS: [Lint; 362] = [ Lint { name: "absurd_extreme_comparisons", group: "correctness", @@ -1463,6 +1463,13 @@ pub const ALL_LINTS: [Lint; 361] = [ deprecation: None, module: "unicode", }, + Lint { + name: "non_scalar_const", + group: "perf", + desc: "large non-scalar const variable may cause performance overhead", + deprecation: None, + module: "non_scalar_const", + }, Lint { name: "nonminimal_bool", group: "complexity", diff --git a/tests/ui/non_scalar_const.fixed b/tests/ui/non_scalar_const.fixed new file mode 100644 index 000000000000..de1bf17aee0f --- /dev/null +++ b/tests/ui/non_scalar_const.fixed @@ -0,0 +1,37 @@ +// run-rustfix + +#![warn(clippy::non_scalar_const)] +#![allow(dead_code)] + +#[derive(Clone, Copy)] +pub struct S { + pub data: [u64; 32], +} + +// Should lint +pub(crate) static FOO_PUB_CRATE: [u32; 1_000_000] = [0u32; 1_000_000]; +pub static FOO_PUB: [u32; 1_000_000] = [0u32; 1_000_000]; +static FOO: [u32; 1_000_000] = [0u32; 1_000_000]; + +// Good +pub(crate) const G_FOO_PUB_CRATE: [u32; 1_000] = [0u32; 1_000]; +pub const G_FOO_PUB: [u32; 1_000] = [0u32; 1_000]; +const G_FOO: [u32; 1_000] = [0u32; 1_000]; + +fn main() { + // Should lint + pub static BAR_PUB: [u32; 1_000_000] = [0u32; 1_000_000]; + static BAR: [u32; 1_000_000] = [0u32; 1_000_000]; + pub static BAR_STRUCT_PUB: [S; 5_000] = [S { data: [0; 32] }; 5_000]; + static BAR_STRUCT: [S; 5_000] = [S { data: [0; 32] }; 5_000]; + pub static BAR_S_PUB: [Option<&str>; 200_000] = [Some("str"); 200_000]; + static BAR_S: [Option<&str>; 200_000] = [Some("str"); 200_000]; + + // Good + pub const G_BAR_PUB: [u32; 1_000] = [0u32; 1_000]; + const G_BAR: [u32; 1_000] = [0u32; 1_000]; + pub const G_BAR_STRUCT_PUB: [S; 500] = [S { data: [0; 32] }; 500]; + const G_BAR_STRUCT: [S; 500] = [S { data: [0; 32] }; 500]; + pub const G_BAR_S_PUB: [Option<&str>; 200] = [Some("str"); 200]; + const G_BAR_S: [Option<&str>; 200] = [Some("str"); 200]; +} diff --git a/tests/ui/non_scalar_const.rs b/tests/ui/non_scalar_const.rs new file mode 100644 index 000000000000..0d48f04f902a --- /dev/null +++ b/tests/ui/non_scalar_const.rs @@ -0,0 +1,37 @@ +// run-rustfix + +#![warn(clippy::non_scalar_const)] +#![allow(dead_code)] + +#[derive(Clone, Copy)] +pub struct S { + pub data: [u64; 32], +} + +// Should lint +pub(crate) const FOO_PUB_CRATE: [u32; 1_000_000] = [0u32; 1_000_000]; +pub const FOO_PUB: [u32; 1_000_000] = [0u32; 1_000_000]; +const FOO: [u32; 1_000_000] = [0u32; 1_000_000]; + +// Good +pub(crate) const G_FOO_PUB_CRATE: [u32; 1_000] = [0u32; 1_000]; +pub const G_FOO_PUB: [u32; 1_000] = [0u32; 1_000]; +const G_FOO: [u32; 1_000] = [0u32; 1_000]; + +fn main() { + // Should lint + pub const BAR_PUB: [u32; 1_000_000] = [0u32; 1_000_000]; + const BAR: [u32; 1_000_000] = [0u32; 1_000_000]; + pub const BAR_STRUCT_PUB: [S; 5_000] = [S { data: [0; 32] }; 5_000]; + const BAR_STRUCT: [S; 5_000] = [S { data: [0; 32] }; 5_000]; + pub const BAR_S_PUB: [Option<&str>; 200_000] = [Some("str"); 200_000]; + const BAR_S: [Option<&str>; 200_000] = [Some("str"); 200_000]; + + // Good + pub const G_BAR_PUB: [u32; 1_000] = [0u32; 1_000]; + const G_BAR: [u32; 1_000] = [0u32; 1_000]; + pub const G_BAR_STRUCT_PUB: [S; 500] = [S { data: [0; 32] }; 500]; + const G_BAR_STRUCT: [S; 500] = [S { data: [0; 32] }; 500]; + pub const G_BAR_S_PUB: [Option<&str>; 200] = [Some("str"); 200]; + const G_BAR_S: [Option<&str>; 200] = [Some("str"); 200]; +} diff --git a/tests/ui/non_scalar_const.stderr b/tests/ui/non_scalar_const.stderr new file mode 100644 index 000000000000..ee37f2a46302 --- /dev/null +++ b/tests/ui/non_scalar_const.stderr @@ -0,0 +1,76 @@ +error: large array defined as const + --> $DIR/non_scalar_const.rs:12:1 + | +LL | pub(crate) const FOO_PUB_CRATE: [u32; 1_000_000] = [0u32; 1_000_000]; + | ^^^^^^^^^^^-----^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | | + | help: make this a static item: `static` + | + = note: `-D clippy::non-scalar-const` implied by `-D warnings` + +error: large array defined as const + --> $DIR/non_scalar_const.rs:13:1 + | +LL | pub const FOO_PUB: [u32; 1_000_000] = [0u32; 1_000_000]; + | ^^^^-----^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | | + | help: make this a static item: `static` + +error: large array defined as const + --> $DIR/non_scalar_const.rs:14:1 + | +LL | const FOO: [u32; 1_000_000] = [0u32; 1_000_000]; + | -----^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | | + | help: make this a static item: `static` + +error: large array defined as const + --> $DIR/non_scalar_const.rs:23:5 + | +LL | pub const BAR_PUB: [u32; 1_000_000] = [0u32; 1_000_000]; + | ^^^^-----^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | | + | help: make this a static item: `static` + +error: large array defined as const + --> $DIR/non_scalar_const.rs:24:5 + | +LL | const BAR: [u32; 1_000_000] = [0u32; 1_000_000]; + | -----^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | | + | help: make this a static item: `static` + +error: large array defined as const + --> $DIR/non_scalar_const.rs:25:5 + | +LL | pub const BAR_STRUCT_PUB: [S; 5_000] = [S { data: [0; 32] }; 5_000]; + | ^^^^-----^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | | + | help: make this a static item: `static` + +error: large array defined as const + --> $DIR/non_scalar_const.rs:26:5 + | +LL | const BAR_STRUCT: [S; 5_000] = [S { data: [0; 32] }; 5_000]; + | -----^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | | + | help: make this a static item: `static` + +error: large array defined as const + --> $DIR/non_scalar_const.rs:27:5 + | +LL | pub const BAR_S_PUB: [Option<&str>; 200_000] = [Some("str"); 200_000]; + | ^^^^-----^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | | + | help: make this a static item: `static` + +error: large array defined as const + --> $DIR/non_scalar_const.rs:28:5 + | +LL | const BAR_S: [Option<&str>; 200_000] = [Some("str"); 200_000]; + | -----^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | | + | help: make this a static item: `static` + +error: aborting due to 9 previous errors +