Skip to content

Commit 4bd4e41

Browse files
borsbors
committed
Auto merge of #54490 - wesleywiser:rewrite_it_in_mir, r=oli-obk
Rewrite the `UnconditionalRecursion` lint to use MIR Part of #51002 r? @eddyb
2 parents 8ec22e7 + f81d1dd commit 4bd4e41

14 files changed

+240
-287
lines changed

‎src/librustc/lint/builtin.rs

+7
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,12 @@ declare_lint! {
233233
"detect mut variables which don't need to be mutable"
234234
}
235235

236+
declare_lint! {
237+
pub UNCONDITIONAL_RECURSION,
238+
Warn,
239+
"functions that cannot return without calling themselves"
240+
}
241+
236242
declare_lint! {
237243
pub SINGLE_USE_LIFETIMES,
238244
Allow,
@@ -402,6 +408,7 @@ impl LintPass for HardwiredLints {
402408
DEPRECATED,
403409
UNUSED_UNSAFE,
404410
UNUSED_MUT,
411+
UNCONDITIONAL_RECURSION,
405412
SINGLE_USE_LIFETIMES,
406413
UNUSED_LIFETIMES,
407414
UNUSED_LABELS,

‎src/librustc_lint/builtin.rs

-277
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,7 @@
3030
3131
use rustc::hir::def::Def;
3232
use rustc::hir::def_id::DefId;
33-
use rustc::cfg;
34-
use rustc::ty::subst::Substs;
3533
use rustc::ty::{self, Ty};
36-
use rustc::traits;
3734
use hir::Node;
3835
use util::nodemap::NodeSet;
3936
use lint::{LateContext, LintContext, LintArray};
@@ -844,279 +841,6 @@ impl EarlyLintPass for UnusedDocComment {
844841
}
845842
}
846843

847-
declare_lint! {
848-
pub UNCONDITIONAL_RECURSION,
849-
Warn,
850-
"functions that cannot return without calling themselves"
851-
}
852-
853-
#[derive(Copy, Clone)]
854-
pub struct UnconditionalRecursion;
855-
856-
857-
impl LintPass for UnconditionalRecursion {
858-
fn get_lints(&self) -> LintArray {
859-
lint_array![UNCONDITIONAL_RECURSION]
860-
}
861-
}
862-
863-
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnconditionalRecursion {
864-
fn check_fn(&mut self,
865-
cx: &LateContext,
866-
fn_kind: FnKind,
867-
_: &hir::FnDecl,
868-
body: &hir::Body,
869-
sp: Span,
870-
id: ast::NodeId) {
871-
let method = match fn_kind {
872-
FnKind::ItemFn(..) => None,
873-
FnKind::Method(..) => {
874-
Some(cx.tcx.associated_item(cx.tcx.hir.local_def_id(id)))
875-
}
876-
// closures can't recur, so they don't matter.
877-
FnKind::Closure(_) => return,
878-
};
879-
880-
// Walk through this function (say `f`) looking to see if
881-
// every possible path references itself, i.e. the function is
882-
// called recursively unconditionally. This is done by trying
883-
// to find a path from the entry node to the exit node that
884-
// *doesn't* call `f` by traversing from the entry while
885-
// pretending that calls of `f` are sinks (i.e. ignoring any
886-
// exit edges from them).
887-
//
888-
// NB. this has an edge case with non-returning statements,
889-
// like `loop {}` or `panic!()`: control flow never reaches
890-
// the exit node through these, so one can have a function
891-
// that never actually calls itself but is still picked up by
892-
// this lint:
893-
//
894-
// fn f(cond: bool) {
895-
// if !cond { panic!() } // could come from `assert!(cond)`
896-
// f(false)
897-
// }
898-
//
899-
// In general, functions of that form may be able to call
900-
// itself a finite number of times and then diverge. The lint
901-
// considers this to be an error for two reasons, (a) it is
902-
// easier to implement, and (b) it seems rare to actually want
903-
// to have behaviour like the above, rather than
904-
// e.g. accidentally recursing after an assert.
905-
906-
let cfg = cfg::CFG::new(cx.tcx, &body);
907-
908-
let mut work_queue = vec![cfg.entry];
909-
let mut reached_exit_without_self_call = false;
910-
let mut self_call_spans = vec![];
911-
let mut visited = FxHashSet::default();
912-
913-
while let Some(idx) = work_queue.pop() {
914-
if idx == cfg.exit {
915-
// found a path!
916-
reached_exit_without_self_call = true;
917-
break;
918-
}
919-
920-
let cfg_id = idx.node_id();
921-
if visited.contains(&cfg_id) {
922-
// already done
923-
continue;
924-
}
925-
visited.insert(cfg_id);
926-
927-
// is this a recursive call?
928-
let local_id = cfg.graph.node_data(idx).id();
929-
if local_id != hir::DUMMY_ITEM_LOCAL_ID {
930-
let node_id = cx.tcx.hir.hir_to_node_id(hir::HirId {
931-
owner: body.value.hir_id.owner,
932-
local_id
933-
});
934-
let self_recursive = match method {
935-
Some(ref method) => expr_refers_to_this_method(cx, method, node_id),
936-
None => expr_refers_to_this_fn(cx, id, node_id),
937-
};
938-
if self_recursive {
939-
self_call_spans.push(cx.tcx.hir.span(node_id));
940-
// this is a self call, so we shouldn't explore past
941-
// this node in the CFG.
942-
continue;
943-
}
944-
}
945-
946-
// add the successors of this node to explore the graph further.
947-
for (_, edge) in cfg.graph.outgoing_edges(idx) {
948-
let target_idx = edge.target();
949-
let target_cfg_id = target_idx.node_id();
950-
if !visited.contains(&target_cfg_id) {
951-
work_queue.push(target_idx)
952-
}
953-
}
954-
}
955-
956-
// Check the number of self calls because a function that
957-
// doesn't return (e.g. calls a `-> !` function or `loop { /*
958-
// no break */ }`) shouldn't be linted unless it actually
959-
// recurs.
960-
if !reached_exit_without_self_call && !self_call_spans.is_empty() {
961-
let sp = cx.tcx.sess.source_map().def_span(sp);
962-
let mut db = cx.struct_span_lint(UNCONDITIONAL_RECURSION,
963-
sp,
964-
"function cannot return without recursing");
965-
db.span_label(sp, "cannot return without recursing");
966-
// offer some help to the programmer.
967-
for call in &self_call_spans {
968-
db.span_label(*call, "recursive call site");
969-
}
970-
db.help("a `loop` may express intention better if this is on purpose");
971-
db.emit();
972-
}
973-
974-
// all done
975-
return;
976-
977-
// Functions for identifying if the given Expr NodeId `id`
978-
// represents a call to the function `fn_id`/method `method`.
979-
980-
fn expr_refers_to_this_fn(cx: &LateContext, fn_id: ast::NodeId, id: ast::NodeId) -> bool {
981-
match cx.tcx.hir.get(id) {
982-
Node::Expr(&hir::Expr { node: hir::ExprKind::Call(ref callee, _), .. }) => {
983-
let def = if let hir::ExprKind::Path(ref qpath) = callee.node {
984-
cx.tables.qpath_def(qpath, callee.hir_id)
985-
} else {
986-
return false;
987-
};
988-
match def {
989-
Def::Local(..) | Def::Upvar(..) => false,
990-
_ => def.def_id() == cx.tcx.hir.local_def_id(fn_id)
991-
}
992-
}
993-
_ => false,
994-
}
995-
}
996-
997-
// Check if the expression `id` performs a call to `method`.
998-
fn expr_refers_to_this_method(cx: &LateContext,
999-
method: &ty::AssociatedItem,
1000-
id: ast::NodeId)
1001-
-> bool {
1002-
use rustc::ty::adjustment::*;
1003-
1004-
// Ignore non-expressions.
1005-
let expr = if let Node::Expr(e) = cx.tcx.hir.get(id) {
1006-
e
1007-
} else {
1008-
return false;
1009-
};
1010-
1011-
// Check for overloaded autoderef method calls.
1012-
let mut source = cx.tables.expr_ty(expr);
1013-
for adjustment in cx.tables.expr_adjustments(expr) {
1014-
if let Adjust::Deref(Some(deref)) = adjustment.kind {
1015-
let (def_id, substs) = deref.method_call(cx.tcx, source);
1016-
if method_call_refers_to_method(cx, method, def_id, substs, id) {
1017-
return true;
1018-
}
1019-
}
1020-
source = adjustment.target;
1021-
}
1022-
1023-
// Check for method calls and overloaded operators.
1024-
if cx.tables.is_method_call(expr) {
1025-
let hir_id = cx.tcx.hir.definitions().node_to_hir_id(id);
1026-
if let Some(def) = cx.tables.type_dependent_defs().get(hir_id) {
1027-
let def_id = def.def_id();
1028-
let substs = cx.tables.node_substs(hir_id);
1029-
if method_call_refers_to_method(cx, method, def_id, substs, id) {
1030-
return true;
1031-
}
1032-
} else {
1033-
cx.tcx.sess.delay_span_bug(expr.span,
1034-
"no type-dependent def for method call");
1035-
}
1036-
}
1037-
1038-
// Check for calls to methods via explicit paths (e.g. `T::method()`).
1039-
match expr.node {
1040-
hir::ExprKind::Call(ref callee, _) => {
1041-
let def = if let hir::ExprKind::Path(ref qpath) = callee.node {
1042-
cx.tables.qpath_def(qpath, callee.hir_id)
1043-
} else {
1044-
return false;
1045-
};
1046-
match def {
1047-
Def::Method(def_id) => {
1048-
let substs = cx.tables.node_substs(callee.hir_id);
1049-
method_call_refers_to_method(cx, method, def_id, substs, id)
1050-
}
1051-
_ => false,
1052-
}
1053-
}
1054-
_ => false,
1055-
}
1056-
}
1057-
1058-
// Check if the method call to the method with the ID `callee_id`
1059-
// and instantiated with `callee_substs` refers to method `method`.
1060-
fn method_call_refers_to_method<'a, 'tcx>(cx: &LateContext<'a, 'tcx>,
1061-
method: &ty::AssociatedItem,
1062-
callee_id: DefId,
1063-
callee_substs: &Substs<'tcx>,
1064-
expr_id: ast::NodeId)
1065-
-> bool {
1066-
let tcx = cx.tcx;
1067-
let callee_item = tcx.associated_item(callee_id);
1068-
1069-
match callee_item.container {
1070-
// This is an inherent method, so the `def_id` refers
1071-
// directly to the method definition.
1072-
ty::ImplContainer(_) => callee_id == method.def_id,
1073-
1074-
// A trait method, from any number of possible sources.
1075-
// Attempt to select a concrete impl before checking.
1076-
ty::TraitContainer(trait_def_id) => {
1077-
let trait_ref = ty::TraitRef::from_method(tcx, trait_def_id, callee_substs);
1078-
let trait_ref = ty::Binder::bind(trait_ref);
1079-
let span = tcx.hir.span(expr_id);
1080-
let obligation =
1081-
traits::Obligation::new(traits::ObligationCause::misc(span, expr_id),
1082-
cx.param_env,
1083-
trait_ref.to_poly_trait_predicate());
1084-
1085-
tcx.infer_ctxt().enter(|infcx| {
1086-
let mut selcx = traits::SelectionContext::new(&infcx);
1087-
match selcx.select(&obligation) {
1088-
// The method comes from a `T: Trait` bound.
1089-
// If `T` is `Self`, then this call is inside
1090-
// a default method definition.
1091-
Ok(Some(traits::VtableParam(_))) => {
1092-
let on_self = trait_ref.self_ty().is_self();
1093-
// We can only be recursing in a default
1094-
// method if we're being called literally
1095-
// on the `Self` type.
1096-
on_self && callee_id == method.def_id
1097-
}
1098-
1099-
// The `impl` is known, so we check that with a
1100-
// special case:
1101-
Ok(Some(traits::VtableImpl(vtable_impl))) => {
1102-
let container = ty::ImplContainer(vtable_impl.impl_def_id);
1103-
// It matches if it comes from the same impl,
1104-
// and has the same method name.
1105-
container == method.container &&
1106-
callee_item.ident.name == method.ident.name
1107-
}
1108-
1109-
// There's no way to know if this call is
1110-
// recursive, so we assume it's not.
1111-
_ => false,
1112-
}
1113-
})
1114-
}
1115-
}
1116-
}
1117-
}
1118-
}
1119-
1120844
declare_lint! {
1121845
PLUGIN_AS_LIBRARY,
1122846
Warn,
@@ -1724,7 +1448,6 @@ impl LintPass for SoftLints {
17241448
MISSING_DEBUG_IMPLEMENTATIONS,
17251449
ANONYMOUS_PARAMETERS,
17261450
UNUSED_DOC_COMMENTS,
1727-
UNCONDITIONAL_RECURSION,
17281451
PLUGIN_AS_LIBRARY,
17291452
NO_MANGLE_CONST_ITEMS,
17301453
NO_MANGLE_GENERIC_ITEMS,

‎src/librustc_lint/lib.rs

-1
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,6 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) {
144144
UnusedAllocation: UnusedAllocation,
145145
MissingCopyImplementations: MissingCopyImplementations,
146146
UnstableFeatures: UnstableFeatures,
147-
UnconditionalRecursion: UnconditionalRecursion,
148147
InvalidNoMangleItems: InvalidNoMangleItems,
149148
PluginAsLibrary: PluginAsLibrary,
150149
MutableTransmutes: MutableTransmutes,

‎src/librustc_mir/build/mod.rs

+4
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,8 @@ use syntax_pos::Span;
3535
use transform::MirSource;
3636
use util as mir_util;
3737

38+
use super::lints;
39+
3840
/// Construct the MIR for a given def-id.
3941
pub fn mir_build<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId) -> Mir<'tcx> {
4042
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
176178
mir_util::dump_mir(tcx, None, "mir_map", &0,
177179
MirSource::item(def_id), &mir, |_, _| Ok(()) );
178180

181+
lints::check(tcx, &mir, def_id);
182+
179183
mir
180184
})
181185
}

‎src/librustc_mir/lib.rs

+1
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@ mod borrow_check;
7979
mod build;
8080
mod dataflow;
8181
mod hair;
82+
mod lints;
8283
mod shim;
8384
pub mod transform;
8485
pub mod util;

0 commit comments

Comments
 (0)