Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rewrite the UnconditionalRecursion lint to use MIR #54490

Merged
merged 1 commit into from
Oct 25, 2018
Merged

Rewrite the UnconditionalRecursion lint to use MIR #54490

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions src/librustc/lint/builtin.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -396,6 +402,7 @@ impl LintPass for HardwiredLints {
DEPRECATED,
UNUSED_UNSAFE,
UNUSED_MUT,
UNCONDITIONAL_RECURSION,
SINGLE_USE_LIFETIMES,
UNUSED_LIFETIMES,
UNUSED_LABELS,
Expand Down
277 changes: 0 additions & 277 deletions src/librustc_lint/builtin.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
1 change: 0 additions & 1 deletion src/librustc_lint/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
4 changes: 4 additions & 0 deletions src/librustc_mir/build/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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
})
}
Expand Down
1 change: 1 addition & 0 deletions src/librustc_mir/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ mod borrow_check;
mod build;
mod dataflow;
mod hair;
mod lints;
mod shim;
pub mod transform;
pub mod util;
Expand Down
Loading