clippy_lints/src/only_used_in_recursion.rs

   1 use std::collections::VecDeque;
   2
   3 use clippy_utils::diagnostics::span_lint_and_sugg;
   4 use itertools::{izip, Itertools};
   5 use rustc_ast::{walk_list, Label, Mutability};
   6 use rustc_data_structures::fx::{FxHashMap, FxHashSet};
   7 use rustc_errors::Applicability;
   8 use rustc_hir::def::{DefKind, Res};
   9 use rustc_hir::def_id::DefId;
  10 use rustc_hir::definitions::{DefPathData, DisambiguatedDefPathData};
  11 use rustc_hir::intravisit::{walk_expr, FnKind, Visitor};
  12 use rustc_hir::{
  13     Arm, Block, Body, Expr, ExprKind, Guard, HirId, ImplicitSelfKind, Let, Local, Pat, PatKind, Path, PathSegment,
  14     QPath, Stmt, StmtKind, TyKind, UnOp,
  15 };
  16 use rustc_lint::{LateContext, LateLintPass};
  17 use rustc_middle::ty;
  18 use rustc_middle::ty::{Ty, TyCtxt, TypeckResults};
  19 use rustc_session::{declare_lint_pass, declare_tool_lint};
  20 use rustc_span::symbol::kw;
  21 use rustc_span::symbol::Ident;
  22 use rustc_span::Span;
  23
  24 declare_clippy_lint! {
  25     /// ### What it does
  26     /// Checks for arguments that are only used in recursion with no side-effects.
  27     ///
  28     /// ### Why is this bad?
  29     /// It could contain a useless calculation and can make function simpler.
  30     ///
  31     /// The arguments can be involved in calculations and assignments but as long as
  32     /// the calculations have no side-effects (function calls or mutating dereference)
  33     /// and the assigned variables are also only in recursion, it is useless.
  34     ///
  35     /// ### Known problems
  36     /// In some cases, this would not catch all useless arguments.
  37     ///
  38     /// ```rust
  39     /// fn foo(a: usize, b: usize) -> usize {
  40     ///     let f = |x| x + 1;
  41     ///
  42     ///     if a == 0 {
  43     ///         1
  44     ///     } else {
  45     ///         foo(a - 1, f(b))
  46     ///     }
  47     /// }
  48     /// ```
  49     ///
  50     /// For example, the argument `b` is only used in recursion, but the lint would not catch it.
  51     ///
  52     /// List of some examples that can not be caught:
  53     /// - binary operation of non-primitive types
  54     /// - closure usage
  55     /// - some `break` relative operations
  56     /// - struct pattern binding
  57     ///
  58     /// Also, when you recurse the function name with path segments, it is not possible to detect.
  59     ///
  60     /// ### Example
  61     /// ```rust
  62     /// fn f(a: usize, b: usize) -> usize {
  63     ///     if a == 0 {
  64     ///         1
  65     ///     } else {
  66     ///         f(a - 1, b + 1)
  67     ///     }
  68     /// }
  69     /// # fn main() {
  70     /// #     print!("{}", f(1, 1));
  71     /// # }
  72     /// ```
  73     /// Use instead:
  74     /// ```rust
  75     /// fn f(a: usize) -> usize {
  76     ///     if a == 0 {
  77     ///         1
  78     ///     } else {
  79     ///         f(a - 1)
  80     ///     }
  81     /// }
  82     /// # fn main() {
  83     /// #     print!("{}", f(1));
  84     /// # }
  85     /// ```
  86     #[clippy::version = "1.60.0"]
  87     pub ONLY_USED_IN_RECURSION,
  88     complexity,
  89     "arguments that is only used in recursion can be removed"
  90 }
  91 declare_lint_pass!(OnlyUsedInRecursion => [ONLY_USED_IN_RECURSION]);
  92
  93 impl<'tcx> LateLintPass<'tcx> for OnlyUsedInRecursion {
  94     fn check_fn(
  95         &mut self,
  96         cx: &LateContext<'tcx>,
  97         kind: FnKind<'tcx>,
  98         decl: &'tcx rustc_hir::FnDecl<'tcx>,
  99         body: &'tcx Body<'tcx>,
 100         _: Span,
 101         id: HirId,
 102     ) {
 103         if let FnKind::ItemFn(ident, ..) | FnKind::Method(ident, ..) = kind {
 104             let def_id = id.owner.to_def_id();
 105             let data = cx.tcx.def_path(def_id).data;
 106
 107             if data.len() > 1 {
 108                 match data.get(data.len() - 2) {
 109                     Some(DisambiguatedDefPathData {
 110                         data: DefPathData::Impl,
 111                         disambiguator,
 112                     }) if *disambiguator != 0 => return,
 113                     _ => {},
 114                 }
 115             }
 116
 117             let has_self = !matches!(decl.implicit_self, ImplicitSelfKind::None);
 118
 119             let ty_res = cx.typeck_results();
 120             let param_span = body
 121                 .params
 122                 .iter()
 123                 .flat_map(|param| {
 124                     let mut v = Vec::new();
 125                     param.pat.each_binding(|_, hir_id, span, ident| {
 126                         v.push((hir_id, span, ident));
 127                     });
 128                     v
 129                 })
 130                 .skip(if has_self { 1 } else { 0 })
 131                 .filter(|(_, _, ident)| !ident.name.as_str().starts_with('_'))
 132                 .collect_vec();
 133
 134             let params = body.params.iter().map(|param| param.pat).collect();
 135
 136             let mut visitor = SideEffectVisit {
 137                 graph: FxHashMap::default(),
 138                 has_side_effect: FxHashSet::default(),
 139                 ret_vars: Vec::new(),
 140                 contains_side_effect: false,
 141                 break_vars: FxHashMap::default(),
 142                 params,
 143                 fn_ident: ident,
 144                 fn_def_id: def_id,
 145                 is_method: matches!(kind, FnKind::Method(..)),
 146                 has_self,
 147                 ty_res,
 148                 ty_ctx: cx.tcx,
 149             };
 150
 151             visitor.visit_expr(&body.value);
 152             let vars = std::mem::take(&mut visitor.ret_vars);
 153             // this would set the return variables to side effect
 154             visitor.add_side_effect(vars);
 155
 156             let mut queue = visitor.has_side_effect.iter().copied().collect::<VecDeque<_>>();
 157
 158             // a simple BFS to check all the variables that have side effect
 159             while let Some(id) = queue.pop_front() {
 160                 if let Some(next) = visitor.graph.get(&id) {
 161                     for i in next {
 162                         if !visitor.has_side_effect.contains(i) {
 163                             visitor.has_side_effect.insert(*i);
 164                             queue.push_back(*i);
 165                         }
 166                     }
 167                 }
 168             }
 169
 170             for (id, span, ident) in param_span {
 171                 // if the variable is not used in recursion, it would be marked as unused
 172                 if !visitor.has_side_effect.contains(&id) {
 173                     let mut queue = VecDeque::new();
 174                     let mut visited = FxHashSet::default();
 175
 176                     queue.push_back(id);
 177
 178                     // a simple BFS to check the graph can reach to itself
 179                     // if it can't, it means the variable is never used in recursion
 180                     while let Some(id) = queue.pop_front() {
 181                         if let Some(next) = visitor.graph.get(&id) {
 182                             for i in next {
 183                                 if !visited.contains(i) {
 184                                     visited.insert(id);
 185                                     queue.push_back(*i);
 186                                 }
 187                             }
 188                         }
 189                     }
 190
 191                     if visited.contains(&id) {
 192                         span_lint_and_sugg(
 193                             cx,
 194                             ONLY_USED_IN_RECURSION,
 195                             span,
 196                             "parameter is only used in recursion",
 197                             "if this is intentional, prefix with an underscore",
 198                             format!("_{}", ident.name.as_str()),
 199                             Applicability::MaybeIncorrect,
 200                         );
 201                     }
 202                 }
 203             }
 204         }
 205     }
 206 }
 207
 208 pub fn is_primitive(ty: Ty<'_>) -> bool {
 209     match ty.kind() {
 210         ty::Bool | ty::Char | ty::Int(_) | ty::Uint(_) | ty::Float(_) | ty::Str => true,
 211         ty::Ref(_, t, _) => is_primitive(*t),
 212         _ => false,
 213     }
 214 }
 215
 216 pub fn is_array(ty: Ty<'_>) -> bool {
 217     match ty.kind() {
 218         ty::Array(..) | ty::Slice(..) => true,
 219         ty::Ref(_, t, _) => is_array(*t),
 220         _ => false,
 221     }
 222 }
 223
 224 /// This builds the graph of side effect.
 225 /// The edge `a -> b` means if `a` has side effect, `b` will have side effect.
 226 ///
 227 /// There are some example in following code:
 228 /// ```rust, ignore
 229 /// let b = 1;
 230 /// let a = b; // a -> b
 231 /// let (c, d) = (a, b); // c -> b, d -> b
 232 ///
 233 /// let e = if a == 0 { // e -> a
 234 ///     c // e -> c
 235 /// } else {
 236 ///     d // e -> d
 237 /// };
 238 /// ```
 239 pub struct SideEffectVisit<'tcx> {
 240     graph: FxHashMap<HirId, FxHashSet<HirId>>,
 241     has_side_effect: FxHashSet<HirId>,
 242     // bool for if the variable was dereferenced from mutable reference
 243     ret_vars: Vec<(HirId, bool)>,
 244     contains_side_effect: bool,
 245     // break label
 246     break_vars: FxHashMap<Ident, Vec<(HirId, bool)>>,
 247     params: Vec<&'tcx Pat<'tcx>>,
 248     fn_ident: Ident,
 249     fn_def_id: DefId,
 250     is_method: bool,
 251     has_self: bool,
 252     ty_res: &'tcx TypeckResults<'tcx>,
 253     ty_ctx: TyCtxt<'tcx>,
 254 }
 255
 256 impl<'tcx> Visitor<'tcx> for SideEffectVisit<'tcx> {
 257     fn visit_block(&mut self, b: &'tcx Block<'tcx>) {
 258         b.stmts.iter().for_each(|stmt| {
 259             self.visit_stmt(stmt);
 260             self.ret_vars.clear();
 261         });
 262         walk_list!(self, visit_expr, b.expr);
 263     }
 264
 265     fn visit_stmt(&mut self, s: &'tcx Stmt<'tcx>) {
 266         match s.kind {
 267             StmtKind::Local(Local {
 268                 pat, init: Some(init), ..
 269             }) => {
 270                 self.visit_pat_expr(pat, init, false);
 271                 self.ret_vars.clear();
 272             },
 273             StmtKind::Item(i) => {
 274                 let item = self.ty_ctx.hir().item(i);
 275                 self.visit_item(item);
 276                 self.ret_vars.clear();
 277             },
 278             StmtKind::Expr(e) | StmtKind::Semi(e) => {
 279                 self.visit_expr(e);
 280                 self.ret_vars.clear();
 281             },
 282             StmtKind::Local(_) => {},
 283         }
 284     }
 285
 286     fn visit_expr(&mut self, ex: &'tcx Expr<'tcx>) {
 287         match ex.kind {
 288             ExprKind::Array(exprs) | ExprKind::Tup(exprs) => {
 289                 self.ret_vars = exprs
 290                     .iter()
 291                     .flat_map(|expr| {
 292                         self.visit_expr(expr);
 293                         std::mem::take(&mut self.ret_vars)
 294                     })
 295                     .collect();
 296             },
 297             ExprKind::Call(callee, args) => self.visit_fn(callee, args),
 298             ExprKind::MethodCall(path, args, _) => self.visit_method_call(path, args),
 299             ExprKind::Binary(_, lhs, rhs) => {
 300                 self.visit_bin_op(lhs, rhs);
 301             },
 302             ExprKind::Unary(op, expr) => self.visit_un_op(op, expr),
 303             ExprKind::Let(Let { pat, init, .. }) => self.visit_pat_expr(pat, init, false),
 304             ExprKind::If(bind, then_expr, else_expr) => {
 305                 self.visit_if(bind, then_expr, else_expr);
 306             },
 307             ExprKind::Match(expr, arms, _) => self.visit_match(expr, arms),
 308             // since analysing the closure is not easy, just set all variables in it to side-effect
 309             ExprKind::Closure(_, _, body_id, _, _) => {
 310                 let body = self.ty_ctx.hir().body(body_id);
 311                 self.visit_body(body);
 312                 let vars = std::mem::take(&mut self.ret_vars);
 313                 self.add_side_effect(vars);
 314             },
 315             ExprKind::Loop(block, label, _, _) | ExprKind::Block(block, label) => {
 316                 self.visit_block_label(block, label);
 317             },
 318             ExprKind::Assign(bind, expr, _) => {
 319                 self.visit_assign(bind, expr);
 320             },
 321             ExprKind::AssignOp(_, bind, expr) => {
 322                 self.visit_assign(bind, expr);
 323                 self.visit_bin_op(bind, expr);
 324             },
 325             ExprKind::Field(expr, _) => {
 326                 self.visit_expr(expr);
 327                 if matches!(self.ty_res.expr_ty(expr).kind(), ty::Ref(_, _, Mutability::Mut)) {
 328                     self.ret_vars.iter_mut().for_each(|(_, b)| *b = true);
 329                 }
 330             },
 331             ExprKind::Index(expr, index) => {
 332                 self.visit_expr(expr);
 333                 let mut vars = std::mem::take(&mut self.ret_vars);
 334                 self.visit_expr(index);
 335                 self.ret_vars.append(&mut vars);
 336
 337                 if !is_array(self.ty_res.expr_ty(expr)) {
 338                     self.add_side_effect(self.ret_vars.clone());
 339                 } else if matches!(self.ty_res.expr_ty(expr).kind(), ty::Ref(_, _, Mutability::Mut)) {
 340                     self.ret_vars.iter_mut().for_each(|(_, b)| *b = true);
 341                 }
 342             },
 343             ExprKind::Break(dest, Some(expr)) => {
 344                 self.visit_expr(expr);
 345                 if let Some(label) = dest.label {
 346                     self.break_vars
 347                         .entry(label.ident)
 348                         .or_insert(Vec::new())
 349                         .append(&mut self.ret_vars);
 350                 }
 351                 self.contains_side_effect = true;
 352             },
 353             ExprKind::Ret(Some(expr)) => {
 354                 self.visit_expr(expr);
 355                 let vars = std::mem::take(&mut self.ret_vars);
 356                 self.add_side_effect(vars);
 357                 self.contains_side_effect = true;
 358             },
 359             ExprKind::Break(_, None) | ExprKind::Continue(_) | ExprKind::Ret(None) => {
 360                 self.contains_side_effect = true;
 361             },
 362             ExprKind::Struct(_, exprs, expr) => {
 363                 let mut ret_vars = exprs
 364                     .iter()
 365                     .flat_map(|field| {
 366                         self.visit_expr(field.expr);
 367                         std::mem::take(&mut self.ret_vars)
 368                     })
 369                     .collect();
 370
 371                 walk_list!(self, visit_expr, expr);
 372                 self.ret_vars.append(&mut ret_vars);
 373             },
 374             _ => walk_expr(self, ex),
 375         }
 376     }
 377
 378     fn visit_path(&mut self, path: &'tcx Path<'tcx>, _id: HirId) {
 379         if let Res::Local(id) = path.res {
 380             self.ret_vars.push((id, false));
 381         }
 382     }
 383 }
 384
 385 impl<'tcx> SideEffectVisit<'tcx> {
 386     fn visit_assign(&mut self, lhs: &'tcx Expr<'tcx>, rhs: &'tcx Expr<'tcx>) {
 387         // Just support array and tuple unwrapping for now.
 388         //
 389         // ex) `(a, b) = (c, d);`
 390         // The graph would look like this:
 391         //   a -> c
 392         //   b -> d
 393         //
 394         // This would minimize the connection of the side-effect graph.
 395         match (&lhs.kind, &rhs.kind) {
 396             (ExprKind::Array(lhs), ExprKind::Array(rhs)) | (ExprKind::Tup(lhs), ExprKind::Tup(rhs)) => {
 397                 // if not, it is a compile error
 398                 debug_assert!(lhs.len() == rhs.len());
 399                 izip!(*lhs, *rhs).for_each(|(lhs, rhs)| self.visit_assign(lhs, rhs));
 400             },
 401             // in other assigns, we have to connect all each other
 402             // because they can be connected somehow
 403             _ => {
 404                 self.visit_expr(lhs);
 405                 let lhs_vars = std::mem::take(&mut self.ret_vars);
 406                 self.visit_expr(rhs);
 407                 let rhs_vars = std::mem::take(&mut self.ret_vars);
 408                 self.connect_assign(&lhs_vars, &rhs_vars, false);
 409             },
 410         }
 411     }
 412
 413     fn visit_block_label(&mut self, block: &'tcx Block<'tcx>, label: Option<Label>) {
 414         self.visit_block(block);
 415         let _ = label.and_then(|label| {
 416             self.break_vars
 417                 .remove(&label.ident)
 418                 .map(|mut break_vars| self.ret_vars.append(&mut break_vars))
 419         });
 420     }
 421
 422     fn visit_bin_op(&mut self, lhs: &'tcx Expr<'tcx>, rhs: &'tcx Expr<'tcx>) {
 423         self.visit_expr(lhs);
 424         let mut ret_vars = std::mem::take(&mut self.ret_vars);
 425         self.visit_expr(rhs);
 426         self.ret_vars.append(&mut ret_vars);
 427
 428         // the binary operation between non primitive values are overloaded operators
 429         // so they can have side-effects
 430         if !is_primitive(self.ty_res.expr_ty(lhs)) || !is_primitive(self.ty_res.expr_ty(rhs)) {
 431             self.ret_vars.iter().for_each(|id| {
 432                 self.has_side_effect.insert(id.0);
 433             });
 434             self.contains_side_effect = true;
 435         }
 436     }
 437
 438     fn visit_un_op(&mut self, op: UnOp, expr: &'tcx Expr<'tcx>) {
 439         self.visit_expr(expr);
 440         let ty = self.ty_res.expr_ty(expr);
 441         // dereferencing a reference has no side-effect
 442         if !is_primitive(ty) && !matches!((op, ty.kind()), (UnOp::Deref, ty::Ref(..))) {
 443             self.add_side_effect(self.ret_vars.clone());
 444         }
 445
 446         if matches!((op, ty.kind()), (UnOp::Deref, ty::Ref(_, _, Mutability::Mut))) {
 447             self.ret_vars.iter_mut().for_each(|(_, b)| *b = true);
 448         }
 449     }
 450
 451     fn visit_pat_expr(&mut self, pat: &'tcx Pat<'tcx>, expr: &'tcx Expr<'tcx>, connect_self: bool) {
 452         match (&pat.kind, &expr.kind) {
 453             (PatKind::Tuple(pats, _), ExprKind::Tup(exprs)) => {
 454                 self.ret_vars = izip!(*pats, *exprs)
 455                     .flat_map(|(pat, expr)| {
 456                         self.visit_pat_expr(pat, expr, connect_self);
 457                         std::mem::take(&mut self.ret_vars)
 458                     })
 459                     .collect();
 460             },
 461             (PatKind::Slice(front_exprs, _, back_exprs), ExprKind::Array(exprs)) => {
 462                 let mut vars = izip!(*front_exprs, *exprs)
 463                     .flat_map(|(pat, expr)| {
 464                         self.visit_pat_expr(pat, expr, connect_self);
 465                         std::mem::take(&mut self.ret_vars)
 466                     })
 467                     .collect();
 468                 self.ret_vars = izip!(back_exprs.iter().rev(), exprs.iter().rev())
 469                     .flat_map(|(pat, expr)| {
 470                         self.visit_pat_expr(pat, expr, connect_self);
 471                         std::mem::take(&mut self.ret_vars)
 472                     })
 473                     .collect();
 474                 self.ret_vars.append(&mut vars);
 475             },
 476             _ => {
 477                 let mut lhs_vars = Vec::new();
 478                 pat.each_binding(|_, id, _, _| lhs_vars.push((id, false)));
 479                 self.visit_expr(expr);
 480                 let rhs_vars = std::mem::take(&mut self.ret_vars);
 481                 self.connect_assign(&lhs_vars, &rhs_vars, connect_self);
 482                 self.ret_vars = rhs_vars;
 483             },
 484         }
 485     }
 486
 487     fn visit_fn(&mut self, callee: &'tcx Expr<'tcx>, args: &'tcx [Expr<'tcx>]) {
 488         self.visit_expr(callee);
 489         let mut ret_vars = std::mem::take(&mut self.ret_vars);
 490         self.add_side_effect(ret_vars.clone());
 491
 492         let mut is_recursive = false;
 493
 494         if_chain! {
 495             if !self.has_self;
 496             if let ExprKind::Path(QPath::Resolved(_, path)) = callee.kind;
 497             if let Res::Def(DefKind::Fn, def_id) = path.res;
 498             if self.fn_def_id == def_id;
 499             then {
 500                 is_recursive = true;
 501             }
 502         }
 503
 504         if_chain! {
 505             if !self.has_self && self.is_method;
 506             if let ExprKind::Path(QPath::TypeRelative(ty, segment)) = callee.kind;
 507             if segment.ident == self.fn_ident;
 508             if let TyKind::Path(QPath::Resolved(_, path)) = ty.kind;
 509             if let Res::SelfTy{ .. } = path.res;
 510             then {
 511                 is_recursive = true;
 512             }
 513         }
 514
 515         if is_recursive {
 516             izip!(self.params.clone(), args).for_each(|(pat, expr)| {
 517                 self.visit_pat_expr(pat, expr, true);
 518                 self.ret_vars.clear();
 519             });
 520         } else {
 521             // This would set arguments used in closure that does not have side-effect.
 522             // Closure itself can be detected whether there is a side-effect, but the
 523             // value of variable that is holding closure can change.
 524             // So, we just check the variables.
 525             self.ret_vars = args
 526                 .iter()
 527                 .flat_map(|expr| {
 528                     self.visit_expr(expr);
 529                     std::mem::take(&mut self.ret_vars)
 530                 })
 531                 .collect_vec()
 532                 .into_iter()
 533                 .map(|id| {
 534                     self.has_side_effect.insert(id.0);
 535                     id
 536                 })
 537                 .collect();
 538             self.contains_side_effect = true;
 539         }
 540
 541         self.ret_vars.append(&mut ret_vars);
 542     }
 543
 544     fn visit_method_call(&mut self, path: &'tcx PathSegment<'tcx>, args: &'tcx [Expr<'tcx>]) {
 545         if_chain! {
 546             if self.is_method;
 547             if path.ident == self.fn_ident;
 548             if let ExprKind::Path(QPath::Resolved(_, path)) = args.first().unwrap().kind;
 549             if let Res::Local(..) = path.res;
 550             let ident = path.segments.last().unwrap().ident;
 551             if ident.name == kw::SelfLower;
 552             then {
 553                 izip!(self.params.clone(), args.iter())
 554                     .for_each(|(pat, expr)| {
 555                         self.visit_pat_expr(pat, expr, true);
 556                         self.ret_vars.clear();
 557                     });
 558             } else {
 559                 self.ret_vars = args
 560                     .iter()
 561                     .flat_map(|expr| {
 562                         self.visit_expr(expr);
 563                         std::mem::take(&mut self.ret_vars)
 564                     })
 565                     .collect_vec()
 566                     .into_iter()
 567                     .map(|a| {
 568                         self.has_side_effect.insert(a.0);
 569                         a
 570                     })
 571                     .collect();
 572                 self.contains_side_effect = true;
 573             }
 574         }
 575     }
 576
 577     fn visit_if(&mut self, bind: &'tcx Expr<'tcx>, then_expr: &'tcx Expr<'tcx>, else_expr: Option<&'tcx Expr<'tcx>>) {
 578         let contains_side_effect = self.contains_side_effect;
 579         self.contains_side_effect = false;
 580         self.visit_expr(bind);
 581         let mut vars = std::mem::take(&mut self.ret_vars);
 582         self.visit_expr(then_expr);
 583         let mut then_vars = std::mem::take(&mut self.ret_vars);
 584         walk_list!(self, visit_expr, else_expr);
 585         if self.contains_side_effect {
 586             self.add_side_effect(vars.clone());
 587         }
 588         self.contains_side_effect |= contains_side_effect;
 589         self.ret_vars.append(&mut vars);
 590         self.ret_vars.append(&mut then_vars);
 591     }
 592
 593     fn visit_match(&mut self, expr: &'tcx Expr<'tcx>, arms: &'tcx [Arm<'tcx>]) {
 594         self.visit_expr(expr);
 595         let mut expr_vars = std::mem::take(&mut self.ret_vars);
 596         self.ret_vars = arms
 597             .iter()
 598             .flat_map(|arm| {
 599                 let contains_side_effect = self.contains_side_effect;
 600                 self.contains_side_effect = false;
 601                 // this would visit `expr` multiple times
 602                 // but couldn't think of a better way
 603                 self.visit_pat_expr(arm.pat, expr, false);
 604                 let mut vars = std::mem::take(&mut self.ret_vars);
 605                 let _ = arm.guard.as_ref().map(|guard| {
 606                     self.visit_expr(match guard {
 607                         Guard::If(expr) | Guard::IfLet(_, expr) => expr,
 608                     });
 609                     vars.append(&mut self.ret_vars);
 610                 });
 611                 self.visit_expr(arm.body);
 612                 if self.contains_side_effect {
 613                     self.add_side_effect(vars.clone());
 614                     self.add_side_effect(expr_vars.clone());
 615                 }
 616                 self.contains_side_effect |= contains_side_effect;
 617                 vars.append(&mut self.ret_vars);
 618                 vars
 619             })
 620             .collect();
 621         self.ret_vars.append(&mut expr_vars);
 622     }
 623
 624     fn connect_assign(&mut self, lhs: &[(HirId, bool)], rhs: &[(HirId, bool)], connect_self: bool) {
 625         // if mutable dereference is on assignment it can have side-effect
 626         // (this can lead to parameter mutable dereference and change the original value)
 627         // too hard to detect whether this value is from parameter, so this would all
 628         // check mutable dereference assignment to side effect
 629         lhs.iter().filter(|(_, b)| *b).for_each(|(id, _)| {
 630             self.has_side_effect.insert(*id);
 631             self.contains_side_effect = true;
 632         });
 633
 634         // there is no connection
 635         if lhs.is_empty() || rhs.is_empty() {
 636             return;
 637         }
 638
 639         // by connected rhs in cycle, the connections would decrease
 640         // from `n * m` to `n + m`
 641         // where `n` and `m` are length of `lhs` and `rhs`.
 642
 643         // unwrap is possible since rhs is not empty
 644         let rhs_first = rhs.first().unwrap();
 645         for (id, _) in lhs.iter() {
 646             if connect_self || *id != rhs_first.0 {
 647                 self.graph
 648                     .entry(*id)
 649                     .or_insert_with(FxHashSet::default)
 650                     .insert(rhs_first.0);
 651             }
 652         }
 653
 654         let rhs = rhs.iter();
 655         izip!(rhs.clone().cycle().skip(1), rhs).for_each(|(from, to)| {
 656             if connect_self || from.0 != to.0 {
 657                 self.graph.entry(from.0).or_insert_with(FxHashSet::default).insert(to.0);
 658             }
 659         });
 660     }
 661
 662     fn add_side_effect(&mut self, v: Vec<(HirId, bool)>) {
 663         for (id, _) in v {
 664             self.has_side_effect.insert(id);
 665             self.contains_side_effect = true;
 666         }
 667     }
 668 }