Merge pull request #2661 from devonhollowood/ptr-ptr-casts

Replace `misaligned_transmute` lint

Author: oli-obk

clippy_lints/src/deprecated_lints.rs

@@ -71,3 +71,14 @@ declare_deprecated_lint! {
     pub STRING_TO_STRING,
     "using `string::to_string` is common even today and specialization will likely happen soon"
 }
+
+/// **What it does:** Nothing. This lint has been deprecated.
+///
+/// **Deprecation reason:** This lint should never have applied to non-pointer types, as transmuting
+/// between non-pointer types of differing alignment is well-defined behavior (it's semantically
+/// equivalent to a memcpy). This lint has thus been refactored into two separate lints:
+/// cast_ptr_alignment and transmute_ptr_to_ptr.
+declare_deprecated_lint! {
+    pub MISALIGNED_TRANSMUTE,
+    "this lint has been split into cast_ptr_alignment and transmute_ptr_to_ptr"
+}

clippy_lints/src/lib.rs

@@ -276,6 +276,10 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
         "string_to_string",
         "using `string::to_string` is common even today and specialization will likely happen soon",
     );
+    store.register_removed(
+        "misaligned_transmute",
+        "this lint has been split into cast_ptr_alignment and transmute_ptr_to_ptr",
+    );
     // end deprecated lints, do not remove this comment, it’s used in `update_lints`
 
     reg.register_late_lint_pass(box serde_api::Serde);
@@ -633,18 +637,19 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
         swap::MANUAL_SWAP,
         temporary_assignment::TEMPORARY_ASSIGNMENT,
         transmute::CROSSPOINTER_TRANSMUTE,
-        transmute::MISALIGNED_TRANSMUTE,
         transmute::TRANSMUTE_BYTES_TO_STR,
         transmute::TRANSMUTE_INT_TO_BOOL,
         transmute::TRANSMUTE_INT_TO_CHAR,
         transmute::TRANSMUTE_INT_TO_FLOAT,
         transmute::TRANSMUTE_PTR_TO_REF,
+        transmute::TRANSMUTE_PTR_TO_PTR,
         transmute::USELESS_TRANSMUTE,
         transmute::WRONG_TRANSMUTE,
         types::ABSURD_EXTREME_COMPARISONS,
         types::BORROWED_BOX,
         types::BOX_VEC,
         types::CAST_LOSSLESS,
+        types::CAST_PTR_ALIGNMENT,
         types::CHAR_LIT_AS_U8,
         types::IMPLICIT_HASHER,
         types::LET_UNIT_VALUE,
@@ -782,12 +787,12 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
         swap::MANUAL_SWAP,
         temporary_assignment::TEMPORARY_ASSIGNMENT,
         transmute::CROSSPOINTER_TRANSMUTE,
-        transmute::MISALIGNED_TRANSMUTE,
         transmute::TRANSMUTE_BYTES_TO_STR,
         transmute::TRANSMUTE_INT_TO_BOOL,
         transmute::TRANSMUTE_INT_TO_CHAR,
         transmute::TRANSMUTE_INT_TO_FLOAT,
         transmute::TRANSMUTE_PTR_TO_REF,
+        transmute::TRANSMUTE_PTR_TO_PTR,
         transmute::USELESS_TRANSMUTE,
         types::BORROWED_BOX,
         types::CAST_LOSSLESS,
@@ -845,6 +850,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
         swap::ALMOST_SWAPPED,
         transmute::WRONG_TRANSMUTE,
         types::ABSURD_EXTREME_COMPARISONS,
+        types::CAST_PTR_ALIGNMENT,
         types::UNIT_CMP,
         unicode::ZERO_WIDTH_SPACE,
         unused_io_amount::UNUSED_IO_AMOUNT,

clippy_lints/src/transmute.rs

@@ -1,7 +1,6 @@
 use rustc::lint::*;
 use rustc::ty::{self, Ty};
 use rustc::hir::*;
-use rustc::ty::layout::LayoutOf;
 use std::borrow::Cow;
 use syntax::ast;
 use utils::{last_path_segment, match_def_path, paths, snippet, span_lint, span_lint_and_then};
@@ -169,21 +168,31 @@ declare_clippy_lint! {
     "transmutes from an integer to a float"
 }
 
-/// **What it does:** Checks for transmutes to a potentially less-aligned type.
+/// **What it does:** Checks for transmutes from a pointer to a pointer, or
+/// from a reference to a reference.
 ///
-/// **Why is this bad?** This might result in undefined behavior.
+/// **Why is this bad?** Transmutes are dangerous, and these can instead be
+/// written as casts.
 ///
 /// **Known problems:** None.
 ///
 /// **Example:**
 /// ```rust
-/// // u32 is 32-bit aligned; u8 is 8-bit aligned
-/// let _: u32 = unsafe { std::mem::transmute([0u8; 4]) };
+/// let ptr = &1u32 as *const u32;
+/// unsafe {
+///     // pointer-to-pointer transmute
+///     let _: *const f32 = std::mem::transmute(ptr);
+///     // ref-ref transmute
+///     let _: &f32 = std::mem::transmute(&1u32);
+/// }
+/// // These can be respectively written:
+/// let _ = ptr as *const f32
+/// let _ = unsafe{ &*(&1u32 as *const u32 as *const f32) };
 /// ```
 declare_clippy_lint! {
-    pub MISALIGNED_TRANSMUTE,
+    pub TRANSMUTE_PTR_TO_PTR,
     complexity,
-    "transmutes to a potentially less-aligned type"
+    "transmutes from a pointer to a reference type"
 }
 
 pub struct Transmute;
@@ -193,13 +202,13 @@ impl LintPass for Transmute {
         lint_array!(
             CROSSPOINTER_TRANSMUTE,
             TRANSMUTE_PTR_TO_REF,
+            TRANSMUTE_PTR_TO_PTR,
             USELESS_TRANSMUTE,
             WRONG_TRANSMUTE,
             TRANSMUTE_INT_TO_CHAR,
             TRANSMUTE_BYTES_TO_STR,
             TRANSMUTE_INT_TO_BOOL,
             TRANSMUTE_INT_TO_FLOAT,
-            MISALIGNED_TRANSMUTE
         )
     }
 }
@@ -220,18 +229,6 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Transmute {
                                 e.span,
                                 &format!("transmute from a type (`{}`) to itself", from_ty),
                             ),
-                            _ if cx.layout_of(from_ty).ok().map(|a| a.align.abi())
-                                < cx.layout_of(to_ty).ok().map(|a| a.align.abi())
-                                => span_lint(
-                                    cx,
-                                    MISALIGNED_TRANSMUTE,
-                                    e.span,
-                                    &format!(
-                                        "transmute from `{}` to a less-aligned type (`{}`)",
-                                        from_ty,
-                                        to_ty,
-                                    )
-                            ),
                             (&ty::TyRef(_, rty), &ty::TyRawPtr(ptr_ty)) => span_lint_and_then(
                                 cx,
                                 USELESS_TRANSMUTE,
@@ -363,9 +360,35 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Transmute {
                                                 );
                                             }
                                         )
+                                    } else {
+                                        span_lint_and_then(
+                                            cx,
+                                            TRANSMUTE_PTR_TO_PTR,
+                                            e.span,
+                                            "transmute from a reference to a reference",
+                                            |db| if let Some(arg) = sugg::Sugg::hir_opt(cx, &args[0]) {
+                                                let sugg_paren = arg.as_ty(cx.tcx.mk_ptr(*ref_from)).as_ty(cx.tcx.mk_ptr(*ref_to));
+                                                let sugg = if ref_to.mutbl == Mutability::MutMutable {
+                                                    sugg_paren.mut_addr_deref()
+                                                } else {
+                                                    sugg_paren.addr_deref()
+                                                };
+                                                db.span_suggestion(e.span, "try", sugg.to_string());
+                                            },
+                                        )
                                     }
                                 }
                             },
+                            (&ty::TyRawPtr(_), &ty::TyRawPtr(to_ty)) => span_lint_and_then(
+                                cx,
+                                TRANSMUTE_PTR_TO_PTR,
+                                e.span,
+                                "transmute from a pointer to a pointer",
+                                |db| if let Some(arg) = sugg::Sugg::hir_opt(cx, &args[0]) {
+                                    let sugg = arg.as_ty(cx.tcx.mk_ptr(to_ty));
+                                    db.span_suggestion(e.span, "try", sugg.to_string());
+                                },
+                            ),
                             (&ty::TyInt(ast::IntTy::I8), &ty::TyBool) | (&ty::TyUint(ast::UintTy::U8), &ty::TyBool) => {
                                 span_lint_and_then(
                                     cx,

clippy_lints/src/types.rs

@@ -679,6 +679,25 @@ declare_clippy_lint! {
     "cast to the same type, e.g. `x as i32` where `x: i32`"
 }
 
+/// **What it does:** Checks for casts from a more-strictly-aligned pointer to a
+/// less-strictly-aligned pointer
+///
+/// **Why is this bad?** Dereferencing the resulting pointer is undefined
+/// behavior.
+///
+/// **Known problems:** None.
+///
+/// **Example:**
+/// ```rust
+/// let _ = (&1u8 as *const u8) as *const u16;
+/// let _ = (&mut 1u8 as *mut u8) as *mut u16;
+/// ```
+declare_clippy_lint! {
+    pub CAST_PTR_ALIGNMENT,
+    correctness,
+    "cast from a pointer to a less-strictly-aligned pointer"
+}
+
 /// Returns the size in bits of an integral type.
 /// Will return 0 if the type is not an int or uint variant
 fn int_ty_to_nbits(typ: Ty, tcx: TyCtxt) -> u64 {
@@ -871,7 +890,8 @@ impl LintPass for CastPass {
             CAST_POSSIBLE_TRUNCATION,
             CAST_POSSIBLE_WRAP,
             CAST_LOSSLESS,
-            UNNECESSARY_CAST
+            UNNECESSARY_CAST,
+            CAST_PTR_ALIGNMENT
         )
     }
 }
@@ -955,6 +975,21 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for CastPass {
                     },
                 }
             }
+            if_chain!{
+                if let ty::TyRawPtr(from_ptr_ty) = &cast_from.sty;
+                if let ty::TyRawPtr(to_ptr_ty) = &cast_to.sty;
+                if let Some(from_align) = cx.layout_of(from_ptr_ty.ty).ok().map(|a| a.align.abi());
+                if let Some(to_align) = cx.layout_of(to_ptr_ty.ty).ok().map(|a| a.align.abi());
+                if from_align < to_align;
+                then {
+                    span_lint(
+                        cx,
+                        CAST_PTR_ALIGNMENT,
+                        expr.span,
+                        &format!("casting from `{}` to a less-strictly-aligned pointer (`{}`)", cast_from, cast_to)
+                    );
+                }
+            }
         }
     }
 }

clippy_lints/src/utils/sugg.rs

@@ -160,6 +160,20 @@ impl<'a> Sugg<'a> {
         make_unop("*", self)
     }
 
+    /// Convenience method to create the `&*<expr>` suggestion. Currently this
+    /// is needed because `sugg.deref().addr()` produces an unnecessary set of
+    /// parentheses around the deref.
+    pub fn addr_deref(self) -> Sugg<'static> {
+        make_unop("&*", self)
+    }
+
+    /// Convenience method to create the `&mut *<expr>` suggestion. Currently
+    /// this is needed because `sugg.deref().mut_addr()` produces an unnecessary
+    /// set of parentheses around the deref.
+    pub fn mut_addr_deref(self) -> Sugg<'static> {
+        make_unop("&mut *", self)
+    }
+
     /// Convenience method to create the `<lhs>..<rhs>` or `<lhs>...<rhs>`
     /// suggestion.
     pub fn range(self, end: &Self, limit: ast::RangeLimits) -> Sugg<'static> {

tests/ui/cast_alignment.rs

@@ -0,0 +1,19 @@
+//! Test casts for alignment issues
+
+#[warn(cast_ptr_alignment)]
+#[allow(no_effect, unnecessary_operation, cast_lossless)]
+fn main() {
+    /* These should be warned against */
+
+    // cast to more-strictly-aligned type
+    (&1u8 as *const u8) as *const u16;
+    (&mut 1u8 as *mut u8) as *mut u16;
+
+    /* These should be okay */
+
+    // not a pointer type
+    1u8 as u16;
+    // cast to less-strictly-aligned type
+    (&1u16 as *const u16) as *const u8;
+    (&mut 1u16 as *mut u16) as *mut u8;
+}

tests/ui/cast_alignment.stderr

@@ -0,0 +1,16 @@
+error: casting from `*const u8` to a less-strictly-aligned pointer (`*const u16`)
+ --> $DIR/cast_alignment.rs:9:5
+  |
+9 |     (&1u8 as *const u8) as *const u16;
+  |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+  |
+  = note: `-D cast-ptr-alignment` implied by `-D warnings`
+
+error: casting from `*mut u8` to a less-strictly-aligned pointer (`*mut u16`)
+  --> $DIR/cast_alignment.rs:10:5
+   |
+10 |     (&mut 1u8 as *mut u8) as *mut u16;
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: aborting due to 2 previous errors
+

tests/ui/deprecated.rs

@@ -9,4 +9,6 @@
 
 #[warn(unstable_as_mut_slice)]
 
+#[warn(misaligned_transmute)]
+
 fn main() {}

tests/ui/deprecated.stderr

@@ -24,5 +24,11 @@ error: lint unstable_as_mut_slice has been removed: `Vec::as_mut_slice` has been
 10 | #[warn(unstable_as_mut_slice)]
    |        ^^^^^^^^^^^^^^^^^^^^^
 
-error: aborting due to 4 previous errors
+error: lint misaligned_transmute has been removed: this lint has been split into cast_ptr_alignment and transmute_ptr_to_ptr
+  --> $DIR/deprecated.rs:12:8
+   |
+12 | #[warn(misaligned_transmute)]
+   |        ^^^^^^^^^^^^^^^^^^^^
+
+error: aborting due to 5 previous errors
 

tests/ui/transmute.rs

@@ -16,7 +16,7 @@ fn my_vec() -> MyVec<i32> {
     vec![]
 }
 
-#[allow(needless_lifetimes)]
+#[allow(needless_lifetimes, transmute_ptr_to_ptr)]
 #[warn(useless_transmute)]
 unsafe fn _generic<'a, T, U: 'a>(t: &'a T) {
     let _: &'a T = core::intrinsics::transmute(t);
@@ -140,11 +140,23 @@ fn bytes_to_str(b: &[u8], mb: &mut [u8]) {
     let _: &mut str = unsafe { std::mem::transmute(mb) };
 }
 
-#[warn(misaligned_transmute)]
-fn misaligned_transmute() {
-    let _: u32 = unsafe { std::mem::transmute([0u8; 4]) }; // err
-    let _: u32 = unsafe { std::mem::transmute(0f32) }; // ok (alignment-wise)
-    let _: [u8; 4] = unsafe { std::mem::transmute(0u32) }; // ok (alignment-wise)
+#[warn(transmute_ptr_to_ptr)]
+fn transmute_ptr_to_ptr() {
+    let ptr = &1u32 as *const u32;
+    let mut_ptr = &mut 1u32 as *mut u32;
+    unsafe {
+        // pointer-to-pointer transmutes; bad
+        let _: *const f32 = std::mem::transmute(ptr);
+        let _: *mut f32 = std::mem::transmute(mut_ptr);
+        // ref-ref transmutes; bad
+        let _: &f32 = std::mem::transmute(&1u32);
+        let _: &mut f32 = std::mem::transmute(&mut 1u32);
+    }
+    // These should be fine
+    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) };
 }
 
 fn main() { }

tests/ui/transmute.stderr

@@ -204,13 +204,31 @@ error: transmute from a `&mut [u8]` to a `&mut str`
 140 |     let _: &mut str = unsafe { std::mem::transmute(mb) };
     |                                ^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `std::str::from_utf8_mut(mb).unwrap()`
 
-error: transmute from `[u8; 4]` to a less-aligned type (`u32`)
-   --> $DIR/transmute.rs:145:27
+error: transmute from a pointer to a pointer
+   --> $DIR/transmute.rs:149:29
     |
-145 |     let _: u32 = unsafe { std::mem::transmute([0u8; 4]) }; // err
-    |                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+149 |         let _: *const f32 = std::mem::transmute(ptr);
+    |                             ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `ptr as *const f32`
     |
-    = note: `-D misaligned-transmute` implied by `-D warnings`
+    = note: `-D transmute-ptr-to-ptr` implied by `-D warnings`
 
-error: aborting due to 33 previous errors
+error: transmute from a pointer to a pointer
+   --> $DIR/transmute.rs:150:27
+    |
+150 |         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:152:23
+    |
+152 |         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:153:27
+    |
+153 |         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