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