Merge pull request #670 from Manishearth/better-ice-fix

Make derive lint handle generics correctly

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,14 @@ 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 +131,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, &parameter_environment.free_substs);
 
-        if ty.moves_by_default(&parameter_environment, item.span) {
+        if subst_ty.moves_by_default(&parameter_environment, item.span) {
             return; // ty is not Copy
         }
 

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)
 }

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 {

tests/ice-666.rs

@@ -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<A> {
-    _foo: A,
-}
-
-impl<A: Copy> Copy for Ty<A> {}
-impl<A> Clone for Ty<A> {
-    fn clone(&self) -> Ty<A> {
-        unimplemented!();
-    }
-}