Fix behavior with generic lifetime parameters

Author: devonhollowood

clippy_lints/src/transmute.rs

@@ -1,7 +1,8 @@
 use rustc::lint::*;
-use rustc::ty::{self, Ty};
+use rustc::ty::{self, Ty, walk::TypeWalker};
 use rustc::hir::*;
 use std::borrow::Cow;
+use std::mem;
 use syntax::ast;
 use utils::{last_path_segment, match_def_path, paths, snippet, span_lint, span_lint_and_then};
 use utils::{opt_def_id, sugg};
@@ -363,8 +364,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Transmute {
                                             }
                                         )
                                     } else {
-                                        // In this case they differ only in lifetime
-                                        if ty_from != ty_to {
+                                        if !differ_only_in_lifetime_params(from_ty, to_ty) {
                                             span_lint_and_then(
                                                 cx,
                                                 TRANSMUTE_PTR_TO_PTR,
@@ -448,6 +448,48 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Transmute {
     }
 }
 
+/// Returns true if `type1` and `type2` are the same type except for their lifetime parameters
+fn differ_only_in_lifetime_params(type1: Ty, type2: Ty) -> bool {
+    use rustc::ty::TypeVariants::*;
+    if TypeWalker::new(type1).count() != TypeWalker::new(type2).count() {
+        return false;
+    }
+    TypeWalker::new(type1)
+        .zip(TypeWalker::new(type2))
+        .all(|(t1, t2)| {
+            match (&t1.sty, &t2.sty) {
+                // types with generic parameters which can contain lifetimes
+                (TyAdt(_, sub1), TyAdt(_, sub2))
+                | (TyFnDef(_, sub1), TyFnDef(_, sub2))
+                | (TyAnon(_, sub1), TyAnon(_, sub2))
+                => {
+                    // Iterate over generic parameters, which are either Lifetimes or Types.
+                    // Here we only need to check that they are the same type of thing, because
+                    // if they are both Lifetimes then we don't care about their equality, and if
+                    // they are both Types, we will check their equality later in the type walk.
+                    sub1.iter().count() == sub2.iter().count()
+                    && sub1.iter().zip(sub2.iter()).all(|(k1, k2)| {
+                        mem::discriminant(&k1.unpack()) == mem::discriminant(&k2.unpack())
+                    })
+                }
+                // types without subtypes: check that the types are equal
+                (TyBool, TyBool)
+                | (TyChar, TyChar)
+                | (TyInt(_), TyInt(_))
+                | (TyUint(_), TyUint(_))
+                | (TyFloat(_), TyFloat(_))
+                | (TyForeign(_), TyForeign(_))
+                | (TyStr, TyStr)
+                | (TyNever, TyNever)
+                | (TyInfer(_), TyInfer(_))
+                => t1.sty == t2.sty,
+                // types with subtypes: return true for now if they are the same sort of type.
+                // we will check their subtypes later
+                (sty1, sty2) => mem::discriminant(sty1) == mem::discriminant(sty2)
+            }
+        })
+}
+
 /// Get the snippet of `Bar` in `…::transmute<Foo, &Bar>`. If that snippet is
 /// not available , use
 /// the type's `ToString` implementation. In weird cases it could lead to types

tests/ui/transmute.rs

@@ -155,6 +155,14 @@ unsafe fn transmute_lifetime<'a, 'b, T>(t: &'a T, u: &'b T) -> &'b T {
     std::mem::transmute::<&'a T, &'b T>(t)
 }
 
+struct LifetimeParam<'a> {
+    s: &'a str,
+}
+
+struct GenericParam<T> {
+    t: T,
+}
+
 #[warn(transmute_ptr_to_ptr)]
 fn transmute_ptr_to_ptr() {
     let ptr = &1u32 as *const u32;
@@ -165,15 +173,27 @@ fn transmute_ptr_to_ptr() {
         let _: *mut f32 = std::mem::transmute(mut_ptr);
         // ref-ref transmutes; bad
         let _: &f32 = std::mem::transmute(&1u32);
+        let _: &f64 = std::mem::transmute(&1f32);
+        // ^ this test is here because both f32 and f64 are the same TypeVariant, but they are not
+        // the same type
         let _: &mut f32 = std::mem::transmute(&mut 1u32);
+        let _: &GenericParam<f32> = std::mem::transmute(&GenericParam { t: 1u32 });
     }
-    // These should be fine:
-    // Recommendations for solving the above; if these break we need to update
+
+    // these are recommendations for solving the above; if these lint we need to update
     // those suggestions
     let _ = ptr as *const f32;
     let _ = mut_ptr as *mut f32;
     let _ = unsafe { &*(&1u32 as *const u32 as *const f32) };
     let _ = unsafe { &mut *(&mut 1u32 as *mut u32 as *mut f32) };
+
+    // transmute internal lifetimes, should not lint
+    let s = "hello world".to_owned();
+    let lp = LifetimeParam { s: &s };
+    let _: &LifetimeParam<'static> = unsafe { std::mem::transmute(&lp) };
+    let _: &GenericParam<&LifetimeParam<'static>> = unsafe {
+        std::mem::transmute(&GenericParam { t: &lp})
+    };
 }
 
 fn main() { }

tests/ui/transmute.stderr

@@ -205,30 +205,42 @@ error: transmute from a `&mut [u8]` to a `&mut str`
     |                                ^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `std::str::from_utf8_mut(mb).unwrap()`
 
 error: transmute from a pointer to a pointer
-   --> $DIR/transmute.rs:164:29
+   --> $DIR/transmute.rs:172:29
     |
-164 |         let _: *const f32 = std::mem::transmute(ptr);
+172 |         let _: *const f32 = std::mem::transmute(ptr);
     |                             ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `ptr as *const f32`
     |
     = note: `-D transmute-ptr-to-ptr` implied by `-D warnings`
 
 error: transmute from a pointer to a pointer
-   --> $DIR/transmute.rs:165:27
+   --> $DIR/transmute.rs:173:27
     |
-165 |         let _: *mut f32 = std::mem::transmute(mut_ptr);
+173 |         let _: *mut f32 = std::mem::transmute(mut_ptr);
     |                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `mut_ptr as *mut f32`
 
 error: transmute from a reference to a reference
-   --> $DIR/transmute.rs:167:23
+   --> $DIR/transmute.rs:175:23
     |
-167 |         let _: &f32 = std::mem::transmute(&1u32);
+175 |         let _: &f32 = std::mem::transmute(&1u32);
     |                       ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `&*(&1u32 as *const u32 as *const f32)`
 
 error: transmute from a reference to a reference
-   --> $DIR/transmute.rs:168:27
+   --> $DIR/transmute.rs:176:23
     |
-168 |         let _: &mut f32 = std::mem::transmute(&mut 1u32);
+176 |         let _: &f64 = std::mem::transmute(&1f32);
+    |                       ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `&*(&1f32 as *const f32 as *const f64)`
+
+error: transmute from a reference to a reference
+   --> $DIR/transmute.rs:179:27
+    |
+179 |         let _: &mut f32 = std::mem::transmute(&mut 1u32);
     |                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `&mut *(&mut 1u32 as *mut u32 as *mut f32)`
 
-error: aborting due to 36 previous errors
+error: transmute from a reference to a reference
+   --> $DIR/transmute.rs:180:37
+    |
+180 |         let _: &GenericParam<f32> = std::mem::transmute(&GenericParam { t: 1u32 });
+    |                                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `&*(&GenericParam { t: 1u32 } as *const GenericParam<u32> as *const GenericParam<f32>)`
+
+error: aborting due to 38 previous errors