-//! Checks for uses of const which the type is not Freeze (Cell-free).
+//! Checks for uses of const which the type is not `Freeze` (`Cell`-free).
//!
//! This lint is **deny** by default.
-use rustc::lint::{LateContext, LateLintPass, Lint, LintArray, LintPass};
-use rustc::{declare_lint, lint_array};
-use rustc::hir::*;
+use std::ptr;
+
use rustc::hir::def::Def;
-use rustc::ty::{self, TypeFlags};
+use rustc::hir::*;
+use rustc::lint::{LateContext, LateLintPass, Lint, LintArray, LintPass};
use rustc::ty::adjustment::Adjust;
+use rustc::ty::{Ty, TypeFlags};
+use rustc::{declare_tool_lint, lint_array};
use rustc_errors::Applicability;
use rustc_typeck::hir_ty_to_ty;
-use syntax_pos::{DUMMY_SP, Span};
-use std::ptr;
+use syntax_pos::{Span, DUMMY_SP};
+
use crate::utils::{in_constant, in_macro, is_copy, span_lint_and_then};
-/// **What it does:** Checks for declaration of `const` items which is interior
-/// mutable (e.g. contains a `Cell`, `Mutex`, `AtomicXxxx` etc).
-///
-/// **Why is this bad?** Consts are copied everywhere they are referenced, i.e.
-/// every time you refer to the const a fresh instance of the `Cell` or `Mutex`
-/// or `AtomicXxxx` will be created, which defeats the whole purpose of using
-/// these types in the first place.
-///
-/// The `const` should better be replaced by a `static` item if a global
-/// variable is wanted, or replaced by a `const fn` if a constructor is wanted.
-///
-/// **Known problems:** A "non-constant" const item is a legacy way to supply an
-/// initialized value to downstream `static` items (e.g. the
-/// `std::sync::ONCE_INIT` constant). In this case the use of `const` is legit,
-/// and this lint should be suppressed.
-///
-/// **Example:**
-/// ```rust
-/// use std::sync::atomic::{Ordering::SeqCst, AtomicUsize};
-///
-/// // Bad.
-/// const CONST_ATOM: AtomicUsize = AtomicUsize::new(12);
-/// CONST_ATOM.store(6, SeqCst); // the content of the atomic is unchanged
-/// assert_eq!(CONST_ATOM.load(SeqCst), 12); // because the CONST_ATOM in these lines are distinct
-///
-/// // Good.
-/// static STATIC_ATOM: AtomicUsize = AtomicUsize::new(15);
-/// STATIC_ATOM.store(9, SeqCst);
-/// assert_eq!(STATIC_ATOM.load(SeqCst), 9); // use a `static` item to refer to the same instance
-/// ```
declare_clippy_lint! {
+ /// **What it does:** Checks for declaration of `const` items which is interior
+ /// mutable (e.g., contains a `Cell`, `Mutex`, `AtomicXxxx`, etc.).
+ ///
+ /// **Why is this bad?** Consts are copied everywhere they are referenced, i.e.,
+ /// every time you refer to the const a fresh instance of the `Cell` or `Mutex`
+ /// or `AtomicXxxx` will be created, which defeats the whole purpose of using
+ /// these types in the first place.
+ ///
+ /// The `const` should better be replaced by a `static` item if a global
+ /// variable is wanted, or replaced by a `const fn` if a constructor is wanted.
+ ///
+ /// **Known problems:** A "non-constant" const item is a legacy way to supply an
+ /// initialized value to downstream `static` items (e.g., the
+ /// `std::sync::ONCE_INIT` constant). In this case the use of `const` is legit,
+ /// and this lint should be suppressed.
+ ///
+ /// **Example:**
+ /// ```rust
+ /// use std::sync::atomic::{AtomicUsize, Ordering::SeqCst};
+ ///
+ /// // Bad.
+ /// const CONST_ATOM: AtomicUsize = AtomicUsize::new(12);
+ /// CONST_ATOM.store(6, SeqCst); // the content of the atomic is unchanged
+ /// assert_eq!(CONST_ATOM.load(SeqCst), 12); // because the CONST_ATOM in these lines are distinct
+ ///
+ /// // Good.
+ /// static STATIC_ATOM: AtomicUsize = AtomicUsize::new(15);
+ /// STATIC_ATOM.store(9, SeqCst);
+ /// assert_eq!(STATIC_ATOM.load(SeqCst), 9); // use a `static` item to refer to the same instance
+ /// ```
pub DECLARE_INTERIOR_MUTABLE_CONST,
correctness,
"declaring const with interior mutability"
}
-/// **What it does:** Checks if `const` items which is interior mutable (e.g.
-/// contains a `Cell`, `Mutex`, `AtomicXxxx` etc) has been borrowed directly.
-///
-/// **Why is this bad?** Consts are copied everywhere they are referenced, i.e.
-/// every time you refer to the const a fresh instance of the `Cell` or `Mutex`
-/// or `AtomicXxxx` will be created, which defeats the whole purpose of using
-/// these types in the first place.
-///
-/// The `const` value should be stored inside a `static` item.
-///
-/// **Known problems:** None
-///
-/// **Example:**
-/// ```rust
-/// use std::sync::atomic::{Ordering::SeqCst, AtomicUsize};
-/// const CONST_ATOM: AtomicUsize = AtomicUsize::new(12);
-///
-/// // Bad.
-/// CONST_ATOM.store(6, SeqCst); // the content of the atomic is unchanged
-/// assert_eq!(CONST_ATOM.load(SeqCst), 12); // because the CONST_ATOM in these lines are distinct
-///
-/// // Good.
-/// static STATIC_ATOM: AtomicUsize = CONST_ATOM;
-/// STATIC_ATOM.store(9, SeqCst);
-/// assert_eq!(STATIC_ATOM.load(SeqCst), 9); // use a `static` item to refer to the same instance
-/// ```
declare_clippy_lint! {
+ /// **What it does:** Checks if `const` items which is interior mutable (e.g.,
+ /// contains a `Cell`, `Mutex`, `AtomicXxxx`, etc.) has been borrowed directly.
+ ///
+ /// **Why is this bad?** Consts are copied everywhere they are referenced, i.e.,
+ /// every time you refer to the const a fresh instance of the `Cell` or `Mutex`
+ /// or `AtomicXxxx` will be created, which defeats the whole purpose of using
+ /// these types in the first place.
+ ///
+ /// The `const` value should be stored inside a `static` item.
+ ///
+ /// **Known problems:** None
+ ///
+ /// **Example:**
+ /// ```rust
+ /// use std::sync::atomic::{AtomicUsize, Ordering::SeqCst};
+ /// const CONST_ATOM: AtomicUsize = AtomicUsize::new(12);
+ ///
+ /// // Bad.
+ /// CONST_ATOM.store(6, SeqCst); // the content of the atomic is unchanged
+ /// assert_eq!(CONST_ATOM.load(SeqCst), 12); // because the CONST_ATOM in these lines are distinct
+ ///
+ /// // Good.
+ /// static STATIC_ATOM: AtomicUsize = CONST_ATOM;
+ /// STATIC_ATOM.store(9, SeqCst);
+ /// assert_eq!(STATIC_ATOM.load(SeqCst), 9); // use a `static` item to refer to the same instance
+ /// ```
pub BORROW_INTERIOR_MUTABLE_CONST,
correctness,
"referencing const with interior mutability"
#[derive(Copy, Clone)]
enum Source {
- Item {
- item: Span,
- },
- Assoc {
- item: Span,
- ty: Span,
- },
- Expr {
- expr: Span,
- },
+ Item { item: Span },
+ Assoc { item: Span, ty: Span },
+ Expr { expr: Span },
}
impl Source {
}
}
-fn verify_ty_bound<'a, 'tcx>(
- cx: &LateContext<'a, 'tcx>,
- ty: ty::Ty<'tcx>,
- source: Source,
-) {
+fn verify_ty_bound<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, ty: Ty<'tcx>, source: Source) {
if ty.is_freeze(cx.tcx, cx.param_env, DUMMY_SP) || is_copy(cx, ty) {
- // an UnsafeCell is !Copy, and an UnsafeCell is also the only type which
- // is !Freeze, thus if our type is Copy we can be sure it must be Freeze
+ // An `UnsafeCell` is `!Copy`, and an `UnsafeCell` is also the only type which
+ // is `!Freeze`, thus if our type is `Copy` we can be sure it must be `Freeze`
// as well.
return;
}
match source {
Source::Item { .. } => {
let const_kw_span = span.from_inner_byte_pos(0, 5);
- db.span_suggestion_with_applicability(
+ db.span_suggestion(
const_kw_span,
"make this a static item",
"static".to_string(),
Applicability::MachineApplicable,
);
- }
+ },
Source::Assoc { ty: ty_span, .. } => {
if ty.flags.contains(TypeFlags::HAS_FREE_LOCAL_NAMES) {
db.span_help(ty_span, &format!("consider requiring `{}` to be `Copy`", ty));
}
- }
+ },
Source::Expr { .. } => {
- db.help(
- "assign this const to a local or static variable, and use the variable here",
- );
- }
+ db.help("assign this const to a local or static variable, and use the variable here");
+ },
}
});
}
-
pub struct NonCopyConst;
impl LintPass for NonCopyConst {
fn get_lints(&self) -> LintArray {
lint_array!(DECLARE_INTERIOR_MUTABLE_CONST, BORROW_INTERIOR_MUTABLE_CONST)
}
+
+ fn name(&self) -> &'static str {
+ "NonCopyConst"
+ }
}
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NonCopyConst {
fn check_trait_item(&mut self, cx: &LateContext<'a, 'tcx>, trait_item: &'tcx TraitItem) {
if let TraitItemKind::Const(hir_ty, ..) = &trait_item.node {
let ty = hir_ty_to_ty(cx.tcx, hir_ty);
- verify_ty_bound(cx, ty, Source::Assoc { ty: hir_ty.span, item: trait_item.span });
+ verify_ty_bound(
+ cx,
+ ty,
+ Source::Assoc {
+ ty: hir_ty.span,
+ item: trait_item.span,
+ },
+ );
}
}
fn check_impl_item(&mut self, cx: &LateContext<'a, 'tcx>, impl_item: &'tcx ImplItem) {
if let ImplItemKind::Const(hir_ty, ..) = &impl_item.node {
- let item_node_id = cx.tcx.hir.get_parent_node(impl_item.id);
- let item = cx.tcx.hir.expect_item(item_node_id);
- // ensure the impl is an inherent impl.
+ let item_hir_id = cx.tcx.hir().get_parent_node_by_hir_id(impl_item.hir_id);
+ let item = cx.tcx.hir().expect_item_by_hir_id(item_hir_id);
+ // Ensure the impl is an inherent impl.
if let ItemKind::Impl(_, _, _, _, None, _, _) = item.node {
let ty = hir_ty_to_ty(cx.tcx, hir_ty);
- verify_ty_bound(cx, ty, Source::Assoc { ty: hir_ty.span, item: impl_item.span });
+ verify_ty_bound(
+ cx,
+ ty,
+ Source::Assoc {
+ ty: hir_ty.span,
+ item: impl_item.span,
+ },
+ );
}
}
}
fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
if let ExprKind::Path(qpath) = &expr.node {
// Only lint if we use the const item inside a function.
- if in_constant(cx, expr.id) {
+ if in_constant(cx, expr.hir_id) {
return;
}
- // make sure it is a const item.
+ // Make sure it is a const item.
match cx.tables.qpath_def(qpath, expr.hir_id) {
Def::Const(_) | Def::AssociatedConst(_) => {},
_ => return,
};
- // climb up to resolve any field access and explicit referencing.
+ // Climb up to resolve any field access and explicit referencing.
let mut cur_expr = expr;
let mut dereferenced_expr = expr;
let mut needs_check_adjustment = true;
loop {
- let parent_id = cx.tcx.hir.get_parent_node(cur_expr.id);
- if parent_id == cur_expr.id {
+ let parent_id = cx.tcx.hir().get_parent_node_by_hir_id(cur_expr.hir_id);
+ if parent_id == cur_expr.hir_id {
break;
}
- if let Some(map::NodeExpr(parent_expr)) = cx.tcx.hir.find(parent_id) {
+ if let Some(Node::Expr(parent_expr)) = cx.tcx.hir().find_by_hir_id(parent_id) {
match &parent_expr.node {
ExprKind::AddrOf(..) => {
- // `&e` => `e` must be referenced
+ // `&e` => `e` must be referenced.
needs_check_adjustment = false;
- }
+ },
ExprKind::Field(..) => {
dereferenced_expr = parent_expr;
needs_check_adjustment = true;
- }
+ },
ExprKind::Index(e, _) if ptr::eq(&**e, cur_expr) => {
// `e[i]` => desugared to `*Index::index(&e, i)`,
// meaning `e` must be referenced.
// no need to go further up since a method call is involved now.
needs_check_adjustment = false;
break;
- }
+ },
ExprKind::Unary(UnDeref, _) => {
// `*e` => desugared to `*Deref::deref(&e)`,
// meaning `e` must be referenced.
// no need to go further up since a method call is involved now.
needs_check_adjustment = false;
break;
- }
+ },
_ => break,
}
cur_expr = parent_expr;
adjustments[i - 1].target
}
} else {
- // No borrow adjustments = the entire const is moved.
+ // No borrow adjustments means the entire const is moved.
return;
}
} else {
projects / rust.git / blobdiff