diff --git a/src/enum_glob_use.rs b/src/enum_glob_use.rs index 5b542a7d67b..7924623c189 100644 --- a/src/enum_glob_use.rs +++ b/src/enum_glob_use.rs @@ -43,20 +43,21 @@ impl EnumGlobUse { } if let ItemUse(ref item_use) = item.node { if let ViewPath_::ViewPathGlob(_) = item_use.node { - let def = cx.tcx.def_map.borrow()[&item.id]; - if let Some(node_id) = cx.tcx.map.as_local_node_id(def.def_id()) { - if let Some(NodeItem(it)) = cx.tcx.map.find(node_id) { - if let ItemEnum(..) = it.node { - span_lint(cx, ENUM_GLOB_USE, item.span, "don't use glob imports for enum variants"); - } - } - } else { - if let Some(dp) = cx.sess().cstore.def_path(def.def_id()).last() { - if let DefPathData::Type(_) = dp.data { - if let TyEnum(..) = cx.sess().cstore.item_type(&cx.tcx, def.def_id()).ty.sty { + if let Some(def) = cx.tcx.def_map.borrow().get(&item.id) { + if let Some(node_id) = cx.tcx.map.as_local_node_id(def.def_id()) { + if let Some(NodeItem(it)) = cx.tcx.map.find(node_id) { + if let ItemEnum(..) = it.node { span_lint(cx, ENUM_GLOB_USE, item.span, "don't use glob imports for enum variants"); } } + } else { + if let Some(dp) = cx.sess().cstore.def_path(def.def_id()).last() { + if let DefPathData::Type(_) = dp.data { + if let TyEnum(..) = cx.sess().cstore.item_type(&cx.tcx, def.def_id()).ty.sty { + span_lint(cx, ENUM_GLOB_USE, item.span, "don't use glob imports for enum variants"); + } + } + } } } } diff --git a/src/methods.rs b/src/methods.rs index 95bbdd441c9..81761695eb8 100644 --- a/src/methods.rs +++ b/src/methods.rs @@ -402,8 +402,8 @@ impl LateLintPass for MethodsPass { } } - let ret_ty = return_ty(cx.tcx.node_id_to_type(implitem.id)); - if &name.as_str() == &"new" && !ret_ty.map_or(false, |ret_ty| ret_ty.walk().any(|t| same_tys(cx, t, ty))) { + let ret_ty = return_ty(cx, implitem.id); + if &name.as_str() == &"new" && !ret_ty.map_or(false, |ret_ty| ret_ty.walk().any(|t| same_tys(cx, t, ty, implitem.id))) { span_lint(cx, NEW_RET_NO_SELF, sig.explicit_self.span, diff --git a/src/new_without_default.rs b/src/new_without_default.rs index d9b11cc49b6..395d69138e1 100644 --- a/src/new_without_default.rs +++ b/src/new_without_default.rs @@ -52,8 +52,8 @@ impl LateLintPass for NewWithoutDefault { if_let_chain!{[ self_ty.walk_shallow().next().is_none(), // implements_trait does not work with generics - let Some(ret_ty) = return_ty(cx.tcx.node_id_to_type(id)), - same_tys(cx, self_ty, ret_ty), + let Some(ret_ty) = return_ty(cx, id), + same_tys(cx, self_ty, ret_ty, id), let Some(default_trait_id) = get_trait_def_id(cx, &DEFAULT_TRAIT_PATH), !implements_trait(cx, self_ty, default_trait_id, Vec::new()) ], { diff --git a/src/types.rs b/src/types.rs index 1dc1f55b773..742298622fc 100644 --- a/src/types.rs +++ b/src/types.rs @@ -1,7 +1,6 @@ use reexport::*; use rustc::lint::*; -use rustc::middle::const_eval; -use rustc::middle::ty; +use rustc::middle::{const_eval, def, ty}; use rustc_front::hir::*; use rustc_front::intravisit::{FnKind, Visitor, walk_ty}; use rustc_front::util::{is_comparison_binop, binop_to_string}; @@ -53,21 +52,34 @@ impl LateLintPass for TypePass { if in_macro(cx, ast_ty.span) { return; } - if let Some(ty) = cx.tcx.ast_ty_to_ty_cache.borrow().get(&ast_ty.id) { - if let ty::TyBox(ref inner) = ty.sty { - if match_type(cx, inner, &VEC_PATH) { + if let Some(did) = cx.tcx.def_map.borrow().get(&ast_ty.id) { + if let def::Def::Struct(..) = did.full_def() { + if Some(did.def_id()) == cx.tcx.lang_items.owned_box() { + if_let_chain! { + [ + let TyPath(_, ref path) = ast_ty.node, + let Some(ref last) = path.segments.last(), + let PathParameters::AngleBracketedParameters(ref ag) = last.parameters, + let Some(ref vec) = ag.types.get(0), + let Some(did) = cx.tcx.def_map.borrow().get(&vec.id), + let def::Def::Struct(..) = did.full_def(), + match_def_path(cx, did.def_id(), &VEC_PATH), + ], + { + span_help_and_lint(cx, + BOX_VEC, + ast_ty.span, + "you seem to be trying to use `Box>`. Consider using just `Vec`", + "`Vec` is already on the heap, `Box>` makes an extra allocation."); + } + } + } else if match_def_path(cx, did.def_id(), &LL_PATH) { span_help_and_lint(cx, - BOX_VEC, + LINKEDLIST, ast_ty.span, - "you seem to be trying to use `Box>`. Consider using just `Vec`", - "`Vec` is already on the heap, `Box>` makes an extra allocation."); + "I see you're using a LinkedList! Perhaps you meant some other data structure?", + "a VecDeque might work"); } - } else if match_type(cx, ty, &LL_PATH) { - span_help_and_lint(cx, - LINKEDLIST, - ast_ty.span, - "I see you're using a LinkedList! Perhaps you meant some other data structure?", - "a VecDeque might work"); } } } diff --git a/src/utils/mod.rs b/src/utils/mod.rs index e7b2a9d5210..0389c3d56c0 100644 --- a/src/utils/mod.rs +++ b/src/utils/mod.rs @@ -4,6 +4,7 @@ use rustc::lint::{LintContext, LateContext, Level, Lint}; use rustc::middle::def_id::DefId; use rustc::middle::traits::ProjectionMode; use rustc::middle::{cstore, def, infer, ty, traits}; +use rustc::middle::subst::Subst; use rustc::session::Session; use rustc_front::hir::*; use std::borrow::Cow; @@ -54,6 +55,7 @@ pub const STRING_PATH: [&'static str; 3] = ["collections", "string", "String"]; pub const TRANSMUTE_PATH: [&'static str; 3] = ["core", "intrinsics", "transmute"]; pub const VEC_FROM_ELEM_PATH: [&'static str; 3] = ["std", "vec", "from_elem"]; pub const VEC_PATH: [&'static str; 3] = ["collections", "vec", "Vec"]; +pub const BOX_PATH: [&'static str; 3] = ["std", "boxed", "Box"]; /// Produce a nested chain of if-lets and ifs from the patterns: /// @@ -89,6 +91,11 @@ macro_rules! if_let_chain { $block } }; + ([let $pat:pat = $expr:expr,], $block:block) => { + if let $pat = $expr { + $block + } + }; ([$expr:expr, $($tt:tt)+], $block:block) => { if $expr { if_let_chain!{ [$($tt)+], $block } @@ -99,6 +106,11 @@ macro_rules! if_let_chain { $block } }; + ([$expr:expr,], $block:block) => { + if $expr { + $block + } + }; } /// Returns true if the two spans come from differing expansions (i.e. one is from a macro and one @@ -764,8 +776,13 @@ pub fn unsugar_range(expr: &Expr) -> Option { } /// Convenience function to get the return type of a function or `None` if the function diverges. -pub fn return_ty(fun: ty::Ty) -> Option { - if let ty::FnConverging(ret_ty) = fun.fn_sig().skip_binder().output { +pub fn return_ty<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, fn_item: NodeId) -> Option> { + let parameter_env = ty::ParameterEnvironment::for_item(cx.tcx, fn_item); + let infcx = infer::new_infer_ctxt(cx.tcx, &cx.tcx.tables, Some(parameter_env), ProjectionMode::Any); + + let fn_sig = cx.tcx.node_id_to_type(fn_item).fn_sig().subst(infcx.tcx, &infcx.parameter_environment.free_substs); + let fn_sig = infcx.tcx.liberate_late_bound_regions(infcx.parameter_environment.free_id_outlive, &fn_sig); + if let ty::FnConverging(ret_ty) = fn_sig.output { Some(ret_ty) } else { None @@ -775,7 +792,10 @@ pub fn return_ty(fun: ty::Ty) -> Option { /// Check if two types are the same. // FIXME: this works correctly for lifetimes bounds (`for <'a> Foo<'a>` == `for <'b> Foo<'b>` but // not for type parameters. -pub fn same_tys<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, a: ty::Ty<'tcx>, b: ty::Ty<'tcx>) -> bool { - let infcx = infer::new_infer_ctxt(cx.tcx, &cx.tcx.tables, None, ProjectionMode::Any); - infcx.can_equate(&cx.tcx.erase_regions(&a), &cx.tcx.erase_regions(&b)).is_ok() +pub fn same_tys<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, a: ty::Ty<'tcx>, b: ty::Ty<'tcx>, parameter_item: NodeId) -> bool { + let parameter_env = ty::ParameterEnvironment::for_item(cx.tcx, parameter_item); + let infcx = infer::new_infer_ctxt(cx.tcx, &cx.tcx.tables, Some(parameter_env), ProjectionMode::Any); + let new_a = a.subst(infcx.tcx, &infcx.parameter_environment.free_substs); + let new_b = b.subst(infcx.tcx, &infcx.parameter_environment.free_substs); + infcx.can_equate(&new_a, &new_b).is_ok() } diff --git a/tests/compile-fail/methods.rs b/tests/compile-fail/methods.rs index edbdeb2e55a..74f262b05a7 100644 --- a/tests/compile-fail/methods.rs +++ b/tests/compile-fail/methods.rs @@ -47,6 +47,15 @@ impl<'a> Lt2<'a> { pub fn new(s: &str) -> Lt2 { unimplemented!() } } +struct Lt3<'a> { + foo: &'a u32, +} + +impl<'a> Lt3<'a> { + // The lifetime is different, but that’s irrelevant, see #734 + pub fn new() -> Lt3<'static> { unimplemented!() } +} + #[derive(Clone,Copy)] struct U;