"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,
DEPRECATED,
UNUSED_UNSAFE,
UNUSED_MUT,
+ UNCONDITIONAL_RECURSION,
SINGLE_USE_LIFETIMES,
UNUSED_LIFETIMES,
UNUSED_LABELS,
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};
}
}
-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,
MISSING_DEBUG_IMPLEMENTATIONS,
ANONYMOUS_PARAMETERS,
UNUSED_DOC_COMMENTS,
- UNCONDITIONAL_RECURSION,
PLUGIN_AS_LIBRARY,
PRIVATE_NO_MANGLE_FNS,
PRIVATE_NO_MANGLE_STATICS,
UnusedAllocation: UnusedAllocation,
MissingCopyImplementations: MissingCopyImplementations,
UnstableFeatures: UnstableFeatures,
- UnconditionalRecursion: UnconditionalRecursion,
InvalidNoMangleItems: InvalidNoMangleItems,
PluginAsLibrary: PluginAsLibrary,
MutableTransmutes: MutableTransmutes,
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();
mir_util::dump_mir(tcx, None, "mir_map", &0,
MirSource::item(def_id), &mir, |_, _| Ok(()) );
+ lints::check(tcx, &mir, def_id);
+
mir
})
}
mod build;
mod dataflow;
mod hair;
+mod lints;
mod shim;
pub mod transform;
pub mod util;
--- /dev/null
+// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+use rustc_data_structures::bit_set::BitSet;
+use rustc::hir::def_id::DefId;
+use rustc::hir::intravisit::FnKind;
+use rustc::hir::map::blocks::FnLikeNode;
+use rustc::lint::builtin::UNCONDITIONAL_RECURSION;
+use rustc::mir::{self, Mir, TerminatorKind};
+use rustc::ty::{AssociatedItem, AssociatedItemContainer, Instance, TyCtxt, TyKind};
+use rustc::ty::subst::Substs;
+
+pub fn check(tcx: TyCtxt<'a, 'tcx, 'tcx>,
+ mir: &Mir<'tcx>,
+ def_id: DefId) {
+ let node_id = tcx.hir.as_local_node_id(def_id).unwrap();
+
+ if let Some(fn_like_node) = FnLikeNode::from_node(tcx.hir.get(node_id)) {
+ check_fn_for_unconditional_recursion(tcx, fn_like_node.kind(), mir, def_id);
+ }
+}
+
+fn check_fn_for_unconditional_recursion(tcx: TyCtxt<'a, 'tcx, 'tcx>,
+ fn_kind: FnKind,
+ mir: &Mir<'tcx>,
+ def_id: DefId) {
+ if let FnKind::Closure(_) = fn_kind {
+ // closures can't recur, so they don't matter.
+ return;
+ }
+
+ //FIXME(#54444) rewrite this lint to use the dataflow framework
+
+ // 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 basic_blocks = mir.basic_blocks();
+ let mut reachable_without_self_call_queue = vec![mir::START_BLOCK];
+ let mut reached_exit_without_self_call = false;
+ let mut self_call_locations = vec![];
+ let mut visited = BitSet::new_empty(basic_blocks.len());
+
+ let param_env = tcx.param_env(def_id);
+ let trait_substs_count =
+ match tcx.opt_associated_item(def_id) {
+ Some(AssociatedItem {
+ container: AssociatedItemContainer::TraitContainer(trait_def_id),
+ ..
+ }) => tcx.generics_of(trait_def_id).count(),
+ _ => 0
+ };
+ let caller_substs = &Substs::identity_for_item(tcx, def_id)[..trait_substs_count];
+
+ while let Some(bb) = reachable_without_self_call_queue.pop() {
+ if visited.contains(bb) {
+ //already done
+ continue;
+ }
+
+ visited.insert(bb);
+
+ let block = &basic_blocks[bb];
+
+ if let Some(ref terminator) = block.terminator {
+ match terminator.kind {
+ TerminatorKind::Call { ref func, .. } => {
+ let func_ty = func.ty(mir, tcx);
+
+ if let TyKind::FnDef(fn_def_id, substs) = func_ty.sty {
+ let (call_fn_id, call_substs) =
+ if let Some(instance) = Instance::resolve(tcx,
+ param_env,
+ fn_def_id,
+ substs) {
+ (instance.def_id(), instance.substs)
+ } else {
+ (fn_def_id, substs)
+ };
+
+ let is_self_call =
+ call_fn_id == def_id &&
+ &call_substs[..caller_substs.len()] == caller_substs;
+
+ if is_self_call {
+ self_call_locations.push(terminator.source_info);
+
+ //this is a self call so we shouldn't explore
+ //further down this path
+ continue;
+ }
+ }
+ },
+ TerminatorKind::Abort | TerminatorKind::Return => {
+ //found a path!
+ reached_exit_without_self_call = true;
+ break;
+ }
+ _ => {}
+ }
+
+ for successor in terminator.successors() {
+ reachable_without_self_call_queue.push(*successor);
+ }
+ }
+ }
+
+ // 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_locations.is_empty() {
+ let node_id = tcx.hir.as_local_node_id(def_id).unwrap();
+ let sp = tcx.sess.source_map().def_span(tcx.hir.span(node_id));
+ let mut db = tcx.struct_span_lint_node(UNCONDITIONAL_RECURSION,
+ node_id,
+ sp,
+ "function cannot return without recursing");
+ db.span_label(sp, "cannot return without recursing");
+ // offer some help to the programmer.
+ for location in &self_call_locations {
+ db.span_label(location.span, "recursive call site");
+ }
+ db.help("a `loop` may express intention better if this is on purpose");
+ db.emit();
+ }
+}
| cannot borrow as mutable
| try removing `&mut` here
+warning: function cannot return without recursing
+ --> $DIR/issue-31424.rs:22:5
+ |
+LL | fn bar(self: &mut Self) {
+ | ^^^^^^^^^^^^^^^^^^^^^^^ cannot return without recursing
+LL | //~^ WARN function cannot return without recursing
+LL | (&mut self).bar(); //~ ERROR cannot borrow
+ | ----------------- recursive call site
+ |
+ = note: #[warn(unconditional_recursion)] on by default
+ = help: a `loop` may express intention better if this is on purpose
+
error[E0596]: cannot borrow `self` as mutable, as it is not declared as mutable
- --> $DIR/issue-31424.rs:23:9
+ --> $DIR/issue-31424.rs:24:9
|
LL | (&mut self).bar(); //~ ERROR cannot borrow
| ^^^^^^^^^^^
// In this case we could keep the suggestion, but to distinguish the
// two cases is pretty hard. It's an obscure case anyway.
fn bar(self: &mut Self) {
+ //~^ WARN function cannot return without recursing
(&mut self).bar(); //~ ERROR cannot borrow
}
}
| cannot reborrow mutably
| try removing `&mut` here
+warning: function cannot return without recursing
+ --> $DIR/issue-31424.rs:22:5
+ |
+LL | fn bar(self: &mut Self) {
+ | ^^^^^^^^^^^^^^^^^^^^^^^ cannot return without recursing
+LL | //~^ WARN function cannot return without recursing
+LL | (&mut self).bar(); //~ ERROR cannot borrow
+ | ----------------- recursive call site
+ |
+ = note: #[warn(unconditional_recursion)] on by default
+ = help: a `loop` may express intention better if this is on purpose
+
error[E0596]: cannot borrow immutable argument `self` as mutable
- --> $DIR/issue-31424.rs:23:15
+ --> $DIR/issue-31424.rs:24:15
|
LL | (&mut self).bar(); //~ ERROR cannot borrow
| ^^^^ cannot borrow mutably
impl Struct {
fn bar(self: &mut Self) {
+ //~^ WARN function cannot return without recursing
(&mut self).bar();
//~^ ERROR cannot borrow `self` as mutable, as it is not declared as mutable [E0596]
}
+warning: function cannot return without recursing
+ --> $DIR/issue-51191.rs:16:5
+ |
+LL | fn bar(self: &mut Self) {
+ | ^^^^^^^^^^^^^^^^^^^^^^^ cannot return without recursing
+LL | //~^ WARN function cannot return without recursing
+LL | (&mut self).bar();
+ | ----------------- recursive call site
+ |
+ = note: #[warn(unconditional_recursion)] on by default
+ = help: a `loop` may express intention better if this is on purpose
+
error[E0596]: cannot borrow `self` as mutable, as it is not declared as mutable
- --> $DIR/issue-51191.rs:17:9
+ --> $DIR/issue-51191.rs:18:9
|
LL | (&mut self).bar();
| ^^^^^^^^^^^
| try removing `&mut` here
error[E0596]: cannot borrow `self` as mutable, as it is not declared as mutable
- --> $DIR/issue-51191.rs:22:9
+ --> $DIR/issue-51191.rs:23:9
|
LL | fn imm(self) {
| ---- help: consider changing this to be mutable: `mut self`
| ^^^^^^^^^^^ cannot borrow as mutable
error[E0596]: cannot borrow `self` as mutable, as it is not declared as mutable
- --> $DIR/issue-51191.rs:31:9
+ --> $DIR/issue-51191.rs:32:9
|
LL | (&mut self).bar();
| ^^^^^^^^^^^ cannot borrow as mutable
error[E0596]: cannot borrow data in a `&` reference as mutable
- --> $DIR/issue-51191.rs:31:9
+ --> $DIR/issue-51191.rs:32:9
|
LL | (&mut self).bar();
| ^^^^^^^^^^^ cannot borrow as mutable
error[E0596]: cannot borrow `self` as mutable, as it is not declared as mutable
- --> $DIR/issue-51191.rs:37:9
+ --> $DIR/issue-51191.rs:38:9
|
LL | (&mut self).bar();
| ^^^^^^^^^^^
+warning: function cannot return without recursing
+ --> $DIR/region-bound-on-closure-outlives-call.rs:11:1
+ |
+LL | fn call_rec<F>(mut f: F) -> usize where F: FnMut(usize) -> usize {
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ cannot return without recursing
+LL | //~^ WARN function cannot return without recursing
+LL | (|x| f(x))(call_rec(f)) //~ ERROR cannot move out of `f`
+ | ----------- recursive call site
+ |
+ = note: #[warn(unconditional_recursion)] on by default
+ = help: a `loop` may express intention better if this is on purpose
+
error[E0505]: cannot move out of `f` because it is borrowed
- --> $DIR/region-bound-on-closure-outlives-call.rs:12:25
+ --> $DIR/region-bound-on-closure-outlives-call.rs:13:25
|
LL | (|x| f(x))(call_rec(f)) //~ ERROR cannot move out of `f`
| --------------------^--
// except according to those terms.
fn call_rec<F>(mut f: F) -> usize where F: FnMut(usize) -> usize {
+ //~^ WARN function cannot return without recursing
(|x| f(x))(call_rec(f)) //~ ERROR cannot move out of `f`
}
+warning: function cannot return without recursing
+ --> $DIR/region-bound-on-closure-outlives-call.rs:11:1
+ |
+LL | fn call_rec<F>(mut f: F) -> usize where F: FnMut(usize) -> usize {
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ cannot return without recursing
+LL | //~^ WARN function cannot return without recursing
+LL | (|x| f(x))(call_rec(f)) //~ ERROR cannot move out of `f`
+ | ----------- recursive call site
+ |
+ = note: #[warn(unconditional_recursion)] on by default
+ = help: a `loop` may express intention better if this is on purpose
+
error[E0505]: cannot move out of `f` because it is borrowed
- --> $DIR/region-bound-on-closure-outlives-call.rs:12:25
+ --> $DIR/region-bound-on-closure-outlives-call.rs:13:25
|
LL | (|x| f(x))(call_rec(f)) //~ ERROR cannot move out of `f`
| --- ^ move out of `f` occurs here