Rewrite the UnconditionalRecursion lint to use MIR

Part of #51002

Author: wesleywiser

src/librustc/lint/builtin.rs

@@ -233,6 +233,12 @@ declare_lint! {
     "detect mut variables which don't need to be mutable"
 }
 
+declare_lint! {
+    pub UNCONDITIONAL_RECURSION,
+    Warn,
+    "functions that cannot return without calling themselves"
+}
+
 declare_lint! {
     pub SINGLE_USE_LIFETIMES,
     Allow,
@@ -396,6 +402,7 @@ impl LintPass for HardwiredLints {
             DEPRECATED,
             UNUSED_UNSAFE,
             UNUSED_MUT,
+            UNCONDITIONAL_RECURSION,
             SINGLE_USE_LIFETIMES,
             UNUSED_LIFETIMES,
             UNUSED_LABELS,

src/librustc_lint/builtin.rs

@@ -30,10 +30,7 @@
 
 use rustc::hir::def::Def;
 use rustc::hir::def_id::DefId;
-use rustc::cfg;
-use rustc::ty::subst::Substs;
 use rustc::ty::{self, Ty};
-use rustc::traits;
 use hir::Node;
 use util::nodemap::NodeSet;
 use lint::{LateContext, LintContext, LintArray};
@@ -844,279 +841,6 @@ impl EarlyLintPass for UnusedDocComment {
     }
 }
 
-declare_lint! {
-    pub UNCONDITIONAL_RECURSION,
-    Warn,
-    "functions that cannot return without calling themselves"
-}
-
-#[derive(Copy, Clone)]
-pub struct UnconditionalRecursion;
-
-
-impl LintPass for UnconditionalRecursion {
-    fn get_lints(&self) -> LintArray {
-        lint_array![UNCONDITIONAL_RECURSION]
-    }
-}
-
-impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnconditionalRecursion {
-    fn check_fn(&mut self,
-                cx: &LateContext,
-                fn_kind: FnKind,
-                _: &hir::FnDecl,
-                body: &hir::Body,
-                sp: Span,
-                id: ast::NodeId) {
-        let method = match fn_kind {
-            FnKind::ItemFn(..) => None,
-            FnKind::Method(..) => {
-                Some(cx.tcx.associated_item(cx.tcx.hir.local_def_id(id)))
-            }
-            // closures can't recur, so they don't matter.
-            FnKind::Closure(_) => return,
-        };
-
-        // Walk through this function (say `f`) looking to see if
-        // every possible path references itself, i.e. the function is
-        // called recursively unconditionally. This is done by trying
-        // to find a path from the entry node to the exit node that
-        // *doesn't* call `f` by traversing from the entry while
-        // pretending that calls of `f` are sinks (i.e. ignoring any
-        // exit edges from them).
-        //
-        // NB. this has an edge case with non-returning statements,
-        // like `loop {}` or `panic!()`: control flow never reaches
-        // the exit node through these, so one can have a function
-        // that never actually calls itself but is still picked up by
-        // this lint:
-        //
-        //     fn f(cond: bool) {
-        //         if !cond { panic!() } // could come from `assert!(cond)`
-        //         f(false)
-        //     }
-        //
-        // In general, functions of that form may be able to call
-        // itself a finite number of times and then diverge. The lint
-        // considers this to be an error for two reasons, (a) it is
-        // easier to implement, and (b) it seems rare to actually want
-        // to have behaviour like the above, rather than
-        // e.g. accidentally recursing after an assert.
-
-        let cfg = cfg::CFG::new(cx.tcx, &body);
-
-        let mut work_queue = vec![cfg.entry];
-        let mut reached_exit_without_self_call = false;
-        let mut self_call_spans = vec![];
-        let mut visited = FxHashSet::default();
-
-        while let Some(idx) = work_queue.pop() {
-            if idx == cfg.exit {
-                // found a path!
-                reached_exit_without_self_call = true;
-                break;
-            }
-
-            let cfg_id = idx.node_id();
-            if visited.contains(&cfg_id) {
-                // already done
-                continue;
-            }
-            visited.insert(cfg_id);
-
-            // is this a recursive call?
-            let local_id = cfg.graph.node_data(idx).id();
-            if local_id != hir::DUMMY_ITEM_LOCAL_ID {
-                let node_id = cx.tcx.hir.hir_to_node_id(hir::HirId {
-                    owner: body.value.hir_id.owner,
-                    local_id
-                });
-                let self_recursive = match method {
-                    Some(ref method) => expr_refers_to_this_method(cx, method, node_id),
-                    None => expr_refers_to_this_fn(cx, id, node_id),
-                };
-                if self_recursive {
-                    self_call_spans.push(cx.tcx.hir.span(node_id));
-                    // this is a self call, so we shouldn't explore past
-                    // this node in the CFG.
-                    continue;
-                }
-            }
-
-            // add the successors of this node to explore the graph further.
-            for (_, edge) in cfg.graph.outgoing_edges(idx) {
-                let target_idx = edge.target();
-                let target_cfg_id = target_idx.node_id();
-                if !visited.contains(&target_cfg_id) {
-                    work_queue.push(target_idx)
-                }
-            }
-        }
-
-        // Check the number of self calls because a function that
-        // doesn't return (e.g. calls a `-> !` function or `loop { /*
-        // no break */ }`) shouldn't be linted unless it actually
-        // recurs.
-        if !reached_exit_without_self_call && !self_call_spans.is_empty() {
-            let sp = cx.tcx.sess.source_map().def_span(sp);
-            let mut db = cx.struct_span_lint(UNCONDITIONAL_RECURSION,
-                                             sp,
-                                             "function cannot return without recursing");
-            db.span_label(sp, "cannot return without recursing");
-            // offer some help to the programmer.
-            for call in &self_call_spans {
-                db.span_label(*call, "recursive call site");
-            }
-            db.help("a `loop` may express intention better if this is on purpose");
-            db.emit();
-        }
-
-        // all done
-        return;
-
-        // Functions for identifying if the given Expr NodeId `id`
-        // represents a call to the function `fn_id`/method `method`.
-
-        fn expr_refers_to_this_fn(cx: &LateContext, fn_id: ast::NodeId, id: ast::NodeId) -> bool {
-            match cx.tcx.hir.get(id) {
-                Node::Expr(&hir::Expr { node: hir::ExprKind::Call(ref callee, _), .. }) => {
-                    let def = if let hir::ExprKind::Path(ref qpath) = callee.node {
-                        cx.tables.qpath_def(qpath, callee.hir_id)
-                    } else {
-                        return false;
-                    };
-                    match def {
-                        Def::Local(..) | Def::Upvar(..) => false,
-                        _ => def.def_id() == cx.tcx.hir.local_def_id(fn_id)
-                    }
-                }
-                _ => false,
-            }
-        }
-
-        // Check if the expression `id` performs a call to `method`.
-        fn expr_refers_to_this_method(cx: &LateContext,
-                                      method: &ty::AssociatedItem,
-                                      id: ast::NodeId)
-                                      -> bool {
-            use rustc::ty::adjustment::*;
-
-            // Ignore non-expressions.
-            let expr = if let Node::Expr(e) = cx.tcx.hir.get(id) {
-                e
-            } else {
-                return false;
-            };
-
-            // Check for overloaded autoderef method calls.
-            let mut source = cx.tables.expr_ty(expr);
-            for adjustment in cx.tables.expr_adjustments(expr) {
-                if let Adjust::Deref(Some(deref)) = adjustment.kind {
-                    let (def_id, substs) = deref.method_call(cx.tcx, source);
-                    if method_call_refers_to_method(cx, method, def_id, substs, id) {
-                        return true;
-                    }
-                }
-                source = adjustment.target;
-            }
-
-            // Check for method calls and overloaded operators.
-            if cx.tables.is_method_call(expr) {
-                let hir_id = cx.tcx.hir.definitions().node_to_hir_id(id);
-                if let Some(def) = cx.tables.type_dependent_defs().get(hir_id) {
-                    let def_id = def.def_id();
-                    let substs = cx.tables.node_substs(hir_id);
-                    if method_call_refers_to_method(cx, method, def_id, substs, id) {
-                        return true;
-                    }
-                } else {
-                    cx.tcx.sess.delay_span_bug(expr.span,
-                                               "no type-dependent def for method call");
-                }
-            }
-
-            // Check for calls to methods via explicit paths (e.g. `T::method()`).
-            match expr.node {
-                hir::ExprKind::Call(ref callee, _) => {
-                    let def = if let hir::ExprKind::Path(ref qpath) = callee.node {
-                        cx.tables.qpath_def(qpath, callee.hir_id)
-                    } else {
-                        return false;
-                    };
-                    match def {
-                        Def::Method(def_id) => {
-                            let substs = cx.tables.node_substs(callee.hir_id);
-                            method_call_refers_to_method(cx, method, def_id, substs, id)
-                        }
-                        _ => false,
-                    }
-                }
-                _ => false,
-            }
-        }
-
-        // Check if the method call to the method with the ID `callee_id`
-        // and instantiated with `callee_substs` refers to method `method`.
-        fn method_call_refers_to_method<'a, 'tcx>(cx: &LateContext<'a, 'tcx>,
-                                                  method: &ty::AssociatedItem,
-                                                  callee_id: DefId,
-                                                  callee_substs: &Substs<'tcx>,
-                                                  expr_id: ast::NodeId)
-                                                  -> bool {
-            let tcx = cx.tcx;
-            let callee_item = tcx.associated_item(callee_id);
-
-            match callee_item.container {
-                // This is an inherent method, so the `def_id` refers
-                // directly to the method definition.
-                ty::ImplContainer(_) => callee_id == method.def_id,
-
-                // A trait method, from any number of possible sources.
-                // Attempt to select a concrete impl before checking.
-                ty::TraitContainer(trait_def_id) => {
-                    let trait_ref = ty::TraitRef::from_method(tcx, trait_def_id, callee_substs);
-                    let trait_ref = ty::Binder::bind(trait_ref);
-                    let span = tcx.hir.span(expr_id);
-                    let obligation =
-                        traits::Obligation::new(traits::ObligationCause::misc(span, expr_id),
-                                                cx.param_env,
-                                                trait_ref.to_poly_trait_predicate());
-
-                    tcx.infer_ctxt().enter(|infcx| {
-                        let mut selcx = traits::SelectionContext::new(&infcx);
-                        match selcx.select(&obligation) {
-                            // The method comes from a `T: Trait` bound.
-                            // If `T` is `Self`, then this call is inside
-                            // a default method definition.
-                            Ok(Some(traits::VtableParam(_))) => {
-                                let on_self = trait_ref.self_ty().is_self();
-                                // We can only be recursing in a default
-                                // method if we're being called literally
-                                // on the `Self` type.
-                                on_self && callee_id == method.def_id
-                            }
-
-                            // The `impl` is known, so we check that with a
-                            // special case:
-                            Ok(Some(traits::VtableImpl(vtable_impl))) => {
-                                let container = ty::ImplContainer(vtable_impl.impl_def_id);
-                                // It matches if it comes from the same impl,
-                                // and has the same method name.
-                                container == method.container &&
-                                callee_item.ident.name == method.ident.name
-                            }
-
-                            // There's no way to know if this call is
-                            // recursive, so we assume it's not.
-                            _ => false,
-                        }
-                    })
-                }
-            }
-        }
-    }
-}
-
 declare_lint! {
     PLUGIN_AS_LIBRARY,
     Warn,
@@ -1783,7 +1507,6 @@ impl LintPass for SoftLints {
             MISSING_DEBUG_IMPLEMENTATIONS,
             ANONYMOUS_PARAMETERS,
             UNUSED_DOC_COMMENTS,
-            UNCONDITIONAL_RECURSION,
             PLUGIN_AS_LIBRARY,
             PRIVATE_NO_MANGLE_FNS,
             PRIVATE_NO_MANGLE_STATICS,

src/librustc_lint/lib.rs

@@ -144,7 +144,6 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) {
         UnusedAllocation: UnusedAllocation,
         MissingCopyImplementations: MissingCopyImplementations,
         UnstableFeatures: UnstableFeatures,
-        UnconditionalRecursion: UnconditionalRecursion,
         InvalidNoMangleItems: InvalidNoMangleItems,
         PluginAsLibrary: PluginAsLibrary,
         MutableTransmutes: MutableTransmutes,

src/librustc_mir/build/mod.rs

@@ -35,6 +35,8 @@ use syntax_pos::Span;
 use transform::MirSource;
 use util as mir_util;
 
+use super::lints;
+
 /// Construct the MIR for a given def-id.
 pub fn mir_build<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId) -> Mir<'tcx> {
     let id = tcx.hir.as_local_node_id(def_id).unwrap();
@@ -176,6 +178,8 @@ pub fn mir_build<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId) -> Mir<'t
         mir_util::dump_mir(tcx, None, "mir_map", &0,
                            MirSource::item(def_id), &mir, |_, _| Ok(()) );
 
+        lints::check(tcx, &mir, def_id);
+
         mir
     })
 }

src/librustc_mir/lib.rs

@@ -79,6 +79,7 @@ mod borrow_check;
 mod build;
 mod dataflow;
 mod hair;
+mod lints;
 mod shim;
 pub mod transform;
 pub mod util;