From 1ca59031389ef3ebd029faf889c5eaae2a169438 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Mon, 15 Feb 2016 10:20:26 +0530 Subject: [PATCH 1/3] Make derive lint handle generics correctly --- src/derive.rs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/derive.rs b/src/derive.rs index 467d55feafd..7cb85b8c97b 100644 --- a/src/derive.rs +++ b/src/derive.rs @@ -1,4 +1,5 @@ use rustc::lint::*; +use rustc::middle::subst::Subst; use rustc::middle::ty::TypeVariants; use rustc::middle::ty::fast_reject::simplify_type; use rustc::middle::ty; @@ -70,16 +71,17 @@ impl LintPass for Derive { impl LateLintPass for Derive { fn check_item(&mut self, cx: &LateContext, item: &Item) { - let ast_ty_to_ty_cache = cx.tcx.ast_ty_to_ty_cache.borrow(); + if_let_chain! {[ - let ItemImpl(_, _, ref ast_generics, Some(ref trait_ref), ref ast_ty, _) = item.node, - let Some(&ty) = ast_ty_to_ty_cache.get(&ast_ty.id) + let ItemImpl(_, _, _, Some(ref trait_ref), _, _) = item.node ], { + + let ty = cx.tcx.lookup_item_type(cx.tcx.map.local_def_id(item.id)).ty; if item.attrs.iter().any(is_automatically_derived) { check_hash_peq(cx, item.span, trait_ref, ty); } - else if !ast_generics.is_lt_parameterized() { + else { check_copy_clone(cx, item, trait_ref, ty); } }} @@ -132,8 +134,9 @@ fn check_copy_clone<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, trait_ref: &TraitRef, ty: ty::Ty<'tcx>) { if match_path(&trait_ref.path, &CLONE_TRAIT_PATH) { let parameter_environment = ty::ParameterEnvironment::for_item(cx.tcx, item.id); + let subst_ty = ty.subst(cx.tcx, ¶meter_environment.free_substs); - if ty.moves_by_default(¶meter_environment, item.span) { + if subst_ty.moves_by_default(¶meter_environment, item.span) { return; // ty is not Copy } From d755b1ebe2204b93c8ab82c6c04699a9e20921f5 Mon Sep 17 00:00:00 2001 From: mcarton Date: Mon, 15 Feb 2016 13:25:29 +0100 Subject: [PATCH 2/3] Cleanup --- src/derive.rs | 3 --- tests/compile-fail/derive.rs | 11 +++++++++++ tests/ice-666.rs | 24 ------------------------ 3 files changed, 11 insertions(+), 27 deletions(-) delete mode 100644 tests/ice-666.rs diff --git a/src/derive.rs b/src/derive.rs index 7cb85b8c97b..7110cf71424 100644 --- a/src/derive.rs +++ b/src/derive.rs @@ -71,12 +71,9 @@ impl LintPass for Derive { impl LateLintPass for Derive { fn check_item(&mut self, cx: &LateContext, item: &Item) { - - if_let_chain! {[ let ItemImpl(_, _, _, Some(ref trait_ref), _, _) = item.node ], { - let ty = cx.tcx.lookup_item_type(cx.tcx.map.local_def_id(item.id)).ty; if item.attrs.iter().any(is_automatically_derived) { check_hash_peq(cx, item.span, trait_ref, ty); diff --git a/tests/compile-fail/derive.rs b/tests/compile-fail/derive.rs index 66b04a66d0f..14b1106add5 100755 --- a/tests/compile-fail/derive.rs +++ b/tests/compile-fail/derive.rs @@ -35,6 +35,17 @@ impl Clone for Qux { fn clone(&self) -> Self { Qux } } +// See #666 +#[derive(Copy)] +struct Lt<'a> { + a: &'a u8, +} + +impl<'a> Clone for Lt<'a> { +//~^ ERROR you are implementing `Clone` explicitly on a `Copy` type + fn clone(&self) -> Self { unimplemented!() } +} + // Ok, `Clone` cannot be derived because of the big array #[derive(Copy)] struct BigArray { diff --git a/tests/ice-666.rs b/tests/ice-666.rs deleted file mode 100644 index b681f4b2c58..00000000000 --- a/tests/ice-666.rs +++ /dev/null @@ -1,24 +0,0 @@ -#![feature(plugin)] -#![plugin(clippy)] - -pub struct Lt<'a> { - _foo: &'a u8, -} - -impl<'a> Copy for Lt<'a> {} -impl<'a> Clone for Lt<'a> { - fn clone(&self) -> Lt<'a> { - unimplemented!(); - } -} - -pub struct Ty { - _foo: A, -} - -impl Copy for Ty {} -impl Clone for Ty { - fn clone(&self) -> Ty { - unimplemented!(); - } -} From 570b9635354c8d41711f464bcaeb6a7128d1cc2a Mon Sep 17 00:00:00 2001 From: mcarton Date: Mon, 15 Feb 2016 13:44:59 +0100 Subject: [PATCH 3/3] Replace potentially ICEgen ast_ty_to_ty_cache --- src/methods.rs | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/src/methods.rs b/src/methods.rs index ed3f61e4a72..b71d2ffd7a2 100644 --- a/src/methods.rs +++ b/src/methods.rs @@ -366,7 +366,7 @@ impl LateLintPass for MethodsPass { return; } - if let ItemImpl(_, _, _, None, ref ty, ref items) = item.node { + if let ItemImpl(_, _, _, None, _, ref items) = item.node { for implitem in items { let name = implitem.name; if let ImplItemKind::Method(ref sig, _) = implitem.node { @@ -387,6 +387,7 @@ impl LateLintPass for MethodsPass { } // check conventions w.r.t. conversion method names and predicates + let ty = cx.tcx.lookup_item_type(cx.tcx.map.local_def_id(item.id)).ty; let is_copy = is_copy(cx, &ty, &item); for &(ref conv, self_kinds) in &CONVENTIONS { if conv.check(&name.as_str()) && @@ -412,12 +413,13 @@ impl LateLintPass for MethodsPass { if &name.as_str() == &"new" { let returns_self = if let FunctionRetTy::Return(ref ret_ty) = sig.decl.output { let ast_ty_to_ty_cache = cx.tcx.ast_ty_to_ty_cache.borrow(); - let ty = ast_ty_to_ty_cache.get(&ty.id); let ret_ty = ast_ty_to_ty_cache.get(&ret_ty.id); - match (ty, ret_ty) { - (Some(&ty), Some(&ret_ty)) => ret_ty.walk().any(|t| t == ty), - _ => false, + if let Some(&ret_ty) = ret_ty { + ret_ty.walk().any(|t| t == ty) + } + else { + false } } else { @@ -983,12 +985,7 @@ fn is_bool(ty: &Ty) -> bool { false } -fn is_copy(cx: &LateContext, ast_ty: &Ty, item: &Item) -> bool { - match cx.tcx.ast_ty_to_ty_cache.borrow().get(&ast_ty.id) { - None => false, - Some(ty) => { - let env = ty::ParameterEnvironment::for_item(cx.tcx, item.id); - !ty.subst(cx.tcx, &env.free_substs).moves_by_default(&env, ast_ty.span) - } - } +fn is_copy<'a, 'ctx>(cx: &LateContext<'a, 'ctx>, ty: ty::Ty<'ctx>, item: &Item) -> bool { + let env = ty::ParameterEnvironment::for_item(cx.tcx, item.id); + !ty.subst(cx.tcx, &env.free_substs).moves_by_default(&env, item.span) }