Skip to content

rustc_lint: handle more method calls in unconditional_recursion. #26783

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

Merged
merged 2 commits into from
Aug 4, 2015
Merged

rustc_lint: handle more method calls in unconditional_recursion. #26783

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a34b0a4
rustc: replace def::MethodProvenance with ty::ImplOrTraitItemContainer.
eddyb Aug 3, 2015
585f0e9
rustc_lint: handle more method calls in unconditional_recursion.
eddyb Aug 3, 2015
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
23 changes: 2 additions & 21 deletions src/librustc/metadata/decoder.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -299,15 +299,7 @@ fn item_to_def_like(cdata: Cmd, item: rbml::Doc, did: ast::DefId) -> DefLike {
Constant => {
// Check whether we have an associated const item.
if item_sort(item) == Some('C') {
// Check whether the associated const is from a trait or impl.
// See the comment for methods below.
let provenance = if reader::maybe_get_doc(
item, tag_item_trait_parent_sort).is_some() {
def::FromTrait(item_require_parent_item(cdata, item))
} else {
def::FromImpl(item_require_parent_item(cdata, item))
};
DlDef(def::DefAssociatedConst(did, provenance))
DlDef(def::DefAssociatedConst(did))
} else {
// Regular const item.
DlDef(def::DefConst(did))
Expand All @@ -319,18 +311,7 @@ fn item_to_def_like(cdata: Cmd, item: rbml::Doc, did: ast::DefId) -> DefLike {
Fn => DlDef(def::DefFn(did, false)),
CtorFn => DlDef(def::DefFn(did, true)),
Method | StaticMethod => {
// def_static_method carries an optional field of its enclosing
// trait or enclosing impl (if this is an inherent static method).
// So we need to detect whether this is in a trait or not, which
// we do through the mildly hacky way of checking whether there is
// a trait_parent_sort.
let provenance = if reader::maybe_get_doc(
item, tag_item_trait_parent_sort).is_some() {
def::FromTrait(item_require_parent_item(cdata, item))
} else {
def::FromImpl(item_require_parent_item(cdata, item))
};
DlDef(def::DefMethod(did, provenance))
DlDef(def::DefMethod(did))
}
Type => {
if item_sort(item) == Some('t') {
Expand Down
8 changes: 2 additions & 6 deletions src/librustc/middle/astencode.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -444,9 +444,7 @@ impl tr for def::Def {
fn tr(&self, dcx: &DecodeContext) -> def::Def {
match *self {
def::DefFn(did, is_ctor) => def::DefFn(did.tr(dcx), is_ctor),
def::DefMethod(did, p) => {
def::DefMethod(did.tr(dcx), p.map(|did2| did2.tr(dcx)))
}
def::DefMethod(did) => def::DefMethod(did.tr(dcx)),
def::DefSelfTy(opt_did, impl_ids) => { def::DefSelfTy(opt_did.map(|did| did.tr(dcx)),
impl_ids.map(|(nid1, nid2)| {
(dcx.tr_id(nid1),
Expand All @@ -456,9 +454,7 @@ impl tr for def::Def {
def::DefForeignMod(did) => { def::DefForeignMod(did.tr(dcx)) }
def::DefStatic(did, m) => { def::DefStatic(did.tr(dcx), m) }
def::DefConst(did) => { def::DefConst(did.tr(dcx)) }
def::DefAssociatedConst(did, p) => {
def::DefAssociatedConst(did.tr(dcx), p.map(|did2| did2.tr(dcx)))
}
def::DefAssociatedConst(did) => def::DefAssociatedConst(did.tr(dcx)),
def::DefLocal(nid) => { def::DefLocal(dcx.tr_id(nid)) }
def::DefVariant(e_did, v_did, is_s) => {
def::DefVariant(e_did.tr(dcx), v_did.tr(dcx), is_s)
Expand Down
11 changes: 9 additions & 2 deletions src/librustc/middle/check_const.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -650,7 +650,7 @@ fn check_expr<'a, 'tcx>(v: &mut CheckCrateVisitor<'a, 'tcx>,
}
}
Some(def::DefConst(did)) |
Some(def::DefAssociatedConst(did, _)) => {
Some(def::DefAssociatedConst(did)) => {
if let Some(expr) = const_eval::lookup_const_by_id(v.tcx, did,
Some(e.id)) {
let inner = v.global_expr(Mode::Const, expr);
Expand Down Expand Up @@ -696,10 +696,17 @@ fn check_expr<'a, 'tcx>(v: &mut CheckCrateVisitor<'a, 'tcx>,
v.add_qualif(ConstQualif::NON_ZERO_SIZED);
true
}
Some(def::DefMethod(did, def::FromImpl(_))) |
Some(def::DefFn(did, _)) => {
v.handle_const_fn_call(e, did, node_ty)
}
Some(def::DefMethod(did)) => {
match v.tcx.impl_or_trait_item(did).container() {
ty::ImplContainer(_) => {
v.handle_const_fn_call(e, did, node_ty)
}
ty::TraitContainer(_) => false
}
}
_ => false
};
if !is_const {
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/middle/check_match.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -442,7 +442,7 @@ impl<'a, 'tcx> Folder for StaticInliner<'a, 'tcx> {
ast::PatIdent(..) | ast::PatEnum(..) | ast::PatQPath(..) => {
let def = self.tcx.def_map.borrow().get(&pat.id).map(|d| d.full_def());
match def {
Some(DefAssociatedConst(did, _)) |
Some(DefAssociatedConst(did)) |
Some(DefConst(did)) => match lookup_const_by_id(self.tcx, did, Some(pat.id)) {
Some(const_expr) => {
const_expr_to_pat(self.tcx, const_expr, pat.span).map(|new_pat| {
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/middle/check_static_recursion.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ impl<'a, 'ast: 'a> Visitor<'ast> for CheckItemRecursionVisitor<'a, 'ast> {
ast::ExprPath(..) => {
match self.def_map.borrow().get(&e.id).map(|d| d.base_def) {
Some(DefStatic(def_id, _)) |
Some(DefAssociatedConst(def_id, _)) |
Some(DefAssociatedConst(def_id)) |
Some(DefConst(def_id))
if ast_util::is_local(def_id) => {
match self.ast_map.get(def_id.node) {
Expand Down
10 changes: 5 additions & 5 deletions src/librustc/middle/const_eval.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ fn lookup_const<'a>(tcx: &'a ty::ctxt, e: &Expr) -> Option<&'a Expr> {
let opt_def = tcx.def_map.borrow().get(&e.id).map(|d| d.full_def());
match opt_def {
Some(def::DefConst(def_id)) |
Some(def::DefAssociatedConst(def_id, _)) => {
Some(def::DefAssociatedConst(def_id)) => {
lookup_const_by_id(tcx, def_id, Some(e.id))
}
Some(def::DefVariant(enum_def, variant_def, _)) => {
Expand Down Expand Up @@ -929,10 +929,10 @@ pub fn eval_const_expr_partial<'tcx>(tcx: &ty::ctxt<'tcx>,
(lookup_const_by_id(tcx, def_id, Some(e.id)), None)
}
}
Some(def::DefAssociatedConst(def_id, provenance)) => {
Some(def::DefAssociatedConst(def_id)) => {
if ast_util::is_local(def_id) {
match provenance {
def::FromTrait(trait_id) => match tcx.map.find(def_id.node) {
match tcx.impl_or_trait_item(def_id).container() {
ty::TraitContainer(trait_id) => match tcx.map.find(def_id.node) {
Some(ast_map::NodeTraitItem(ti)) => match ti.node {
ast::ConstTraitItem(ref ty, _) => {
if let ExprTypeChecked = ty_hint {
Expand All @@ -950,7 +950,7 @@ pub fn eval_const_expr_partial<'tcx>(tcx: &ty::ctxt<'tcx>,
},
_ => (None, None)
},
def::FromImpl(_) => match tcx.map.find(def_id.node) {
ty::ImplContainer(_) => match tcx.map.find(def_id.node) {
Some(ast_map::NodeImplItem(ii)) => match ii.node {
ast::ConstImplItem(ref ty, ref expr) => {
(Some(&**expr), Some(&**ty))
Expand Down
24 changes: 3 additions & 21 deletions src/librustc/middle/def.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
// except according to those terms.

pub use self::Def::*;
pub use self::MethodProvenance::*;

use middle::privacy::LastPrivate;
use middle::subst::ParamSpace;
Expand All @@ -28,7 +27,7 @@ pub enum Def {
DefForeignMod(ast::DefId),
DefStatic(ast::DefId, bool /* is_mutbl */),
DefConst(ast::DefId),
DefAssociatedConst(ast::DefId /* const */, MethodProvenance),
DefAssociatedConst(ast::DefId),
DefLocal(ast::NodeId),
DefVariant(ast::DefId /* enum */, ast::DefId /* variant */, bool /* is_structure */),
DefTy(ast::DefId, bool /* is_enum */),
Expand All @@ -51,7 +50,7 @@ pub enum Def {
DefStruct(ast::DefId),
DefRegion(ast::NodeId),
DefLabel(ast::NodeId),
DefMethod(ast::DefId /* method */, MethodProvenance),
DefMethod(ast::DefId),
}

/// The result of resolving a path.
Expand Down Expand Up @@ -112,23 +111,6 @@ pub struct Export {
pub def_id: ast::DefId, // The definition of the target.
}

#[derive(Clone, Copy, PartialEq, Eq, RustcEncodable, RustcDecodable, Hash, Debug)]
pub enum MethodProvenance {
FromTrait(ast::DefId),
FromImpl(ast::DefId),
}

impl MethodProvenance {
pub fn map<F>(self, f: F) -> MethodProvenance where
F: FnOnce(ast::DefId) -> ast::DefId,
{
match self {
FromTrait(did) => FromTrait(f(did)),
FromImpl(did) => FromImpl(f(did))
}
}
}

impl Def {
pub fn local_node_id(&self) -> ast::NodeId {
let def_id = self.def_id();
Expand All @@ -141,7 +123,7 @@ impl Def {
DefFn(id, _) | DefMod(id) | DefForeignMod(id) | DefStatic(id, _) |
DefVariant(_, id, _) | DefTy(id, _) | DefAssociatedTy(_, id) |
DefTyParam(_, _, id, _) | DefUse(id) | DefStruct(id) | DefTrait(id) |
DefMethod(id, _) | DefConst(id) | DefAssociatedConst(id, _) |
DefMethod(id) | DefConst(id) | DefAssociatedConst(id) |
DefSelfTy(Some(id), None)=> {
id
}
Expand Down
68 changes: 53 additions & 15 deletions src/librustc_lint/builtin.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@

use metadata::{csearch, decoder};
use middle::{cfg, def, infer, pat_util, stability, traits};
use middle::def::*;
use middle::subst::Substs;
use middle::ty::{self, Ty};
use middle::const_eval::{eval_const_expr_partial, ConstVal};
Expand Down Expand Up @@ -2251,34 +2250,73 @@ impl LintPass for UnconditionalRecursion {
}
}

// Check if the method call `id` refers to method `method`.
// Check if the expression `id` performs a call to `method`.
fn expr_refers_to_this_method(tcx: &ty::ctxt,
method: &ty::Method,
id: ast::NodeId) -> bool {
let method_call = ty::MethodCall::expr(id);
let callee = match tcx.tables.borrow().method_map.get(&method_call) {
Some(&m) => m,
None => return false
};
let callee_item = tcx.impl_or_trait_item(callee.def_id);
let tables = tcx.tables.borrow();

// Check for method calls and overloaded operators.
if let Some(m) = tables.method_map.get(&ty::MethodCall::expr(id)) {
if method_call_refers_to_method(tcx, method, m.def_id, m.substs, id) {
return true;
}
}

// Check for overloaded autoderef method calls.
if let Some(&ty::AdjustDerefRef(ref adj)) = tables.adjustments.get(&id) {
for i in 0..adj.autoderefs {
let method_call = ty::MethodCall::autoderef(id, i as u32);
if let Some(m) = tables.method_map.get(&method_call) {
if method_call_refers_to_method(tcx, method, m.def_id, m.substs, id) {
return true;
}
}
}
}

// Check for calls to methods via explicit paths (e.g. `T::method()`).
match tcx.map.get(id) {
ast_map::NodeExpr(&ast::Expr { node: ast::ExprCall(ref callee, _), .. }) => {
match tcx.def_map.borrow().get(&callee.id).map(|d| d.full_def()) {
Some(def::DefMethod(def_id)) => {
let no_substs = &ty::ItemSubsts::empty();
let ts = tables.item_substs.get(&callee.id).unwrap_or(no_substs);
method_call_refers_to_method(tcx, method, def_id, &ts.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<'tcx>(tcx: &ty::ctxt<'tcx>,
method: &ty::Method,
callee_id: ast::DefId,
callee_substs: &Substs<'tcx>,
expr_id: ast::NodeId) -> bool {
let callee_item = tcx.impl_or_trait_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.def_id == method.def_id
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_substs = callee.substs.clone().method_to_trait();
let trait_substs = callee_substs.clone().method_to_trait();
let trait_substs = tcx.mk_substs(trait_substs);
let trait_ref = ty::TraitRef::new(trait_def_id, trait_substs);
let trait_ref = ty::Binder(trait_ref);
let span = tcx.map.span(id);
let span = tcx.map.span(expr_id);
let obligation =
traits::Obligation::new(traits::ObligationCause::misc(span, id),
traits::Obligation::new(traits::ObligationCause::misc(span, expr_id),
trait_ref.to_poly_trait_predicate());

let param_env = ty::ParameterEnvironment::for_item(tcx, method.def_id.node);
Expand All @@ -2289,12 +2327,12 @@ impl LintPass for UnconditionalRecursion {
// If `T` is `Self`, then this call is inside
// a default method definition.
Ok(Some(traits::VtableParam(_))) => {
let self_ty = callee.substs.self_ty();
let self_ty = callee_substs.self_ty();
let on_self = self_ty.map_or(false, |t| t.is_self());
// We can only be recurring in a default
// method if we're being called literally
// on the `Self` type.
on_self && callee.def_id == method.def_id
on_self && callee_id == method.def_id
}

// The `impl` is known, so we check that with a
Expand Down Expand Up @@ -2454,7 +2492,7 @@ impl LintPass for MutableTransmutes {
ast::ExprPath(..) => (),
_ => return None
}
if let DefFn(did, _) = cx.tcx.resolve_expr(expr) {
if let def::DefFn(did, _) = cx.tcx.resolve_expr(expr) {
if !def_id_is_transmute(cx, did) {
return None;
}
Expand Down
6 changes: 2 additions & 4 deletions src/librustc_resolve/build_reduced_graph.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -545,14 +545,12 @@ impl<'a, 'b:'a, 'tcx:'b> GraphBuilder<'a, 'b, 'tcx> {

match trait_item.node {
ast::ConstTraitItem(..) => {
let def = DefAssociatedConst(local_def(trait_item.id),
FromTrait(local_def(item.id)));
let def = DefAssociatedConst(local_def(trait_item.id));
// NB: not DefModifiers::IMPORTABLE
name_bindings.define_value(def, trait_item.span, DefModifiers::PUBLIC);
}
ast::MethodTraitItem(..) => {
let def = DefMethod(local_def(trait_item.id),
FromTrait(local_def(item.id)));
let def = DefMethod(local_def(trait_item.id));
// NB: not DefModifiers::IMPORTABLE
name_bindings.define_value(def, trait_item.span, DefModifiers::PUBLIC);
}
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_resolve/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3448,7 +3448,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
// Look for a method in the current self type's impl module.
if let Some(module) = get_module(self, path.span, &name_path) {
if let Some(binding) = module.children.borrow().get(&name) {
if let Some(DefMethod(did, _)) = binding.def_for_namespace(ValueNS) {
if let Some(DefMethod(did)) = binding.def_for_namespace(ValueNS) {
if is_static_method(self, did) {
return StaticMethod(path_names_to_string(&path, 0))
}
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_trans/save/dump_csv.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -719,7 +719,7 @@ impl <'l, 'tcx> DumpCsvVisitor<'l, 'tcx> {
let def_map = self.tcx.def_map.borrow();
let def = def_map.get(&id).unwrap().full_def();
match def {
def::DefMethod(did, _) => {
def::DefMethod(did) => {
let ti = self.tcx.impl_or_trait_item(did);
if let ty::MethodTraitItem(m) = ti {
if m.explicit_self == ty::StaticExplicitSelfCategory {
Expand Down
8 changes: 4 additions & 4 deletions src/librustc_trans/save/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -551,20 +551,20 @@ impl<'l, 'tcx: 'l> SaveContext<'l, 'tcx> {
scope: self.enclosing_scope(id),
}))
}
def::DefMethod(decl_id, provenence) => {
def::DefMethod(decl_id) => {
let sub_span = self.span_utils.sub_span_for_meth_name(path.span);
let def_id = if decl_id.krate == ast::LOCAL_CRATE {
let ti = self.tcx.impl_or_trait_item(decl_id);
match provenence {
def::FromTrait(def_id) => {
match ti.container() {
ty::TraitContainer(def_id) => {
self.tcx.trait_items(def_id)
.iter()
.find(|mr| {
mr.name() == ti.name() && self.trait_method_has_body(mr)
})
.map(|mr| mr.def_id())
}
def::FromImpl(def_id) => {
ty::ImplContainer(def_id) => {
let impl_items = self.tcx.impl_items.borrow();
Some(impl_items.get(&def_id)
.unwrap()
Expand Down
Loading