Auto merge of rust-lang#97014 - Mark-Simulacrum:beta-next, r=Mark-Simulacrum

[beta] backports

This backports/rolls up:

*  Quick fix for rust-lang#96223. rust-lang#96679
*  [beta] Revert rust-lang#92519 on beta rust-lang#96556
*  [beta] Clippy backport ICE/infinite loop fix rust-lang#96740
*  Revert "Prefer projection candidates instead of param_env candidates for Sized predicates" rust-lang#96593

Author: bors

compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs

@@ -713,7 +713,13 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
                 return false;
             }
 
-            let orig_ty = old_pred.self_ty().skip_binder();
+            // This is a quick fix to resolve an ICE (#96223).
+            // This change should probably be deeper.
+            // As suggested by @jackh726, `mk_trait_obligation_with_new_self_ty` could take a `Binder<(TraitRef, Ty)>
+            // instead of `Binder<Ty>` leading to some changes to its call places.
+            let Some(orig_ty) = old_pred.self_ty().no_bound_vars() else {
+                return false;
+            };
             let mk_result = |new_ty| {
                 let obligation =
                     self.mk_trait_obligation_with_new_self_ty(param_env, old_pred, new_ty);

compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs

@@ -175,9 +175,6 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
 
         let needs_infer = stack.obligation.predicate.has_infer_types_or_consts();
 
-        let sized_predicate = self.tcx().lang_items().sized_trait()
-            == Some(stack.obligation.predicate.skip_binder().def_id());
-
         // If there are STILL multiple candidates, we can further
         // reduce the list by dropping duplicates -- including
         // resolving specializations.
@@ -186,7 +183,6 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
             while i < candidates.len() {
                 let is_dup = (0..candidates.len()).filter(|&j| i != j).any(|j| {
                     self.candidate_should_be_dropped_in_favor_of(
-                        sized_predicate,
                         &candidates[i],
                         &candidates[j],
                         needs_infer,

compiler/rustc_trait_selection/src/traits/select/mod.rs

@@ -1553,7 +1553,6 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
     /// See the comment for "SelectionCandidate" for more details.
     fn candidate_should_be_dropped_in_favor_of(
         &mut self,
-        sized_predicate: bool,
         victim: &EvaluatedCandidate<'tcx>,
         other: &EvaluatedCandidate<'tcx>,
         needs_infer: bool,
@@ -1625,16 +1624,6 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
             // Drop otherwise equivalent non-const fn pointer candidates
             (FnPointerCandidate { .. }, FnPointerCandidate { is_const: false }) => true,
 
-            // If obligation is a sized predicate or the where-clause bound is
-            // global, prefer the projection or object candidate. See issue
-            // #50825 and #89352.
-            (ObjectCandidate(_) | ProjectionCandidate(_), ParamCandidate(ref cand)) => {
-                sized_predicate || is_global(cand)
-            }
-            (ParamCandidate(ref cand), ObjectCandidate(_) | ProjectionCandidate(_)) => {
-                !(sized_predicate || is_global(cand))
-            }
-
             // Global bounds from the where clause should be ignored
             // here (see issue #50825). Otherwise, we have a where
             // clause so don't go around looking for impls.
@@ -1650,8 +1639,15 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
                 | BuiltinUnsizeCandidate
                 | TraitUpcastingUnsizeCandidate(_)
                 | BuiltinCandidate { .. }
-                | TraitAliasCandidate(..),
+                | TraitAliasCandidate(..)
+                | ObjectCandidate(_)
+                | ProjectionCandidate(_),
             ) => !is_global(cand),
+            (ObjectCandidate(_) | ProjectionCandidate(_), ParamCandidate(ref cand)) => {
+                // Prefer these to a global where-clause bound
+                // (see issue #50825).
+                is_global(cand)
+            }
             (
                 ImplCandidate(_)
                 | ClosureCandidate

library/std/src/sys/windows/process.rs

@@ -19,12 +19,12 @@ use crate::path::{Path, PathBuf};
 use crate::ptr;
 use crate::sys::c;
 use crate::sys::c::NonZeroDWORD;
-use crate::sys::cvt;
 use crate::sys::fs::{File, OpenOptions};
 use crate::sys::handle::Handle;
 use crate::sys::path;
 use crate::sys::pipe::{self, AnonPipe};
 use crate::sys::stdio;
+use crate::sys::{cvt, to_u16s};
 use crate::sys_common::mutex::StaticMutex;
 use crate::sys_common::process::{CommandEnv, CommandEnvs};
 use crate::sys_common::{AsInner, IntoInner};
@@ -269,13 +269,8 @@ impl Command {
             None
         };
         let program = resolve_exe(&self.program, || env::var_os("PATH"), child_paths)?;
-        // Case insensitive "ends_with" of UTF-16 encoded ".bat" or ".cmd"
-        let is_batch_file = matches!(
-            program.len().checked_sub(5).and_then(|i| program.get(i..)),
-            Some([46, 98 | 66, 97 | 65, 116 | 84, 0] | [46, 99 | 67, 109 | 77, 100 | 68, 0])
-        );
         let mut cmd_str =
-            make_command_line(&program, &self.args, self.force_quotes_enabled, is_batch_file)?;
+            make_command_line(program.as_os_str(), &self.args, self.force_quotes_enabled)?;
         cmd_str.push(0); // add null terminator
 
         // stolen from the libuv code.
@@ -314,6 +309,7 @@ impl Command {
         si.hStdOutput = stdout.as_raw_handle();
         si.hStdError = stderr.as_raw_handle();
 
+        let program = to_u16s(&program)?;
         unsafe {
             cvt(c::CreateProcessW(
                 program.as_ptr(),
@@ -370,7 +366,7 @@ fn resolve_exe<'a>(
     exe_path: &'a OsStr,
     parent_paths: impl FnOnce() -> Option<OsString>,
     child_paths: Option<&OsStr>,
-) -> io::Result<Vec<u16>> {
+) -> io::Result<PathBuf> {
     // Early return if there is no filename.
     if exe_path.is_empty() || path::has_trailing_slash(exe_path) {
         return Err(io::const_io_error!(
@@ -392,19 +388,19 @@ fn resolve_exe<'a>(
         if has_exe_suffix {
             // The application name is a path to a `.exe` file.
             // Let `CreateProcessW` figure out if it exists or not.
-            return path::maybe_verbatim(Path::new(exe_path));
+            return Ok(exe_path.into());
         }
         let mut path = PathBuf::from(exe_path);
 
         // Append `.exe` if not already there.
         path = path::append_suffix(path, EXE_SUFFIX.as_ref());
-        if let Some(path) = program_exists(&path) {
+        if program_exists(&path) {
             return Ok(path);
         } else {
             // It's ok to use `set_extension` here because the intent is to
             // remove the extension that was just added.
             path.set_extension("");
-            return path::maybe_verbatim(&path);
+            return Ok(path);
         }
     } else {
         ensure_no_nuls(exe_path)?;
@@ -419,7 +415,7 @@ fn resolve_exe<'a>(
             if !has_extension {
                 path.set_extension(EXE_EXTENSION);
             }
-            program_exists(&path)
+            if program_exists(&path) { Some(path) } else { None }
         });
         if let Some(path) = result {
             return Ok(path);
@@ -435,10 +431,10 @@ fn search_paths<Paths, Exists>(
     parent_paths: Paths,
     child_paths: Option<&OsStr>,
     mut exists: Exists,
-) -> Option<Vec<u16>>
+) -> Option<PathBuf>
 where
     Paths: FnOnce() -> Option<OsString>,
-    Exists: FnMut(PathBuf) -> Option<Vec<u16>>,
+    Exists: FnMut(PathBuf) -> Option<PathBuf>,
 {
     // 1. Child paths
     // This is for consistency with Rust's historic behaviour.
@@ -490,18 +486,17 @@ where
 }
 
 /// Check if a file exists without following symlinks.
-fn program_exists(path: &Path) -> Option<Vec<u16>> {
+fn program_exists(path: &Path) -> bool {
     unsafe {
-        let path = path::maybe_verbatim(path).ok()?;
-        // Getting attributes using `GetFileAttributesW` does not follow symlinks
-        // and it will almost always be successful if the link exists.
-        // There are some exceptions for special system files (e.g. the pagefile)
-        // but these are not executable.
-        if c::GetFileAttributesW(path.as_ptr()) == c::INVALID_FILE_ATTRIBUTES {
-            None
-        } else {
-            Some(path)
-        }
+        to_u16s(path)
+            .map(|path| {
+                // Getting attributes using `GetFileAttributesW` does not follow symlinks
+                // and it will almost always be successful if the link exists.
+                // There are some exceptions for special system files (e.g. the pagefile)
+                // but these are not executable.
+                c::GetFileAttributesW(path.as_ptr()) != c::INVALID_FILE_ATTRIBUTES
+            })
+            .unwrap_or(false)
     }
 }
 
@@ -735,12 +730,7 @@ enum Quote {
 
 // Produces a wide string *without terminating null*; returns an error if
 // `prog` or any of the `args` contain a nul.
-fn make_command_line(
-    prog: &[u16],
-    args: &[Arg],
-    force_quotes: bool,
-    is_batch_file: bool,
-) -> io::Result<Vec<u16>> {
+fn make_command_line(prog: &OsStr, args: &[Arg], force_quotes: bool) -> io::Result<Vec<u16>> {
     // Encode the command and arguments in a command line string such
     // that the spawned process may recover them using CommandLineToArgvW.
     let mut cmd: Vec<u16> = Vec::new();
@@ -749,18 +739,17 @@ fn make_command_line(
     // need to add an extra pair of quotes surrounding the whole command line
     // so they are properly passed on to the script.
     // See issue #91991.
+    let is_batch_file = Path::new(prog)
+        .extension()
+        .map(|ext| ext.eq_ignore_ascii_case("cmd") || ext.eq_ignore_ascii_case("bat"))
+        .unwrap_or(false);
     if is_batch_file {
         cmd.push(b'"' as u16);
     }
 
-    // Always quote the program name so CreateProcess to avoid ambiguity when
-    // the child process parses its arguments.
-    // Note that quotes aren't escaped here because they can't be used in arg0.
-    // But that's ok because file paths can't contain quotes.
-    cmd.push(b'"' as u16);
-    cmd.extend_from_slice(prog.strip_suffix(&[0]).unwrap_or(prog));
-    cmd.push(b'"' as u16);
-
+    // Always quote the program name so CreateProcess doesn't interpret args as
+    // part of the name if the binary wasn't found first time.
+    append_arg(&mut cmd, prog, Quote::Always)?;
     for arg in args {
         cmd.push(' ' as u16);
         let (arg, quote) = match arg {

library/std/src/sys/windows/process/tests.rs

@@ -3,12 +3,11 @@ use super::Arg;
 use crate::env;
 use crate::ffi::{OsStr, OsString};
 use crate::process::Command;
-use crate::sys::to_u16s;
 
 #[test]
 fn test_raw_args() {
     let command_line = &make_command_line(
-        &to_u16s("quoted exe").unwrap(),
+        OsStr::new("quoted exe"),
         &[
             Arg::Regular(OsString::from("quote me")),
             Arg::Raw(OsString::from("quote me *not*")),
@@ -17,7 +16,6 @@ fn test_raw_args() {
             Arg::Regular(OsString::from("optional-quotes")),
         ],
         false,
-        false,
     )
     .unwrap();
     assert_eq!(
@@ -30,10 +28,9 @@ fn test_raw_args() {
 fn test_make_command_line() {
     fn test_wrapper(prog: &str, args: &[&str], force_quotes: bool) -> String {
         let command_line = &make_command_line(
-            &to_u16s(prog).unwrap(),
+            OsStr::new(prog),
             &args.iter().map(|a| Arg::Regular(OsString::from(a))).collect::<Vec<_>>(),
             force_quotes,
-            false,
         )
         .unwrap();
         String::from_utf16(command_line).unwrap()

src/test/ui/generic-associated-types/bugs/issue-89352.base.stderr

@@ -0,0 +1,22 @@
+error[E0308]: mismatched types
+  --> $DIR/issue-89352.rs:36:13
+   |
+LL |         let a = A::reborrow::<'ai, 's>(self.a.clone());
+   |             ^ lifetime mismatch
+   |
+   = note: expected type `<<A as GenAssoc<T>>::Iter<'s> as Sized>`
+              found type `<<A as GenAssoc<T>>::Iter<'ai> as Sized>`
+note: the lifetime `'s` as defined here...
+  --> $DIR/issue-89352.rs:35:13
+   |
+LL |     fn iter<'s>(&'s self) -> Self::Iter<'s> {
+   |             ^^
+note: ...does not necessarily outlive the lifetime `'ai` as defined here
+  --> $DIR/issue-89352.rs:30:6
+   |
+LL | impl<'ai, T: 'ai, A: GenAssoc<T>> GenAssoc<T> for Wrapper<'ai, T, A>
+   |      ^^^
+
+error: aborting due to previous error
+
+For more information about this error, try `rustc --explain E0308`.

src/test/ui/generic-associated-types/issue-89352.rs renamed to src/test/ui/generic-associated-types/bugs/issue-89352.rs

@@ -1,4 +1,16 @@
-// check-pass
+// revisions: base nll
+// ignore-compare-mode-nll
+//[nll] compile-flags: -Z borrowck=mir
+
+//[base] check-fail
+//[nll] check-pass
+// known-bug
+
+// This should pass, but we end up with `A::Iter<'ai>: Sized` for some specific
+// `'ai`. We also know that `for<'at> A::Iter<'at>: Sized` from the definition,
+// but we prefer param env candidates. We changed this to preference in #92191,
+// but this led to unintended consequences (#93262). Suprisingly, this passes
+// under NLL. So only a bug in migrate mode.
 
 #![feature(generic_associated_types)]
 

src/test/ui/generic-associated-types/issue-93262.rs

@@ -0,0 +1,21 @@
+// check-pass
+
+#![feature(generic_associated_types)]
+
+pub trait Trait {
+    type Assoc<'a> where Self: 'a;
+}
+
+pub trait Foo<T: Trait>
+where
+    for<'a> T::Assoc<'a>: Clone
+{}
+
+pub struct Type;
+
+impl<T: Trait> Foo<T> for Type
+where
+    for<'a> T::Assoc<'a>: Clone
+{}
+
+fn main() {}

src/test/ui/suggestions/issue-96223.rs

@@ -0,0 +1,52 @@
+// Previously ICEd because we didn't properly track binders in suggestions
+// check-fail
+
+pub trait Foo<'de>: Sized {}
+
+pub trait Bar<'a>: 'static {
+    type Inner: 'a;
+}
+
+pub trait Fubar {
+    type Bar: for<'a> Bar<'a>;
+}
+
+pub struct Baz<T>(pub T);
+
+impl<'de, T> Foo<'de> for Baz<T> where T: Foo<'de> {}
+
+struct Empty;
+
+impl<M> Dummy<M> for Empty
+where
+    M: Fubar,
+    for<'de> Baz<<M::Bar as Bar<'de>>::Inner>: Foo<'de>,
+{
+}
+
+pub trait Dummy<M>
+where
+    M: Fubar,
+{
+}
+
+pub struct EmptyBis<'a>(&'a [u8]);
+
+impl<'a> Bar<'a> for EmptyBis<'static> {
+    type Inner = EmptyBis<'a>;
+}
+
+pub struct EmptyMarker;
+
+impl Fubar for EmptyMarker {
+    type Bar = EmptyBis<'static>;
+}
+
+fn icey_bounds<D: Dummy<EmptyMarker>>(p: &D) {}
+
+fn trigger_ice() {
+    let p = Empty;
+    icey_bounds(&p); //~ERROR the trait bound
+}
+
+fn main() {}

src/test/ui/suggestions/issue-96223.stderr

@@ -0,0 +1,27 @@
+error[E0277]: the trait bound `for<'de> EmptyBis<'de>: Foo<'_>` is not satisfied
+  --> $DIR/issue-96223.rs:49:17
+   |
+LL |     icey_bounds(&p);
+   |     ----------- ^^ the trait `for<'de> Foo<'_>` is not implemented for `EmptyBis<'de>`
+   |     |
+   |     required by a bound introduced by this call
+   |
+note: required because of the requirements on the impl of `for<'de> Foo<'de>` for `Baz<EmptyBis<'de>>`
+  --> $DIR/issue-96223.rs:16:14
+   |
+LL | impl<'de, T> Foo<'de> for Baz<T> where T: Foo<'de> {}
+   |              ^^^^^^^^     ^^^^^^
+note: required because of the requirements on the impl of `Dummy<EmptyMarker>` for `Empty`
+  --> $DIR/issue-96223.rs:20:9
+   |
+LL | impl<M> Dummy<M> for Empty
+   |         ^^^^^^^^     ^^^^^
+note: required by a bound in `icey_bounds`
+  --> $DIR/issue-96223.rs:45:19
+   |
+LL | fn icey_bounds<D: Dummy<EmptyMarker>>(p: &D) {}
+   |                   ^^^^^^^^^^^^^^^^^^ required by this bound in `icey_bounds`
+
+error: aborting due to previous error
+
+For more information about this error, try `rustc --explain E0277`.

src/tools/clippy/clippy_lints/src/lib.register_all.rs

@@ -237,7 +237,6 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
     LintId::of(non_expressive_names::JUST_UNDERSCORES_AND_DIGITS),
     LintId::of(non_octal_unix_permissions::NON_OCTAL_UNIX_PERMISSIONS),
     LintId::of(octal_escapes::OCTAL_ESCAPES),
-    LintId::of(only_used_in_recursion::ONLY_USED_IN_RECURSION),
     LintId::of(open_options::NONSENSICAL_OPEN_OPTIONS),
     LintId::of(option_env_unwrap::OPTION_ENV_UNWRAP),
     LintId::of(overflow_check_conditional::OVERFLOW_CHECK_CONDITIONAL),

src/tools/clippy/clippy_lints/src/lib.register_complexity.rs

@@ -66,7 +66,6 @@ store.register_group(true, "clippy::complexity", Some("clippy_complexity"), vec!
     LintId::of(neg_cmp_op_on_partial_ord::NEG_CMP_OP_ON_PARTIAL_ORD),
     LintId::of(no_effect::NO_EFFECT),
     LintId::of(no_effect::UNNECESSARY_OPERATION),
-    LintId::of(only_used_in_recursion::ONLY_USED_IN_RECURSION),
     LintId::of(overflow_check_conditional::OVERFLOW_CHECK_CONDITIONAL),
     LintId::of(partialeq_ne_impl::PARTIALEQ_NE_IMPL),
     LintId::of(precedence::PRECEDENCE),

src/tools/clippy/clippy_lints/src/lib.register_nursery.rs

@@ -20,6 +20,7 @@ store.register_group(true, "clippy::nursery", Some("clippy_nursery"), vec![
     LintId::of(mutex_atomic::MUTEX_INTEGER),
     LintId::of(non_send_fields_in_send_ty::NON_SEND_FIELDS_IN_SEND_TY),
     LintId::of(nonstandard_macro_braces::NONSTANDARD_MACRO_BRACES),
+    LintId::of(only_used_in_recursion::ONLY_USED_IN_RECURSION),
     LintId::of(option_if_let_else::OPTION_IF_LET_ELSE),
     LintId::of(path_buf_push_overwrite::PATH_BUF_PUSH_OVERWRITE),
     LintId::of(redundant_pub_crate::REDUNDANT_PUB_CRATE),

src/tools/clippy/clippy_lints/src/only_used_in_recursion.rs

@@ -1,14 +1,15 @@
 use std::collections::VecDeque;
 
 use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::is_lint_allowed;
 use itertools::{izip, Itertools};
 use rustc_ast::{walk_list, Label, Mutability};
 use rustc_data_structures::fx::{FxHashMap, FxHashSet};
 use rustc_errors::Applicability;
 use rustc_hir::def::{DefKind, Res};
 use rustc_hir::def_id::DefId;
 use rustc_hir::definitions::{DefPathData, DisambiguatedDefPathData};
-use rustc_hir::intravisit::{walk_expr, FnKind, Visitor};
+use rustc_hir::intravisit::{walk_expr, walk_stmt, FnKind, Visitor};
 use rustc_hir::{
     Arm, Block, Body, Expr, ExprKind, Guard, HirId, ImplicitSelfKind, Let, Local, Pat, PatKind, Path, PathSegment,
     QPath, Stmt, StmtKind, TyKind, UnOp,
@@ -33,6 +34,9 @@ declare_clippy_lint! {
     /// and the assigned variables are also only in recursion, it is useless.
     ///
     /// ### Known problems
+    /// Too many code paths in the linting code are currently untested and prone to produce false
+    /// positives or are prone to have performance implications.
+    ///
     /// In some cases, this would not catch all useless arguments.
     ///
     /// ```rust
@@ -85,7 +89,7 @@ declare_clippy_lint! {
     /// ```
     #[clippy::version = "1.60.0"]
     pub ONLY_USED_IN_RECURSION,
-    complexity,
+    nursery,
     "arguments that is only used in recursion can be removed"
 }
 declare_lint_pass!(OnlyUsedInRecursion => [ONLY_USED_IN_RECURSION]);
@@ -100,6 +104,9 @@ impl<'tcx> LateLintPass<'tcx> for OnlyUsedInRecursion {
         _: Span,
         id: HirId,
     ) {
+        if is_lint_allowed(cx, ONLY_USED_IN_RECURSION, id) {
+            return;
+        }
         if let FnKind::ItemFn(ident, ..) | FnKind::Method(ident, ..) = kind {
             let def_id = id.owner.to_def_id();
             let data = cx.tcx.def_path(def_id).data;
@@ -145,7 +152,8 @@ impl<'tcx> LateLintPass<'tcx> for OnlyUsedInRecursion {
                 is_method: matches!(kind, FnKind::Method(..)),
                 has_self,
                 ty_res,
-                ty_ctx: cx.tcx,
+                tcx: cx.tcx,
+                visited_exprs: FxHashSet::default(),
             };
 
             visitor.visit_expr(&body.value);
@@ -206,19 +214,13 @@ impl<'tcx> LateLintPass<'tcx> for OnlyUsedInRecursion {
 }
 
 pub fn is_primitive(ty: Ty<'_>) -> bool {
-    match ty.kind() {
-        ty::Bool | ty::Char | ty::Int(_) | ty::Uint(_) | ty::Float(_) | ty::Str => true,
-        ty::Ref(_, t, _) => is_primitive(*t),
-        _ => false,
-    }
+    let ty = ty.peel_refs();
+    ty.is_primitive() || ty.is_str()
 }
 
 pub fn is_array(ty: Ty<'_>) -> bool {
-    match ty.kind() {
-        ty::Array(..) | ty::Slice(..) => true,
-        ty::Ref(_, t, _) => is_array(*t),
-        _ => false,
-    }
+    let ty = ty.peel_refs();
+    ty.is_array() || ty.is_array_slice()
 }
 
 /// This builds the graph of side effect.
@@ -250,40 +252,30 @@ pub struct SideEffectVisit<'tcx> {
     is_method: bool,
     has_self: bool,
     ty_res: &'tcx TypeckResults<'tcx>,
-    ty_ctx: TyCtxt<'tcx>,
+    tcx: TyCtxt<'tcx>,
+    visited_exprs: FxHashSet<HirId>,
 }
 
 impl<'tcx> Visitor<'tcx> for SideEffectVisit<'tcx> {
-    fn visit_block(&mut self, b: &'tcx Block<'tcx>) {
-        b.stmts.iter().for_each(|stmt| {
-            self.visit_stmt(stmt);
-            self.ret_vars.clear();
-        });
-        walk_list!(self, visit_expr, b.expr);
-    }
-
     fn visit_stmt(&mut self, s: &'tcx Stmt<'tcx>) {
         match s.kind {
             StmtKind::Local(Local {
                 pat, init: Some(init), ..
             }) => {
                 self.visit_pat_expr(pat, init, false);
-                self.ret_vars.clear();
             },
-            StmtKind::Item(i) => {
-                let item = self.ty_ctx.hir().item(i);
-                self.visit_item(item);
-                self.ret_vars.clear();
-            },
-            StmtKind::Expr(e) | StmtKind::Semi(e) => {
-                self.visit_expr(e);
-                self.ret_vars.clear();
+            StmtKind::Item(_) | StmtKind::Expr(_) | StmtKind::Semi(_) => {
+                walk_stmt(self, s);
             },
             StmtKind::Local(_) => {},
         }
+        self.ret_vars.clear();
     }
 
     fn visit_expr(&mut self, ex: &'tcx Expr<'tcx>) {
+        if !self.visited_exprs.insert(ex.hir_id) {
+            return;
+        }
         match ex.kind {
             ExprKind::Array(exprs) | ExprKind::Tup(exprs) => {
                 self.ret_vars = exprs
@@ -307,7 +299,7 @@ impl<'tcx> Visitor<'tcx> for SideEffectVisit<'tcx> {
             ExprKind::Match(expr, arms, _) => self.visit_match(expr, arms),
             // since analysing the closure is not easy, just set all variables in it to side-effect
             ExprKind::Closure(_, _, body_id, _, _) => {
-                let body = self.ty_ctx.hir().body(body_id);
+                let body = self.tcx.hir().body(body_id);
                 self.visit_body(body);
                 let vars = std::mem::take(&mut self.ret_vars);
                 self.add_side_effect(vars);