clippy_lints/src/non_copy_const.rs

   1 //! Checks for uses of const which the type is not Freeze (Cell-free).
   2 //!
   3 //! This lint is **deny** by default.
   4
   5 use crate::utils::{in_constant, in_macro, is_copy, span_lint_and_then};
   6 use rustc::hir::def::Def;
   7 use rustc::hir::*;
   8 use rustc::lint::{LateContext, LateLintPass, Lint, LintArray, LintPass};
   9 use rustc::ty::adjustment::Adjust;
  10 use rustc::ty::{self, TypeFlags};
  11 use rustc::{declare_tool_lint, lint_array};
  12 use rustc_errors::Applicability;
  13 use rustc_typeck::hir_ty_to_ty;
  14 use std::ptr;
  15 use syntax_pos::{Span, DUMMY_SP};
  16
  17 /// **What it does:** Checks for declaration of `const` items which is interior
  18 /// mutable (e.g. contains a `Cell`, `Mutex`, `AtomicXxxx` etc).
  19 ///
  20 /// **Why is this bad?** Consts are copied everywhere they are referenced, i.e.
  21 /// every time you refer to the const a fresh instance of the `Cell` or `Mutex`
  22 /// or `AtomicXxxx` will be created, which defeats the whole purpose of using
  23 /// these types in the first place.
  24 ///
  25 /// The `const` should better be replaced by a `static` item if a global
  26 /// variable is wanted, or replaced by a `const fn` if a constructor is wanted.
  27 ///
  28 /// **Known problems:** A "non-constant" const item is a legacy way to supply an
  29 /// initialized value to downstream `static` items (e.g. the
  30 /// `std::sync::ONCE_INIT` constant). In this case the use of `const` is legit,
  31 /// and this lint should be suppressed.
  32 ///
  33 /// **Example:**
  34 /// ```rust
  35 /// use std::sync::atomic::{AtomicUsize, Ordering::SeqCst};
  36 ///
  37 /// // Bad.
  38 /// const CONST_ATOM: AtomicUsize = AtomicUsize::new(12);
  39 /// CONST_ATOM.store(6, SeqCst); // the content of the atomic is unchanged
  40 /// assert_eq!(CONST_ATOM.load(SeqCst), 12); // because the CONST_ATOM in these lines are distinct
  41 ///
  42 /// // Good.
  43 /// static STATIC_ATOM: AtomicUsize = AtomicUsize::new(15);
  44 /// STATIC_ATOM.store(9, SeqCst);
  45 /// assert_eq!(STATIC_ATOM.load(SeqCst), 9); // use a `static` item to refer to the same instance
  46 /// ```
  47 declare_clippy_lint! {
  48     pub DECLARE_INTERIOR_MUTABLE_CONST,
  49     correctness,
  50     "declaring const with interior mutability"
  51 }
  52
  53 /// **What it does:** Checks if `const` items which is interior mutable (e.g.
  54 /// contains a `Cell`, `Mutex`, `AtomicXxxx` etc) has been borrowed directly.
  55 ///
  56 /// **Why is this bad?** Consts are copied everywhere they are referenced, i.e.
  57 /// every time you refer to the const a fresh instance of the `Cell` or `Mutex`
  58 /// or `AtomicXxxx` will be created, which defeats the whole purpose of using
  59 /// these types in the first place.
  60 ///
  61 /// The `const` value should be stored inside a `static` item.
  62 ///
  63 /// **Known problems:** None
  64 ///
  65 /// **Example:**
  66 /// ```rust
  67 /// use std::sync::atomic::{AtomicUsize, Ordering::SeqCst};
  68 /// const CONST_ATOM: AtomicUsize = AtomicUsize::new(12);
  69 ///
  70 /// // Bad.
  71 /// CONST_ATOM.store(6, SeqCst); // the content of the atomic is unchanged
  72 /// assert_eq!(CONST_ATOM.load(SeqCst), 12); // because the CONST_ATOM in these lines are distinct
  73 ///
  74 /// // Good.
  75 /// static STATIC_ATOM: AtomicUsize = CONST_ATOM;
  76 /// STATIC_ATOM.store(9, SeqCst);
  77 /// assert_eq!(STATIC_ATOM.load(SeqCst), 9); // use a `static` item to refer to the same instance
  78 /// ```
  79 declare_clippy_lint! {
  80     pub BORROW_INTERIOR_MUTABLE_CONST,
  81     correctness,
  82     "referencing const with interior mutability"
  83 }
  84
  85 #[derive(Copy, Clone)]
  86 enum Source {
  87     Item { item: Span },
  88     Assoc { item: Span, ty: Span },
  89     Expr { expr: Span },
  90 }
  91
  92 impl Source {
  93     fn lint(&self) -> (&'static Lint, &'static str, Span) {
  94         match self {
  95             Source::Item { item } | Source::Assoc { item, .. } => (
  96                 DECLARE_INTERIOR_MUTABLE_CONST,
  97                 "a const item should never be interior mutable",
  98                 *item,
  99             ),
 100             Source::Expr { expr } => (
 101                 BORROW_INTERIOR_MUTABLE_CONST,
 102                 "a const item with interior mutability should not be borrowed",
 103                 *expr,
 104             ),
 105         }
 106     }
 107 }
 108
 109 fn verify_ty_bound<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, ty: ty::Ty<'tcx>, source: Source) {
 110     if ty.is_freeze(cx.tcx, cx.param_env, DUMMY_SP) || is_copy(cx, ty) {
 111         // an UnsafeCell is !Copy, and an UnsafeCell is also the only type which
 112         // is !Freeze, thus if our type is Copy we can be sure it must be Freeze
 113         // as well.
 114         return;
 115     }
 116
 117     let (lint, msg, span) = source.lint();
 118     span_lint_and_then(cx, lint, span, msg, |db| {
 119         if in_macro(span) {
 120             return; // Don't give suggestions into macros.
 121         }
 122         match source {
 123             Source::Item { .. } => {
 124                 let const_kw_span = span.from_inner_byte_pos(0, 5);
 125                 db.span_suggestion(
 126                     const_kw_span,
 127                     "make this a static item",
 128                     "static".to_string(),
 129                     Applicability::MachineApplicable,
 130                 );
 131             },
 132             Source::Assoc { ty: ty_span, .. } => {
 133                 if ty.flags.contains(TypeFlags::HAS_FREE_LOCAL_NAMES) {
 134                     db.span_help(ty_span, &format!("consider requiring `{}` to be `Copy`", ty));
 135                 }
 136             },
 137             Source::Expr { .. } => {
 138                 db.help("assign this const to a local or static variable, and use the variable here");
 139             },
 140         }
 141     });
 142 }
 143
 144 pub struct NonCopyConst;
 145
 146 impl LintPass for NonCopyConst {
 147     fn get_lints(&self) -> LintArray {
 148         lint_array!(DECLARE_INTERIOR_MUTABLE_CONST, BORROW_INTERIOR_MUTABLE_CONST)
 149     }
 150
 151     fn name(&self) -> &'static str {
 152         "NonCopyConst"
 153     }
 154 }
 155
 156 impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NonCopyConst {
 157     fn check_item(&mut self, cx: &LateContext<'a, 'tcx>, it: &'tcx Item) {
 158         if let ItemKind::Const(hir_ty, ..) = &it.node {
 159             let ty = hir_ty_to_ty(cx.tcx, hir_ty);
 160             verify_ty_bound(cx, ty, Source::Item { item: it.span });
 161         }
 162     }
 163
 164     fn check_trait_item(&mut self, cx: &LateContext<'a, 'tcx>, trait_item: &'tcx TraitItem) {
 165         if let TraitItemKind::Const(hir_ty, ..) = &trait_item.node {
 166             let ty = hir_ty_to_ty(cx.tcx, hir_ty);
 167             verify_ty_bound(
 168                 cx,
 169                 ty,
 170                 Source::Assoc {
 171                     ty: hir_ty.span,
 172                     item: trait_item.span,
 173                 },
 174             );
 175         }
 176     }
 177
 178     fn check_impl_item(&mut self, cx: &LateContext<'a, 'tcx>, impl_item: &'tcx ImplItem) {
 179         if let ImplItemKind::Const(hir_ty, ..) = &impl_item.node {
 180             let item_node_id = cx.tcx.hir().get_parent_node(impl_item.id);
 181             let item = cx.tcx.hir().expect_item(item_node_id);
 182             // ensure the impl is an inherent impl.
 183             if let ItemKind::Impl(_, _, _, _, None, _, _) = item.node {
 184                 let ty = hir_ty_to_ty(cx.tcx, hir_ty);
 185                 verify_ty_bound(
 186                     cx,
 187                     ty,
 188                     Source::Assoc {
 189                         ty: hir_ty.span,
 190                         item: impl_item.span,
 191                     },
 192                 );
 193             }
 194         }
 195     }
 196
 197     fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
 198         if let ExprKind::Path(qpath) = &expr.node {
 199             // Only lint if we use the const item inside a function.
 200             if in_constant(cx, expr.id) {
 201                 return;
 202             }
 203
 204             // make sure it is a const item.
 205             match cx.tables.qpath_def(qpath, expr.hir_id) {
 206                 Def::Const(_) | Def::AssociatedConst(_) => {},
 207                 _ => return,
 208             };
 209
 210             // climb up to resolve any field access and explicit referencing.
 211             let mut cur_expr = expr;
 212             let mut dereferenced_expr = expr;
 213             let mut needs_check_adjustment = true;
 214             loop {
 215                 let parent_id = cx.tcx.hir().get_parent_node(cur_expr.id);
 216                 if parent_id == cur_expr.id {
 217                     break;
 218                 }
 219                 if let Some(Node::Expr(parent_expr)) = cx.tcx.hir().find(parent_id) {
 220                     match &parent_expr.node {
 221                         ExprKind::AddrOf(..) => {
 222                             // `&e` => `e` must be referenced
 223                             needs_check_adjustment = false;
 224                         },
 225                         ExprKind::Field(..) => {
 226                             dereferenced_expr = parent_expr;
 227                             needs_check_adjustment = true;
 228                         },
 229                         ExprKind::Index(e, _) if ptr::eq(&**e, cur_expr) => {
 230                             // `e[i]` => desugared to `*Index::index(&e, i)`,
 231                             // meaning `e` must be referenced.
 232                             // no need to go further up since a method call is involved now.
 233                             needs_check_adjustment = false;
 234                             break;
 235                         },
 236                         ExprKind::Unary(UnDeref, _) => {
 237                             // `*e` => desugared to `*Deref::deref(&e)`,
 238                             // meaning `e` must be referenced.
 239                             // no need to go further up since a method call is involved now.
 240                             needs_check_adjustment = false;
 241                             break;
 242                         },
 243                         _ => break,
 244                     }
 245                     cur_expr = parent_expr;
 246                 } else {
 247                     break;
 248                 }
 249             }
 250
 251             let ty = if needs_check_adjustment {
 252                 let adjustments = cx.tables.expr_adjustments(dereferenced_expr);
 253                 if let Some(i) = adjustments.iter().position(|adj| match adj.kind {
 254                     Adjust::Borrow(_) | Adjust::Deref(_) => true,
 255                     _ => false,
 256                 }) {
 257                     if i == 0 {
 258                         cx.tables.expr_ty(dereferenced_expr)
 259                     } else {
 260                         adjustments[i - 1].target
 261                     }
 262                 } else {
 263                     // No borrow adjustments = the entire const is moved.
 264                     return;
 265                 }
 266             } else {
 267                 cx.tables.expr_ty(dereferenced_expr)
 268             };
 269
 270             verify_ty_bound(cx, ty, Source::Expr { expr: expr.span });
 271         }
 272     }
 273 }