Auto merge of rust-lang#5891 - flip1995:rustup, r=flip1995

Rustup

r? @ghost

Sync back rust-lang#75098

changelog: none

Author: bors

CHANGELOG.md

@@ -1735,6 +1735,7 @@ Released 2018-09-13
 [`transmute_int_to_float`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmute_int_to_float
 [`transmute_ptr_to_ptr`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmute_ptr_to_ptr
 [`transmute_ptr_to_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmute_ptr_to_ref
+[`transmutes_expressible_as_ptr_casts`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmutes_expressible_as_ptr_casts
 [`transmuting_null`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmuting_null
 [`trivial_regex`]: https://rust-lang.github.io/rust-clippy/master/index.html#trivial_regex
 [`trivially_copy_pass_by_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#trivially_copy_pass_by_ref

clippy_lints/src/lib.rs

@@ -791,6 +791,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         &trait_bounds::TRAIT_DUPLICATION_IN_BOUNDS,
         &trait_bounds::TYPE_REPETITION_IN_BOUNDS,
         &transmute::CROSSPOINTER_TRANSMUTE,
+        &transmute::TRANSMUTES_EXPRESSIBLE_AS_PTR_CASTS,
         &transmute::TRANSMUTE_BYTES_TO_STR,
         &transmute::TRANSMUTE_FLOAT_TO_INT,
         &transmute::TRANSMUTE_INT_TO_BOOL,
@@ -1427,6 +1428,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&temporary_assignment::TEMPORARY_ASSIGNMENT),
         LintId::of(&to_digit_is_some::TO_DIGIT_IS_SOME),
         LintId::of(&transmute::CROSSPOINTER_TRANSMUTE),
+        LintId::of(&transmute::TRANSMUTES_EXPRESSIBLE_AS_PTR_CASTS),
         LintId::of(&transmute::TRANSMUTE_BYTES_TO_STR),
         LintId::of(&transmute::TRANSMUTE_FLOAT_TO_INT),
         LintId::of(&transmute::TRANSMUTE_INT_TO_BOOL),
@@ -1629,6 +1631,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&swap::MANUAL_SWAP),
         LintId::of(&temporary_assignment::TEMPORARY_ASSIGNMENT),
         LintId::of(&transmute::CROSSPOINTER_TRANSMUTE),
+        LintId::of(&transmute::TRANSMUTES_EXPRESSIBLE_AS_PTR_CASTS),
         LintId::of(&transmute::TRANSMUTE_BYTES_TO_STR),
         LintId::of(&transmute::TRANSMUTE_FLOAT_TO_INT),
         LintId::of(&transmute::TRANSMUTE_INT_TO_BOOL),

clippy_lints/src/transmute.rs

@@ -7,8 +7,10 @@ use rustc_ast::ast;
 use rustc_errors::Applicability;
 use rustc_hir::{Expr, ExprKind, GenericArg, Mutability, QPath, TyKind, UnOp};
 use rustc_lint::{LateContext, LateLintPass};
-use rustc_middle::ty::{self, Ty};
+use rustc_middle::ty::{self, cast::CastKind, Ty};
 use rustc_session::{declare_lint_pass, declare_tool_lint};
+use rustc_span::DUMMY_SP;
+use rustc_typeck::check::{cast::CastCheck, FnCtxt, Inherited};
 use std::borrow::Cow;
 
 declare_clippy_lint! {
@@ -48,6 +50,31 @@ declare_clippy_lint! {
     "transmutes that have the same to and from types or could be a cast/coercion"
 }
 
+// FIXME: Merge this lint with USELESS_TRANSMUTE once that is out of the nursery.
+declare_clippy_lint! {
+    /// **What it does:**Checks for transmutes that could be a pointer cast.
+    ///
+    /// **Why is this bad?** Readability. The code tricks people into thinking that
+    /// something complex is going on.
+    ///
+    /// **Known problems:** None.
+    ///
+    /// **Example:**
+    ///
+    /// ```rust
+    /// # let p: *const [i32] = &[];
+    /// unsafe { std::mem::transmute::<*const [i32], *const [u16]>(p) };
+    /// ```
+    /// Use instead:
+    /// ```rust
+    /// # let p: *const [i32] = &[];
+    /// p as *const [u16];
+    /// ```
+    pub TRANSMUTES_EXPRESSIBLE_AS_PTR_CASTS,
+    complexity,
+    "transmutes that could be a pointer cast"
+}
+
 declare_clippy_lint! {
     /// **What it does:** Checks for transmutes between a type `T` and `*T`.
     ///
@@ -269,6 +296,7 @@ declare_clippy_lint! {
     correctness,
     "transmute between collections of layout-incompatible types"
 }
+
 declare_lint_pass!(Transmute => [
     CROSSPOINTER_TRANSMUTE,
     TRANSMUTE_PTR_TO_REF,
@@ -281,6 +309,7 @@ declare_lint_pass!(Transmute => [
     TRANSMUTE_INT_TO_FLOAT,
     TRANSMUTE_FLOAT_TO_INT,
     UNSOUND_COLLECTION_TRANSMUTE,
+    TRANSMUTES_EXPRESSIBLE_AS_PTR_CASTS,
 ]);
 
 // used to check for UNSOUND_COLLECTION_TRANSMUTE
@@ -601,7 +630,25 @@ impl<'tcx> LateLintPass<'tcx> for Transmute {
                             );
                         }
                     },
-                    _ => return,
+                    (_, _) if can_be_expressed_as_pointer_cast(cx, e, from_ty, to_ty) => span_lint_and_then(
+                        cx,
+                        TRANSMUTES_EXPRESSIBLE_AS_PTR_CASTS,
+                        e.span,
+                        &format!(
+                            "transmute from `{}` to `{}` which could be expressed as a pointer cast instead",
+                            from_ty,
+                            to_ty
+                        ),
+                        |diag| {
+                            if let Some(arg) = sugg::Sugg::hir_opt(cx, &args[0]) {
+                                let sugg = arg.as_ty(&to_ty.to_string()).to_string();
+                                diag.span_suggestion(e.span, "try", sugg, Applicability::MachineApplicable);
+                            }
+                        }
+                    ),
+                    _ => {
+                        return;
+                    },
                 }
             }
         }
@@ -648,3 +695,59 @@ fn is_layout_incompatible<'tcx>(cx: &LateContext<'tcx>, from: Ty<'tcx>, to: Ty<'
         false
     }
 }
+
+/// Check if the type conversion can be expressed as a pointer cast, instead of
+/// a transmute. In certain cases, including some invalid casts from array
+/// references to pointers, this may cause additional errors to be emitted and/or
+/// ICE error messages. This function will panic if that occurs.
+fn can_be_expressed_as_pointer_cast<'tcx>(
+    cx: &LateContext<'tcx>,
+    e: &'tcx Expr<'_>,
+    from_ty: Ty<'tcx>,
+    to_ty: Ty<'tcx>,
+) -> bool {
+    use CastKind::{AddrPtrCast, ArrayPtrCast, FnPtrAddrCast, FnPtrPtrCast, PtrAddrCast, PtrPtrCast};
+    matches!(
+        check_cast(cx, e, from_ty, to_ty),
+        Some(PtrPtrCast | PtrAddrCast | AddrPtrCast | ArrayPtrCast | FnPtrPtrCast | FnPtrAddrCast)
+    )
+}
+
+/// If a cast from `from_ty` to `to_ty` is valid, returns an Ok containing the kind of
+/// the cast. In certain cases, including some invalid casts from array references
+/// to pointers, this may cause additional errors to be emitted and/or ICE error
+/// messages. This function will panic if that occurs.
+fn check_cast<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>, from_ty: Ty<'tcx>, to_ty: Ty<'tcx>) -> Option<CastKind> {
+    let hir_id = e.hir_id;
+    let local_def_id = hir_id.owner;
+
+    Inherited::build(cx.tcx, local_def_id).enter(|inherited| {
+        let fn_ctxt = FnCtxt::new(&inherited, cx.param_env, hir_id);
+
+        // If we already have errors, we can't be sure we can pointer cast.
+        assert!(
+            !fn_ctxt.errors_reported_since_creation(),
+            "Newly created FnCtxt contained errors"
+        );
+
+        if let Ok(check) = CastCheck::new(
+            &fn_ctxt, e, from_ty, to_ty,
+            // We won't show any error to the user, so we don't care what the span is here.
+            DUMMY_SP, DUMMY_SP,
+        ) {
+            let res = check.do_check(&fn_ctxt);
+
+            // do_check's documentation says that it might return Ok and create
+            // errors in the fcx instead of returing Err in some cases. Those cases
+            // should be filtered out before getting here.
+            assert!(
+                !fn_ctxt.errors_reported_since_creation(),
+                "`fn_ctxt` contained errors after cast check!"
+            );
+
+            res.ok()
+        } else {
+            None
+        }
+    })
+}

src/lintlist/mod.rs

@@ -2250,6 +2250,13 @@ pub static ref ALL_LINTS: Vec<Lint> = vec![
         deprecation: None,
         module: "transmute",
     },
+    Lint {
+        name: "transmutes_expressible_as_ptr_casts",
+        group: "complexity",
+        desc: "transmutes that could be a pointer cast",
+        deprecation: None,
+        module: "transmute",
+    },
     Lint {
         name: "transmuting_null",
         group: "correctness",

tests/compile-test.rs

@@ -216,6 +216,10 @@ fn run_ui_cargo(config: &mut compiletest::Config) {
         Ok(result)
     }
 
+    if cargo::is_rustc_test_suite() {
+        return;
+    }
+
     config.mode = TestMode::Ui;
     config.src_base = Path::new("tests").join("ui-cargo").canonicalize().unwrap();
 

tests/ui/transmutes_expressible_as_ptr_casts.fixed

@@ -0,0 +1,78 @@
+// run-rustfix
+#![warn(clippy::transmutes_expressible_as_ptr_casts)]
+// These two warnings currrently cover the cases transmutes_expressible_as_ptr_casts
+// would otherwise be responsible for
+#![warn(clippy::useless_transmute)]
+#![warn(clippy::transmute_ptr_to_ptr)]
+#![allow(unused_unsafe)]
+#![allow(dead_code)]
+
+use std::mem::{size_of, transmute};
+
+// rustc_typeck::check::cast contains documentation about when a cast `e as U` is
+// valid, which we quote from below.
+fn main() {
+    // We should see an error message for each transmute, and no error messages for
+    // the casts, since the casts are the recommended fixes.
+
+    // e is an integer and U is *U_0, while U_0: Sized; addr-ptr-cast
+    let _ptr_i32_transmute = unsafe { usize::MAX as *const i32 };
+    let ptr_i32 = usize::MAX as *const i32;
+
+    // e has type *T, U is *U_0, and either U_0: Sized ...
+    let _ptr_i8_transmute = unsafe { ptr_i32 as *const i8 };
+    let _ptr_i8 = ptr_i32 as *const i8;
+
+    let slice_ptr = &[0, 1, 2, 3] as *const [i32];
+
+    // ... or pointer_kind(T) = pointer_kind(U_0); ptr-ptr-cast
+    let _ptr_to_unsized_transmute = unsafe { slice_ptr as *const [u16] };
+    let _ptr_to_unsized = slice_ptr as *const [u16];
+    // TODO: We could try testing vtable casts here too, but maybe
+    // we should wait until std::raw::TraitObject is stabilized?
+
+    // e has type *T and U is a numeric type, while T: Sized; ptr-addr-cast
+    let _usize_from_int_ptr_transmute = unsafe { ptr_i32 as usize };
+    let _usize_from_int_ptr = ptr_i32 as usize;
+
+    let array_ref: &[i32; 4] = &[1, 2, 3, 4];
+
+    // e has type &[T; n] and U is *const T; array-ptr-cast
+    let _array_ptr_transmute = unsafe { array_ref as *const [i32; 4] };
+    let _array_ptr = array_ref as *const [i32; 4];
+
+    fn foo(_: usize) -> u8 {
+        42
+    }
+
+    // e is a function pointer type and U has type *T, while T: Sized; fptr-ptr-cast
+    let _usize_ptr_transmute = unsafe { foo as *const usize };
+    let _usize_ptr_transmute = foo as *const usize;
+
+    // e is a function pointer type and U is an integer; fptr-addr-cast
+    let _usize_from_fn_ptr_transmute = unsafe { foo as usize };
+    let _usize_from_fn_ptr = foo as *const usize;
+}
+
+// If a ref-to-ptr cast of this form where the pointer type points to a type other
+// than the referenced type, calling `CastCheck::do_check` has been observed to
+// cause an ICE error message. `do_check` is currently called inside the
+// `transmutes_expressible_as_ptr_casts` check, but other, more specific lints
+// currently prevent it from being called in these cases. This test is meant to
+// fail if the ordering of the checks ever changes enough to cause these cases to
+// fall through into `do_check`.
+fn trigger_do_check_to_emit_error(in_param: &[i32; 1]) -> *const u8 {
+    unsafe { in_param as *const [i32; 1] as *const u8 }
+}
+
+#[repr(C)]
+struct Single(u64);
+
+#[repr(C)]
+struct Pair(u32, u32);
+
+fn cannot_be_expressed_as_pointer_cast(in_param: Single) -> Pair {
+    assert_eq!(size_of::<Single>(), size_of::<Pair>());
+
+    unsafe { transmute::<Single, Pair>(in_param) }
+}

tests/ui/transmutes_expressible_as_ptr_casts.rs

@@ -0,0 +1,78 @@
+// run-rustfix
+#![warn(clippy::transmutes_expressible_as_ptr_casts)]
+// These two warnings currrently cover the cases transmutes_expressible_as_ptr_casts
+// would otherwise be responsible for
+#![warn(clippy::useless_transmute)]
+#![warn(clippy::transmute_ptr_to_ptr)]
+#![allow(unused_unsafe)]
+#![allow(dead_code)]
+
+use std::mem::{size_of, transmute};
+
+// rustc_typeck::check::cast contains documentation about when a cast `e as U` is
+// valid, which we quote from below.
+fn main() {
+    // We should see an error message for each transmute, and no error messages for
+    // the casts, since the casts are the recommended fixes.
+
+    // e is an integer and U is *U_0, while U_0: Sized; addr-ptr-cast
+    let _ptr_i32_transmute = unsafe { transmute::<usize, *const i32>(usize::MAX) };
+    let ptr_i32 = usize::MAX as *const i32;
+
+    // e has type *T, U is *U_0, and either U_0: Sized ...
+    let _ptr_i8_transmute = unsafe { transmute::<*const i32, *const i8>(ptr_i32) };
+    let _ptr_i8 = ptr_i32 as *const i8;
+
+    let slice_ptr = &[0, 1, 2, 3] as *const [i32];
+
+    // ... or pointer_kind(T) = pointer_kind(U_0); ptr-ptr-cast
+    let _ptr_to_unsized_transmute = unsafe { transmute::<*const [i32], *const [u16]>(slice_ptr) };
+    let _ptr_to_unsized = slice_ptr as *const [u16];
+    // TODO: We could try testing vtable casts here too, but maybe
+    // we should wait until std::raw::TraitObject is stabilized?
+
+    // e has type *T and U is a numeric type, while T: Sized; ptr-addr-cast
+    let _usize_from_int_ptr_transmute = unsafe { transmute::<*const i32, usize>(ptr_i32) };
+    let _usize_from_int_ptr = ptr_i32 as usize;
+
+    let array_ref: &[i32; 4] = &[1, 2, 3, 4];
+
+    // e has type &[T; n] and U is *const T; array-ptr-cast
+    let _array_ptr_transmute = unsafe { transmute::<&[i32; 4], *const [i32; 4]>(array_ref) };
+    let _array_ptr = array_ref as *const [i32; 4];
+
+    fn foo(_: usize) -> u8 {
+        42
+    }
+
+    // e is a function pointer type and U has type *T, while T: Sized; fptr-ptr-cast
+    let _usize_ptr_transmute = unsafe { transmute::<fn(usize) -> u8, *const usize>(foo) };
+    let _usize_ptr_transmute = foo as *const usize;
+
+    // e is a function pointer type and U is an integer; fptr-addr-cast
+    let _usize_from_fn_ptr_transmute = unsafe { transmute::<fn(usize) -> u8, usize>(foo) };
+    let _usize_from_fn_ptr = foo as *const usize;
+}
+
+// If a ref-to-ptr cast of this form where the pointer type points to a type other
+// than the referenced type, calling `CastCheck::do_check` has been observed to
+// cause an ICE error message. `do_check` is currently called inside the
+// `transmutes_expressible_as_ptr_casts` check, but other, more specific lints
+// currently prevent it from being called in these cases. This test is meant to
+// fail if the ordering of the checks ever changes enough to cause these cases to
+// fall through into `do_check`.
+fn trigger_do_check_to_emit_error(in_param: &[i32; 1]) -> *const u8 {
+    unsafe { transmute::<&[i32; 1], *const u8>(in_param) }
+}
+
+#[repr(C)]
+struct Single(u64);
+
+#[repr(C)]
+struct Pair(u32, u32);
+
+fn cannot_be_expressed_as_pointer_cast(in_param: Single) -> Pair {
+    assert_eq!(size_of::<Single>(), size_of::<Pair>());
+
+    unsafe { transmute::<Single, Pair>(in_param) }
+}