From ef2916170b62f99411a787e1a1985f45c221ebef Mon Sep 17 00:00:00 2001 From: Ryan1729 Date: Sun, 2 Aug 2020 18:19:33 -0600 Subject: [PATCH 01/37] make parts of rustc_typeck public --- src/librustc_typeck/check/cast.rs | 4 ++-- src/librustc_typeck/check/mod.rs | 2 +- src/librustc_typeck/lib.rs | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/librustc_typeck/check/cast.rs b/src/librustc_typeck/check/cast.rs index a877df68326d..418ea29b84f2 100644 --- a/src/librustc_typeck/check/cast.rs +++ b/src/librustc_typeck/check/cast.rs @@ -147,7 +147,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } #[derive(Copy, Clone)] -enum CastError { +pub enum CastError { ErrorReported, CastToBool, @@ -593,7 +593,7 @@ impl<'a, 'tcx> CastCheck<'tcx> { /// Checks a cast, and report an error if one exists. In some cases, this /// can return Ok and create type errors in the fcx rather than returning /// directly. coercion-cast is handled in check instead of here. - fn do_check(&self, fcx: &FnCtxt<'a, 'tcx>) -> Result { + pub fn do_check(&self, fcx: &FnCtxt<'a, 'tcx>) -> Result { use rustc_middle::ty::cast::CastTy::*; use rustc_middle::ty::cast::IntTy::*; diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index 6cefc99f7b17..3af571417ff5 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -67,7 +67,7 @@ type parameter). pub mod _match; mod autoderef; mod callee; -mod cast; +pub mod cast; mod closure; pub mod coercion; mod compare_method; diff --git a/src/librustc_typeck/lib.rs b/src/librustc_typeck/lib.rs index 9ba2545ba63c..4eee4f572c2b 100644 --- a/src/librustc_typeck/lib.rs +++ b/src/librustc_typeck/lib.rs @@ -74,11 +74,11 @@ extern crate log; #[macro_use] extern crate rustc_middle; -// This is used by Clippy. +// These are used by Clippy. pub mod expr_use_visitor; +pub mod check; mod astconv; -mod check; mod check_unused; mod coherence; mod collect; From 5565c1a1ca5e9a8fc941a55bdee4a774774e5be9 Mon Sep 17 00:00:00 2001 From: Ryan1729 Date: Sun, 2 Aug 2020 18:41:50 -0600 Subject: [PATCH 02/37] run cargo dev new_lint then move transmutes_expressible_as_ptr_casts into transmute module --- src/tools/clippy/CHANGELOG.md | 1 + src/tools/clippy/clippy_lints/src/lib.rs | 3 +++ .../clippy/clippy_lints/src/transmute.rs | 23 +++++++++++++++++++ src/tools/clippy/src/lintlist/mod.rs | 7 ++++++ .../ui/transmutes_expressible_as_ptr_casts.rs | 5 ++++ 5 files changed, 39 insertions(+) create mode 100644 src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.rs diff --git a/src/tools/clippy/CHANGELOG.md b/src/tools/clippy/CHANGELOG.md index 776b04295f94..a6e694910177 100644 --- a/src/tools/clippy/CHANGELOG.md +++ b/src/tools/clippy/CHANGELOG.md @@ -1730,6 +1730,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 diff --git a/src/tools/clippy/clippy_lints/src/lib.rs b/src/tools/clippy/clippy_lints/src/lib.rs index f371942dbeec..87b5309ff1c7 100644 --- a/src/tools/clippy/clippy_lints/src/lib.rs +++ b/src/tools/clippy/clippy_lints/src/lib.rs @@ -798,6 +798,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: &transmute::UNSOUND_COLLECTION_TRANSMUTE, &transmute::USELESS_TRANSMUTE, &transmute::WRONG_TRANSMUTE, + &transmute::TRANSMUTES_EXPRESSIBLE_AS_PTR_CASTS, &transmuting_null::TRANSMUTING_NULL, &trivially_copy_pass_by_ref::TRIVIALLY_COPY_PASS_BY_REF, &try_err::TRY_ERR, @@ -1426,6 +1427,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&transmute::TRANSMUTE_PTR_TO_REF), LintId::of(&transmute::UNSOUND_COLLECTION_TRANSMUTE), LintId::of(&transmute::WRONG_TRANSMUTE), + LintId::of(&transmute::TRANSMUTES_EXPRESSIBLE_AS_PTR_CASTS), LintId::of(&transmuting_null::TRANSMUTING_NULL), LintId::of(&try_err::TRY_ERR), LintId::of(&types::ABSURD_EXTREME_COMPARISONS), @@ -1624,6 +1626,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&transmute::TRANSMUTE_INT_TO_FLOAT), LintId::of(&transmute::TRANSMUTE_PTR_TO_PTR), LintId::of(&transmute::TRANSMUTE_PTR_TO_REF), + LintId::of(&transmute::TRANSMUTES_EXPRESSIBLE_AS_PTR_CASTS), LintId::of(&types::BORROWED_BOX), LintId::of(&types::CHAR_LIT_AS_U8), LintId::of(&types::TYPE_COMPLEXITY), diff --git a/src/tools/clippy/clippy_lints/src/transmute.rs b/src/tools/clippy/clippy_lints/src/transmute.rs index d55eb1a0c938..ce2ac24f5325 100644 --- a/src/tools/clippy/clippy_lints/src/transmute.rs +++ b/src/tools/clippy/clippy_lints/src/transmute.rs @@ -269,6 +269,28 @@ declare_clippy_lint! { correctness, "transmute between collections of layout-incompatible types" } + +declare_clippy_lint! { + /// **What it does:** + /// + /// **Why is this bad?** + /// + /// **Known problems:** None. + /// + /// **Example:** + /// + /// ```rust + /// // example code where clippy issues a warning + /// ``` + /// Use instead: + /// ```rust + /// // example code which does not raise clippy warning + /// ``` + pub TRANSMUTES_EXPRESSIBLE_AS_PTR_CASTS, + complexity, + "default lint description" +} + declare_lint_pass!(Transmute => [ CROSSPOINTER_TRANSMUTE, TRANSMUTE_PTR_TO_REF, @@ -281,6 +303,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 diff --git a/src/tools/clippy/src/lintlist/mod.rs b/src/tools/clippy/src/lintlist/mod.rs index 1879aae77fb6..9363039041da 100644 --- a/src/tools/clippy/src/lintlist/mod.rs +++ b/src/tools/clippy/src/lintlist/mod.rs @@ -2215,6 +2215,13 @@ pub static ref ALL_LINTS: Vec = vec![ deprecation: None, module: "transmute", }, + Lint { + name: "transmutes_expressible_as_ptr_casts", + group: "complexity", + desc: "default lint description", + deprecation: None, + module: "transmute", + }, Lint { name: "transmuting_null", group: "correctness", diff --git a/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.rs b/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.rs new file mode 100644 index 000000000000..2b32ae213fbb --- /dev/null +++ b/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.rs @@ -0,0 +1,5 @@ +#![warn(clippy::transmutes_expressible_as_ptr_casts)] + +fn main() { + // test code goes here +} From 0c37239e0e75a18315367ca77d63b86d444742bd Mon Sep 17 00:00:00 2001 From: Ryan1729 Date: Sun, 2 Aug 2020 21:30:25 -0600 Subject: [PATCH 03/37] make InheritedBuilder::enter public --- src/librustc_typeck/check/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index 3af571417ff5..1e016d47123c 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -648,7 +648,7 @@ impl Inherited<'_, 'tcx> { } impl<'tcx> InheritedBuilder<'tcx> { - fn enter(&mut self, f: F) -> R + pub fn enter(&mut self, f: F) -> R where F: for<'a> FnOnce(Inherited<'a, 'tcx>) -> R, { From b4e6e7047817ac12a0330060d2e176a40809c91b Mon Sep 17 00:00:00 2001 From: Ryan1729 Date: Sun, 2 Aug 2020 22:00:51 -0600 Subject: [PATCH 04/37] initial compiling version of TRANSMUTES_EXPRESSIBLE_AS_PTR_CASTS --- .../clippy/clippy_lints/src/transmute.rs | 74 ++++++++++++++++++- 1 file changed, 72 insertions(+), 2 deletions(-) diff --git a/src/tools/clippy/clippy_lints/src/transmute.rs b/src/tools/clippy/clippy_lints/src/transmute.rs index ce2ac24f5325..d5b694ce3118 100644 --- a/src/tools/clippy/clippy_lints/src/transmute.rs +++ b/src/tools/clippy/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_span::DUMMY_SP; use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_typeck::check::{cast::CastCheck, FnCtxt, Inherited}; use std::borrow::Cow; declare_clippy_lint! { @@ -624,7 +626,21 @@ impl<'tcx> LateLintPass<'tcx> for Transmute { ); } }, - _ => return, + (_, _) if can_be_expressed_as_pointer_cast(cx, e, from_ty, to_ty) => { + span_lint( + 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 + ) + ); + }, + _ => { + return + }, } } } @@ -671,3 +687,57 @@ fn is_layout_incompatible<'tcx>(cx: &LateContext<'tcx>, from: Ty<'tcx>, to: Ty<' false } } + +/// Check if the the type conversion can be expressed as a pointer cast, instead of a transmute. +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::*; + 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. +fn check_cast<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>, from_ty: Ty<'tcx>, to_ty: Ty<'tcx>) -> Option { + 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, + // TODO should we try to get the correct ParamEnv? + ty::ParamEnv::empty(), + hir_id + ); + + // If we already have errors, we can't be sure we can pointer cast. + if fn_ctxt.errors_reported_since_creation() { + return None; + } + + 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, + ) { + check.do_check(&fn_ctxt) + .ok() + // do_check's documentation says that it might return Ok and create + // errors in the fcx instead of returing Err in some cases. + .filter(|_| !fn_ctxt.errors_reported_since_creation()) + } else { + None + } + }) +} \ No newline at end of file From 7061b1c0933efc2321d9e30413a746610c8e5e8c Mon Sep 17 00:00:00 2001 From: Ryan1729 Date: Sun, 2 Aug 2020 23:17:11 -0600 Subject: [PATCH 05/37] write currently failing test for transmutes_expressible_as_ptr_casts There are 5 errors, when there should be 7. --- .../ui/transmutes_expressible_as_ptr_casts.rs | 54 ++++++++++++++++++- ...transmutes_expressible_as_ptr_casts.stderr | 1 + 2 files changed, 54 insertions(+), 1 deletion(-) create mode 100644 src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.stderr diff --git a/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.rs b/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.rs index 2b32ae213fbb..e6b9dddd3421 100644 --- a/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.rs +++ b/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.rs @@ -1,5 +1,57 @@ #![warn(clippy::transmutes_expressible_as_ptr_casts)] +// rustc_typeck::check::cast contains documentation about when a cast `e as U` is +// valid, which we quote from below. +use std::mem::transmute; + fn main() { - // test code goes here + // e is an integer and U is *U_0, while U_0: Sized; addr-ptr-cast + let ptr_i32_transmute = unsafe { + transmute::(-1) + }; + let ptr_i32 = -1isize 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:: 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:: u8, usize>(foo) + }; + let usize_from_fn_ptr = foo as *const usize; } diff --git a/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.stderr b/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.stderr new file mode 100644 index 000000000000..6bae1fa1b4fc --- /dev/null +++ b/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.stderr @@ -0,0 +1 @@ +Should have 7 errors, one for each transmute From b4ecee9edeca0f234c2074b929f5333bee4e76a5 Mon Sep 17 00:00:00 2001 From: Ryan1729 Date: Mon, 3 Aug 2020 00:16:11 -0600 Subject: [PATCH 06/37] accidentally cause an ICE by putting the TRANSMUTES_EXPRESSIBLE_AS_PTR_CASTS handling after the match The reason I did this in the first place was to try and figure out why I don't see my expected 7 error messages --- .../clippy/clippy_lints/src/transmute.rs | 34 +++++++++++-------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/src/tools/clippy/clippy_lints/src/transmute.rs b/src/tools/clippy/clippy_lints/src/transmute.rs index d5b694ce3118..d6b1a5df71fa 100644 --- a/src/tools/clippy/clippy_lints/src/transmute.rs +++ b/src/tools/clippy/clippy_lints/src/transmute.rs @@ -626,21 +626,25 @@ impl<'tcx> LateLintPass<'tcx> for Transmute { ); } }, - (_, _) if can_be_expressed_as_pointer_cast(cx, e, from_ty, to_ty) => { - span_lint( - 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 - ) - ); - }, - _ => { - 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 = format!("{} as {}", arg, to_ty); + diag.span_suggestion(e.span, "try", sugg, Applicability::Unspecified); + } + } + ) } } } From 1e5c14df58ddbaf203eb2b6a3d89edd6080f3dd7 Mon Sep 17 00:00:00 2001 From: Ryan1729 Date: Mon, 3 Aug 2020 00:54:03 -0600 Subject: [PATCH 07/37] try putting the can_be_expressed_as_pointer_cast at the top and find that we still get an ICE --- .../clippy/clippy_lints/src/transmute.rs | 42 ++++++++++--------- src/tools/clippy/tests/ui/transmute.rs | 2 +- .../clippy/tests/ui/transmute_ptr_to_ptr.rs | 2 +- 3 files changed, 25 insertions(+), 21 deletions(-) diff --git a/src/tools/clippy/clippy_lints/src/transmute.rs b/src/tools/clippy/clippy_lints/src/transmute.rs index d6b1a5df71fa..7ab3f0d96763 100644 --- a/src/tools/clippy/clippy_lints/src/transmute.rs +++ b/src/tools/clippy/clippy_lints/src/transmute.rs @@ -330,6 +330,26 @@ impl<'tcx> LateLintPass<'tcx> for Transmute { let from_ty = cx.typeck_results().expr_ty(&args[0]); let to_ty = cx.typeck_results().expr_ty(e); + 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 = format!("{} as {}", arg, to_ty); + diag.span_suggestion(e.span, "try", sugg, Applicability::Unspecified); + } + } + ); + return + } + match (&from_ty.kind, &to_ty.kind) { _ if from_ty == to_ty => span_lint( cx, @@ -626,25 +646,9 @@ impl<'tcx> LateLintPass<'tcx> for Transmute { ); } }, - _ => {}, - } - 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 = format!("{} as {}", arg, to_ty); - diag.span_suggestion(e.span, "try", sugg, Applicability::Unspecified); - } - } - ) + _ => { + return; + }, } } } diff --git a/src/tools/clippy/tests/ui/transmute.rs b/src/tools/clippy/tests/ui/transmute.rs index bb853d237047..b3171d2e7dcf 100644 --- a/src/tools/clippy/tests/ui/transmute.rs +++ b/src/tools/clippy/tests/ui/transmute.rs @@ -1,7 +1,7 @@ #![allow(dead_code)] +#![allow(clippy::transmutes_expressible_as_ptr_casts)] extern crate core; - use std::mem::transmute as my_transmute; use std::vec::Vec as MyVec; diff --git a/src/tools/clippy/tests/ui/transmute_ptr_to_ptr.rs b/src/tools/clippy/tests/ui/transmute_ptr_to_ptr.rs index 0d8a322f2b2b..009b5fa534cf 100644 --- a/src/tools/clippy/tests/ui/transmute_ptr_to_ptr.rs +++ b/src/tools/clippy/tests/ui/transmute_ptr_to_ptr.rs @@ -1,5 +1,5 @@ #![warn(clippy::transmute_ptr_to_ptr)] - +#![allow(clippy::transmutes_expressible_as_ptr_casts)] // Make sure we can modify lifetimes, which is one of the recommended uses // of transmute From 7a818c096c42df787468b9550a1814121a7b1080 Mon Sep 17 00:00:00 2001 From: Ryan1729 Date: Mon, 3 Aug 2020 02:47:25 -0600 Subject: [PATCH 08/37] get the expected number of errors by acknowledging that other lints are covering the same ground --- .../clippy/clippy_lints/src/transmute.rs | 80 +++++++++---------- src/tools/clippy/tests/ui/transmute.rs | 2 +- .../clippy/tests/ui/transmute_ptr_to_ptr.rs | 2 +- .../ui/transmutes_expressible_as_ptr_casts.rs | 10 ++- ...transmutes_expressible_as_ptr_casts.stderr | 51 +++++++++++- 5 files changed, 100 insertions(+), 45 deletions(-) diff --git a/src/tools/clippy/clippy_lints/src/transmute.rs b/src/tools/clippy/clippy_lints/src/transmute.rs index 7ab3f0d96763..269d2f00353a 100644 --- a/src/tools/clippy/clippy_lints/src/transmute.rs +++ b/src/tools/clippy/clippy_lints/src/transmute.rs @@ -50,6 +50,29 @@ 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,ignore + /// core::intrinsics::transmute::<*const [i32], *const [u16]>(p) + /// ``` + /// Use instead: + /// ```rust + /// 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`. /// @@ -272,27 +295,6 @@ declare_clippy_lint! { "transmute between collections of layout-incompatible types" } -declare_clippy_lint! { - /// **What it does:** - /// - /// **Why is this bad?** - /// - /// **Known problems:** None. - /// - /// **Example:** - /// - /// ```rust - /// // example code where clippy issues a warning - /// ``` - /// Use instead: - /// ```rust - /// // example code which does not raise clippy warning - /// ``` - pub TRANSMUTES_EXPRESSIBLE_AS_PTR_CASTS, - complexity, - "default lint description" -} - declare_lint_pass!(Transmute => [ CROSSPOINTER_TRANSMUTE, TRANSMUTE_PTR_TO_REF, @@ -330,26 +332,6 @@ impl<'tcx> LateLintPass<'tcx> for Transmute { let from_ty = cx.typeck_results().expr_ty(&args[0]); let to_ty = cx.typeck_results().expr_ty(e); - 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 = format!("{} as {}", arg, to_ty); - diag.span_suggestion(e.span, "try", sugg, Applicability::Unspecified); - } - } - ); - return - } - match (&from_ty.kind, &to_ty.kind) { _ if from_ty == to_ty => span_lint( cx, @@ -646,6 +628,22 @@ impl<'tcx> LateLintPass<'tcx> for Transmute { ); } }, + (_, _) 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::Unspecified); + } + } + ), _ => { return; }, diff --git a/src/tools/clippy/tests/ui/transmute.rs b/src/tools/clippy/tests/ui/transmute.rs index b3171d2e7dcf..bb853d237047 100644 --- a/src/tools/clippy/tests/ui/transmute.rs +++ b/src/tools/clippy/tests/ui/transmute.rs @@ -1,7 +1,7 @@ #![allow(dead_code)] -#![allow(clippy::transmutes_expressible_as_ptr_casts)] extern crate core; + use std::mem::transmute as my_transmute; use std::vec::Vec as MyVec; diff --git a/src/tools/clippy/tests/ui/transmute_ptr_to_ptr.rs b/src/tools/clippy/tests/ui/transmute_ptr_to_ptr.rs index 009b5fa534cf..0d8a322f2b2b 100644 --- a/src/tools/clippy/tests/ui/transmute_ptr_to_ptr.rs +++ b/src/tools/clippy/tests/ui/transmute_ptr_to_ptr.rs @@ -1,5 +1,5 @@ #![warn(clippy::transmute_ptr_to_ptr)] -#![allow(clippy::transmutes_expressible_as_ptr_casts)] + // Make sure we can modify lifetimes, which is one of the recommended uses // of transmute diff --git a/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.rs b/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.rs index e6b9dddd3421..db544b438a28 100644 --- a/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.rs +++ b/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.rs @@ -1,10 +1,18 @@ #![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)] + +use std::mem::transmute; // rustc_typeck::check::cast contains documentation about when a cast `e as U` is // valid, which we quote from below. -use std::mem::transmute; 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::(-1) diff --git a/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.stderr b/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.stderr index 6bae1fa1b4fc..7cd316bf38aa 100644 --- a/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.stderr +++ b/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.stderr @@ -1 +1,50 @@ -Should have 7 errors, one for each transmute +error: transmute from an integer to a pointer + --> $DIR/transmutes_expressible_as_ptr_casts.rs:18:9 + | +LL | transmute::(-1) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `-1 as *const i32` + | + = note: `-D clippy::useless-transmute` implied by `-D warnings` + +error: transmute from a pointer to a pointer + --> $DIR/transmutes_expressible_as_ptr_casts.rs:24:9 + | +LL | transmute::<*const i32, *const i8>(ptr_i32) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `ptr_i32 as *const i8` + | + = note: `-D clippy::transmute-ptr-to-ptr` implied by `-D warnings` + +error: transmute from a pointer to a pointer + --> $DIR/transmutes_expressible_as_ptr_casts.rs:32:9 + | +LL | transmute::<*const [i32], *const [u16]>(slice_ptr) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `slice_ptr as *const [u16]` + +error: transmute from `*const i32` to `usize` which could be expressed as a pointer cast instead + --> $DIR/transmutes_expressible_as_ptr_casts.rs:40:9 + | +LL | transmute::<*const i32, usize>(ptr_i32) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `ptr_i32 as usize` + | + = note: `-D clippy::transmutes-expressible-as-ptr-casts` implied by `-D warnings` + +error: transmute from a reference to a pointer + --> $DIR/transmutes_expressible_as_ptr_casts.rs:48:9 + | +LL | transmute::<&[i32; 4], *const [i32; 4]>(array_ref) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `array_ref as *const [i32; 4]` + +error: transmute from `fn(usize) -> u8 {main::foo}` to `*const usize` which could be expressed as a pointer cast instead + --> $DIR/transmutes_expressible_as_ptr_casts.rs:56:9 + | +LL | transmute:: u8, *const usize>(foo) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `foo as *const usize` + +error: transmute from `fn(usize) -> u8 {main::foo}` to `usize` which could be expressed as a pointer cast instead + --> $DIR/transmutes_expressible_as_ptr_casts.rs:62:9 + | +LL | transmute:: u8, usize>(foo) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `foo as usize` + +error: aborting due to 7 previous errors + From ab93133e83048f0b583e5ce3ff3794f4e1d425aa Mon Sep 17 00:00:00 2001 From: Ryan1729 Date: Mon, 3 Aug 2020 19:00:38 -0600 Subject: [PATCH 09/37] address some review comments --- src/tools/clippy/clippy_lints/src/transmute.rs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/tools/clippy/clippy_lints/src/transmute.rs b/src/tools/clippy/clippy_lints/src/transmute.rs index 269d2f00353a..b6747c73f705 100644 --- a/src/tools/clippy/clippy_lints/src/transmute.rs +++ b/src/tools/clippy/clippy_lints/src/transmute.rs @@ -718,15 +718,12 @@ fn check_cast<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>, from_ty: Ty<'tcx> Inherited::build(cx.tcx, local_def_id).enter(|inherited| { let fn_ctxt = FnCtxt::new( &inherited, - // TODO should we try to get the correct ParamEnv? - ty::ParamEnv::empty(), + cx.param_env, hir_id ); // If we already have errors, we can't be sure we can pointer cast. - if fn_ctxt.errors_reported_since_creation() { - return None; - } + assert!(!fn_ctxt.errors_reported_since_creation()); if let Ok(check) = CastCheck::new( &fn_ctxt, @@ -746,4 +743,4 @@ fn check_cast<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>, from_ty: Ty<'tcx> None } }) -} \ No newline at end of file +} From 6fdd1dbe033557cb64c8e0afb5cf675299990659 Mon Sep 17 00:00:00 2001 From: Ryan1729 Date: Tue, 4 Aug 2020 16:45:47 -0600 Subject: [PATCH 10/37] add description to assert --- src/tools/clippy/clippy_lints/src/transmute.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/tools/clippy/clippy_lints/src/transmute.rs b/src/tools/clippy/clippy_lints/src/transmute.rs index b6747c73f705..ea14f2829ea5 100644 --- a/src/tools/clippy/clippy_lints/src/transmute.rs +++ b/src/tools/clippy/clippy_lints/src/transmute.rs @@ -723,7 +723,10 @@ fn check_cast<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>, from_ty: Ty<'tcx> ); // If we already have errors, we can't be sure we can pointer cast. - assert!(!fn_ctxt.errors_reported_since_creation()); + assert!( + !fn_ctxt.errors_reported_since_creation(), + "Newly created FnCtxt contained errors" + ); if let Ok(check) = CastCheck::new( &fn_ctxt, From 8ba41017e7fa14707700f8469247501b7afaa437 Mon Sep 17 00:00:00 2001 From: Ryan1729 Date: Wed, 5 Aug 2020 20:23:29 -0600 Subject: [PATCH 11/37] add documentation to functions that call `do_check` and add a test against lint ordering changing --- src/tools/clippy/clippy_lints/src/transmute.rs | 10 ++++++++-- .../tests/ui/transmutes_expressible_as_ptr_casts.rs | 11 +++++++++++ 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/src/tools/clippy/clippy_lints/src/transmute.rs b/src/tools/clippy/clippy_lints/src/transmute.rs index ea14f2829ea5..231f13b236c1 100644 --- a/src/tools/clippy/clippy_lints/src/transmute.rs +++ b/src/tools/clippy/clippy_lints/src/transmute.rs @@ -694,7 +694,10 @@ fn is_layout_incompatible<'tcx>(cx: &LateContext<'tcx>, from: Ty<'tcx>, to: Ty<' } } -/// Check if the the type conversion can be expressed as a pointer cast, instead of a transmute. +/// Check if the 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. 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::*; matches!( @@ -710,7 +713,10 @@ fn can_be_expressed_as_pointer_cast<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr< ) } -/// If a cast from from_ty to to_ty is valid, returns an Ok containing the kind of the cast. +/// 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. fn check_cast<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>, from_ty: Ty<'tcx>, to_ty: Ty<'tcx>) -> Option { let hir_id = e.hir_id; let local_def_id = hir_id.owner; diff --git a/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.rs b/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.rs index db544b438a28..007526da40df 100644 --- a/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.rs +++ b/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.rs @@ -63,3 +63,14 @@ fn main() { }; 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) } +} \ No newline at end of file From afd4909d41cd0316ef2a01fe583b95eab9cb6475 Mon Sep 17 00:00:00 2001 From: Ryan1729 Date: Wed, 5 Aug 2020 21:28:22 -0600 Subject: [PATCH 12/37] add extra error message to the expected stderr for transmutes_expressible_as_ptr_casts test --- .../tests/ui/transmutes_expressible_as_ptr_casts.stderr | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.stderr b/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.stderr index 7cd316bf38aa..e6edacbd0de8 100644 --- a/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.stderr +++ b/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.stderr @@ -46,5 +46,11 @@ error: transmute from `fn(usize) -> u8 {main::foo}` to `usize` which could be ex LL | transmute:: u8, usize>(foo) | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `foo as usize` -error: aborting due to 7 previous errors +error: transmute from a reference to a pointer + --> $DIR/transmutes_expressible_as_ptr_casts.rs:75:14 + | +LL | unsafe { transmute::<&[i32; 1], *const u8>(in_param) } + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `in_param as *const [i32; 1] as *const u8` + +error: aborting due to 8 previous errors From 54472478fa29c2d121da469bc8f10f8b68e3e134 Mon Sep 17 00:00:00 2001 From: Ryan1729 Date: Thu, 6 Aug 2020 04:18:14 -0600 Subject: [PATCH 13/37] change filter to assert, and update comments --- .../clippy/clippy_lints/src/transmute.rs | 20 ++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/src/tools/clippy/clippy_lints/src/transmute.rs b/src/tools/clippy/clippy_lints/src/transmute.rs index 231f13b236c1..9f356811f220 100644 --- a/src/tools/clippy/clippy_lints/src/transmute.rs +++ b/src/tools/clippy/clippy_lints/src/transmute.rs @@ -697,7 +697,7 @@ fn is_layout_incompatible<'tcx>(cx: &LateContext<'tcx>, from: Ty<'tcx>, to: Ty<' /// Check if the 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. +/// 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::*; matches!( @@ -716,7 +716,7 @@ fn can_be_expressed_as_pointer_cast<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr< /// 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. +/// 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 { let hir_id = e.hir_id; let local_def_id = hir_id.owner; @@ -743,11 +743,17 @@ fn check_cast<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>, from_ty: Ty<'tcx> DUMMY_SP, DUMMY_SP, ) { - check.do_check(&fn_ctxt) - .ok() - // do_check's documentation says that it might return Ok and create - // errors in the fcx instead of returing Err in some cases. - .filter(|_| !fn_ctxt.errors_reported_since_creation()) + 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 } From e3170bb1ece69cf3a10551a34d7170722ccd3855 Mon Sep 17 00:00:00 2001 From: Ryan1729 Date: Thu, 6 Aug 2020 04:49:06 -0600 Subject: [PATCH 14/37] add newline to transmutes_expressible_as_ptr_casts.rs --- .../clippy/tests/ui/transmutes_expressible_as_ptr_casts.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.rs b/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.rs index 007526da40df..7e99c9283598 100644 --- a/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.rs +++ b/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.rs @@ -73,4 +73,4 @@ fn main() { // 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) } -} \ No newline at end of file +} From 4027f15dd653f8250da91d9bd8c6d20513399a34 Mon Sep 17 00:00:00 2001 From: Ryan1729 Date: Thu, 6 Aug 2020 06:08:32 -0600 Subject: [PATCH 15/37] run ./x.py fmt --- src/librustc_typeck/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc_typeck/lib.rs b/src/librustc_typeck/lib.rs index 4eee4f572c2b..e96fb67c7da1 100644 --- a/src/librustc_typeck/lib.rs +++ b/src/librustc_typeck/lib.rs @@ -75,8 +75,8 @@ extern crate log; extern crate rustc_middle; // These are used by Clippy. -pub mod expr_use_visitor; pub mod check; +pub mod expr_use_visitor; mod astconv; mod check_unused; From 007dc944da130158ba30a524356d680eb663b80f Mon Sep 17 00:00:00 2001 From: Ryan1729 Date: Thu, 6 Aug 2020 06:11:23 -0600 Subject: [PATCH 16/37] run clippy_dev update_lints --- src/tools/clippy/clippy_lints/src/lib.rs | 6 +++--- src/tools/clippy/src/lintlist/mod.rs | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/tools/clippy/clippy_lints/src/lib.rs b/src/tools/clippy/clippy_lints/src/lib.rs index 87b5309ff1c7..828ee9105960 100644 --- a/src/tools/clippy/clippy_lints/src/lib.rs +++ b/src/tools/clippy/clippy_lints/src/lib.rs @@ -788,6 +788,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: &to_digit_is_some::TO_DIGIT_IS_SOME, &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, @@ -798,7 +799,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: &transmute::UNSOUND_COLLECTION_TRANSMUTE, &transmute::USELESS_TRANSMUTE, &transmute::WRONG_TRANSMUTE, - &transmute::TRANSMUTES_EXPRESSIBLE_AS_PTR_CASTS, &transmuting_null::TRANSMUTING_NULL, &trivially_copy_pass_by_ref::TRIVIALLY_COPY_PASS_BY_REF, &try_err::TRY_ERR, @@ -1418,6 +1418,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), @@ -1427,7 +1428,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&transmute::TRANSMUTE_PTR_TO_REF), LintId::of(&transmute::UNSOUND_COLLECTION_TRANSMUTE), LintId::of(&transmute::WRONG_TRANSMUTE), - LintId::of(&transmute::TRANSMUTES_EXPRESSIBLE_AS_PTR_CASTS), LintId::of(&transmuting_null::TRANSMUTING_NULL), LintId::of(&try_err::TRY_ERR), LintId::of(&types::ABSURD_EXTREME_COMPARISONS), @@ -1619,6 +1619,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), @@ -1626,7 +1627,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&transmute::TRANSMUTE_INT_TO_FLOAT), LintId::of(&transmute::TRANSMUTE_PTR_TO_PTR), LintId::of(&transmute::TRANSMUTE_PTR_TO_REF), - LintId::of(&transmute::TRANSMUTES_EXPRESSIBLE_AS_PTR_CASTS), LintId::of(&types::BORROWED_BOX), LintId::of(&types::CHAR_LIT_AS_U8), LintId::of(&types::TYPE_COMPLEXITY), diff --git a/src/tools/clippy/src/lintlist/mod.rs b/src/tools/clippy/src/lintlist/mod.rs index 9363039041da..1f3f70631fb2 100644 --- a/src/tools/clippy/src/lintlist/mod.rs +++ b/src/tools/clippy/src/lintlist/mod.rs @@ -2218,7 +2218,7 @@ pub static ref ALL_LINTS: Vec = vec![ Lint { name: "transmutes_expressible_as_ptr_casts", group: "complexity", - desc: "default lint description", + desc: "transmutes that could be a pointer cast", deprecation: None, module: "transmute", }, From 32691da3d92adf88ffe44b197e64b86dd00f1a67 Mon Sep 17 00:00:00 2001 From: Ryan1729 Date: Thu, 6 Aug 2020 06:15:57 -0600 Subject: [PATCH 17/37] run clippy_dev fmt This seemed to overdo it a bit, affecting multiple submodules, and changing a file I didn't touch, so I didn't commit those changes --- .../clippy/clippy_lints/src/transmute.rs | 34 +++++++------------ 1 file changed, 12 insertions(+), 22 deletions(-) diff --git a/src/tools/clippy/clippy_lints/src/transmute.rs b/src/tools/clippy/clippy_lints/src/transmute.rs index 9f356811f220..7d707b8a0c85 100644 --- a/src/tools/clippy/clippy_lints/src/transmute.rs +++ b/src/tools/clippy/clippy_lints/src/transmute.rs @@ -8,8 +8,8 @@ use rustc_errors::Applicability; use rustc_hir::{Expr, ExprKind, GenericArg, Mutability, QPath, TyKind, UnOp}; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::ty::{self, cast::CastKind, Ty}; -use rustc_span::DUMMY_SP; 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; @@ -698,18 +698,16 @@ fn is_layout_incompatible<'tcx>(cx: &LateContext<'tcx>, from: Ty<'tcx>, to: Ty<' /// 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 { +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::*; matches!( check_cast(cx, e, from_ty, to_ty), - Some( - PtrPtrCast - | PtrAddrCast - | AddrPtrCast - | ArrayPtrCast - | FnPtrPtrCast - | FnPtrAddrCast - ) + Some(PtrPtrCast | PtrAddrCast | AddrPtrCast | ArrayPtrCast | FnPtrPtrCast | FnPtrAddrCast) ) } @@ -722,26 +720,18 @@ fn check_cast<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>, from_ty: Ty<'tcx> 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 - ); + 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(), + !fn_ctxt.errors_reported_since_creation(), "Newly created FnCtxt contained errors" ); if let Ok(check) = CastCheck::new( - &fn_ctxt, - e, - from_ty, - to_ty, + &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, + DUMMY_SP, DUMMY_SP, ) { let res = check.do_check(&fn_ctxt); From d897fd28ae46b031a67814ffe29922b14bd028d4 Mon Sep 17 00:00:00 2001 From: Ryan Wiedemann Date: Thu, 6 Aug 2020 07:57:31 -0600 Subject: [PATCH 18/37] Apply suggestions from code review Co-authored-by: Philipp Krones --- src/tools/clippy/clippy_lints/src/transmute.rs | 4 ++-- .../clippy/tests/ui/transmutes_expressible_as_ptr_casts.rs | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/tools/clippy/clippy_lints/src/transmute.rs b/src/tools/clippy/clippy_lints/src/transmute.rs index 7d707b8a0c85..f077c1461831 100644 --- a/src/tools/clippy/clippy_lints/src/transmute.rs +++ b/src/tools/clippy/clippy_lints/src/transmute.rs @@ -640,7 +640,7 @@ impl<'tcx> LateLintPass<'tcx> for Transmute { |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::Unspecified); + diag.span_suggestion(e.span, "try", sugg, Applicability::MachineApplicable); } } ), @@ -694,7 +694,7 @@ fn is_layout_incompatible<'tcx>(cx: &LateContext<'tcx>, from: Ty<'tcx>, to: Ty<' } } -/// Check if the the type conversion can be expressed as a pointer cast, instead of +/// 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. diff --git a/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.rs b/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.rs index 7e99c9283598..2693094ba6ce 100644 --- a/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.rs +++ b/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.rs @@ -1,3 +1,4 @@ +// 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 From c04c4cb6e4b02b44501ab8e136616cbccaabd602 Mon Sep 17 00:00:00 2001 From: Ryan1729 Date: Thu, 6 Aug 2020 20:28:29 -0600 Subject: [PATCH 19/37] copy over *.fixed file --- .../transmutes_expressible_as_ptr_casts.fixed | 77 +++++++++++++++++++ 1 file changed, 77 insertions(+) create mode 100644 src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.fixed diff --git a/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.fixed b/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.fixed new file mode 100644 index 000000000000..ab181687e1eb --- /dev/null +++ b/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.fixed @@ -0,0 +1,77 @@ +// 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)] + +use std::mem::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 { + -1 as *const i32 + }; + let ptr_i32 = -1isize 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 } +} From 48a6c2125b5313f5d0f6d713d72033dbe559588e Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 7 Aug 2020 11:19:07 +0200 Subject: [PATCH 20/37] Only add a border for the rust logo --- src/librustdoc/html/layout.rs | 2 +- src/librustdoc/html/static/themes/ayu.css | 7 +++++-- src/librustdoc/html/static/themes/dark.css | 7 +++++-- src/librustdoc/html/static/themes/light.css | 4 ++-- 4 files changed, 13 insertions(+), 7 deletions(-) diff --git a/src/librustdoc/html/layout.rs b/src/librustdoc/html/layout.rs index cc6b38ebcdb7..03011ce4ab30 100644 --- a/src/librustdoc/html/layout.rs +++ b/src/librustdoc/html/layout.rs @@ -138,7 +138,7 @@ pub fn render( if layout.logo.is_empty() { format!( "\ -
\ + ", path = p, static_root_path = static_root_path, diff --git a/src/librustdoc/html/static/themes/ayu.css b/src/librustdoc/html/static/themes/ayu.css index f4710f6ae873..d07661e91dba 100644 --- a/src/librustdoc/html/static/themes/ayu.css +++ b/src/librustdoc/html/static/themes/ayu.css @@ -62,8 +62,11 @@ pre { background-color: #14191f; } -.logo-container > img { - filter: drop-shadow(0 0 5px #fff); +.logo-container.rust-logo > img { + filter: drop-shadow(1px 0 0px #fff) + drop-shadow(0 1px 0 #fff) + drop-shadow(-1px 0 0 #fff) + drop-shadow(0 -1px 0 #fff); } /* Improve the scrollbar display on firefox */ diff --git a/src/librustdoc/html/static/themes/dark.css b/src/librustdoc/html/static/themes/dark.css index b3b586ba362f..1e723d31fc8b 100644 --- a/src/librustdoc/html/static/themes/dark.css +++ b/src/librustdoc/html/static/themes/dark.css @@ -34,8 +34,11 @@ pre { background-color: #505050; } -.logo-container > img { - filter: drop-shadow(0 0 5px #fff); +.logo-container.rust-logo > img { + filter: drop-shadow(1px 0 0px #fff) + drop-shadow(0 1px 0 #fff) + drop-shadow(-1px 0 0 #fff) + drop-shadow(0 -1px 0 #fff) } /* Improve the scrollbar display on firefox */ diff --git a/src/librustdoc/html/static/themes/light.css b/src/librustdoc/html/static/themes/light.css index b0c5715604ba..793c68042ff5 100644 --- a/src/librustdoc/html/static/themes/light.css +++ b/src/librustdoc/html/static/themes/light.css @@ -45,8 +45,8 @@ pre { scrollbar-color: rgba(36, 37, 39, 0.6) #d9d9d9; } -.logo-container > img { - filter: drop-shadow(0 0 5px #aaa); +.logo-container.rust-logo > img { + /* No need for a border in here! */ } /* Improve the scrollbar display on webkit-based browsers */ From b21a3de35f4841ce4f5f7cd997faf9531ea998f2 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Sat, 8 Aug 2020 18:09:40 -0700 Subject: [PATCH 21/37] Don't try to use wasm intrinsics on vectors This commit fixes an issue with #74695 where the fptosi and fptoui specializations on wasm were accidentally used on vector types by the `simd_cast` intrinsic. This issue showed up as broken CI for the stdsimd crate. Here this commit simply skips the specialization on vector kinds flowing into `fpto{s,u}i`. --- src/librustc_codegen_llvm/builder.rs | 55 ++++++++++++++++------------ 1 file changed, 31 insertions(+), 24 deletions(-) diff --git a/src/librustc_codegen_llvm/builder.rs b/src/librustc_codegen_llvm/builder.rs index 1124e91bf718..a0f4311b33a2 100644 --- a/src/librustc_codegen_llvm/builder.rs +++ b/src/librustc_codegen_llvm/builder.rs @@ -728,20 +728,25 @@ impl BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> { // codegen. Note that this has a semantic difference in that the // intrinsic can trap whereas `fptoui` never traps. That difference, // however, is handled by `fptosui_may_trap` above. + // + // Note that we skip the wasm intrinsics for vector types where `fptoui` + // must be used instead. if self.wasm_and_missing_nontrapping_fptoint() { let src_ty = self.cx.val_ty(val); - let float_width = self.cx.float_width(src_ty); - let int_width = self.cx.int_width(dest_ty); - let name = match (int_width, float_width) { - (32, 32) => Some("llvm.wasm.trunc.unsigned.i32.f32"), - (32, 64) => Some("llvm.wasm.trunc.unsigned.i32.f64"), - (64, 32) => Some("llvm.wasm.trunc.unsigned.i64.f32"), - (64, 64) => Some("llvm.wasm.trunc.unsigned.i64.f64"), - _ => None, - }; - if let Some(name) = name { - let intrinsic = self.get_intrinsic(name); - return self.call(intrinsic, &[val], None); + if self.cx.type_kind(src_ty) != TypeKind::Vector { + let float_width = self.cx.float_width(src_ty); + let int_width = self.cx.int_width(dest_ty); + let name = match (int_width, float_width) { + (32, 32) => Some("llvm.wasm.trunc.unsigned.i32.f32"), + (32, 64) => Some("llvm.wasm.trunc.unsigned.i32.f64"), + (64, 32) => Some("llvm.wasm.trunc.unsigned.i64.f32"), + (64, 64) => Some("llvm.wasm.trunc.unsigned.i64.f64"), + _ => None, + }; + if let Some(name) = name { + let intrinsic = self.get_intrinsic(name); + return self.call(intrinsic, &[val], None); + } } } unsafe { llvm::LLVMBuildFPToUI(self.llbuilder, val, dest_ty, UNNAMED) } @@ -750,18 +755,20 @@ impl BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> { fn fptosi(&mut self, val: &'ll Value, dest_ty: &'ll Type) -> &'ll Value { if self.wasm_and_missing_nontrapping_fptoint() { let src_ty = self.cx.val_ty(val); - let float_width = self.cx.float_width(src_ty); - let int_width = self.cx.int_width(dest_ty); - let name = match (int_width, float_width) { - (32, 32) => Some("llvm.wasm.trunc.signed.i32.f32"), - (32, 64) => Some("llvm.wasm.trunc.signed.i32.f64"), - (64, 32) => Some("llvm.wasm.trunc.signed.i64.f32"), - (64, 64) => Some("llvm.wasm.trunc.signed.i64.f64"), - _ => None, - }; - if let Some(name) = name { - let intrinsic = self.get_intrinsic(name); - return self.call(intrinsic, &[val], None); + if self.cx.type_kind(src_ty) != TypeKind::Vector { + let float_width = self.cx.float_width(src_ty); + let int_width = self.cx.int_width(dest_ty); + let name = match (int_width, float_width) { + (32, 32) => Some("llvm.wasm.trunc.signed.i32.f32"), + (32, 64) => Some("llvm.wasm.trunc.signed.i32.f64"), + (64, 32) => Some("llvm.wasm.trunc.signed.i64.f32"), + (64, 64) => Some("llvm.wasm.trunc.signed.i64.f64"), + _ => None, + }; + if let Some(name) = name { + let intrinsic = self.get_intrinsic(name); + return self.call(intrinsic, &[val], None); + } } } unsafe { llvm::LLVMBuildFPToSI(self.llbuilder, val, dest_ty, UNNAMED) } From b02bf056905b504044a3c491870ac3c0042b7dea Mon Sep 17 00:00:00 2001 From: Ryan1729 Date: Sat, 8 Aug 2020 21:03:41 -0600 Subject: [PATCH 22/37] update stderr for transmutes_expressible_as_ptr_casts --- .../transmutes_expressible_as_ptr_casts.stderr | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.stderr b/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.stderr index e6edacbd0de8..721888802157 100644 --- a/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.stderr +++ b/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.stderr @@ -1,5 +1,5 @@ error: transmute from an integer to a pointer - --> $DIR/transmutes_expressible_as_ptr_casts.rs:18:9 + --> $DIR/transmutes_expressible_as_ptr_casts.rs:19:9 | LL | transmute::(-1) | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `-1 as *const i32` @@ -7,7 +7,7 @@ LL | transmute::(-1) = note: `-D clippy::useless-transmute` implied by `-D warnings` error: transmute from a pointer to a pointer - --> $DIR/transmutes_expressible_as_ptr_casts.rs:24:9 + --> $DIR/transmutes_expressible_as_ptr_casts.rs:25:9 | LL | transmute::<*const i32, *const i8>(ptr_i32) | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `ptr_i32 as *const i8` @@ -15,13 +15,13 @@ LL | transmute::<*const i32, *const i8>(ptr_i32) = note: `-D clippy::transmute-ptr-to-ptr` implied by `-D warnings` error: transmute from a pointer to a pointer - --> $DIR/transmutes_expressible_as_ptr_casts.rs:32:9 + --> $DIR/transmutes_expressible_as_ptr_casts.rs:33:9 | LL | transmute::<*const [i32], *const [u16]>(slice_ptr) | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `slice_ptr as *const [u16]` error: transmute from `*const i32` to `usize` which could be expressed as a pointer cast instead - --> $DIR/transmutes_expressible_as_ptr_casts.rs:40:9 + --> $DIR/transmutes_expressible_as_ptr_casts.rs:41:9 | LL | transmute::<*const i32, usize>(ptr_i32) | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `ptr_i32 as usize` @@ -29,25 +29,25 @@ LL | transmute::<*const i32, usize>(ptr_i32) = note: `-D clippy::transmutes-expressible-as-ptr-casts` implied by `-D warnings` error: transmute from a reference to a pointer - --> $DIR/transmutes_expressible_as_ptr_casts.rs:48:9 + --> $DIR/transmutes_expressible_as_ptr_casts.rs:49:9 | LL | transmute::<&[i32; 4], *const [i32; 4]>(array_ref) | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `array_ref as *const [i32; 4]` error: transmute from `fn(usize) -> u8 {main::foo}` to `*const usize` which could be expressed as a pointer cast instead - --> $DIR/transmutes_expressible_as_ptr_casts.rs:56:9 + --> $DIR/transmutes_expressible_as_ptr_casts.rs:57:9 | LL | transmute:: u8, *const usize>(foo) | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `foo as *const usize` error: transmute from `fn(usize) -> u8 {main::foo}` to `usize` which could be expressed as a pointer cast instead - --> $DIR/transmutes_expressible_as_ptr_casts.rs:62:9 + --> $DIR/transmutes_expressible_as_ptr_casts.rs:63:9 | LL | transmute:: u8, usize>(foo) | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `foo as usize` error: transmute from a reference to a pointer - --> $DIR/transmutes_expressible_as_ptr_casts.rs:75:14 + --> $DIR/transmutes_expressible_as_ptr_casts.rs:76:14 | LL | unsafe { transmute::<&[i32; 1], *const u8>(in_param) } | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `in_param as *const [i32; 1] as *const u8` From 0fbf450afc4d085694907e3dbf278a08636ba4cb Mon Sep 17 00:00:00 2001 From: Ryan1729 Date: Sun, 9 Aug 2020 00:15:56 -0600 Subject: [PATCH 23/37] add a test example of where transmutes_expressible_as_ptr_casts should not suggest anything --- .../ui/transmutes_expressible_as_ptr_casts.fixed | 14 +++++++++++++- .../ui/transmutes_expressible_as_ptr_casts.rs | 14 +++++++++++++- 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.fixed b/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.fixed index ab181687e1eb..d80c9f62ed06 100644 --- a/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.fixed +++ b/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.fixed @@ -5,7 +5,7 @@ #![warn(clippy::useless_transmute)] #![warn(clippy::transmute_ptr_to_ptr)] -use std::mem::transmute; +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. @@ -75,3 +75,15 @@ fn main() { 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::(), size_of::()); + + unsafe { transmute::(in_param) } +} diff --git a/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.rs b/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.rs index 2693094ba6ce..4f27a3a88ba7 100644 --- a/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.rs +++ b/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.rs @@ -5,7 +5,7 @@ #![warn(clippy::useless_transmute)] #![warn(clippy::transmute_ptr_to_ptr)] -use std::mem::transmute; +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. @@ -75,3 +75,15 @@ fn main() { 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::(), size_of::()); + + unsafe { transmute::(in_param) } +} From 58b8b117917902ffa1773dddd6c3eb979efd1eac Mon Sep 17 00:00:00 2001 From: Ryan1729 Date: Sun, 9 Aug 2020 00:28:56 -0600 Subject: [PATCH 24/37] fix unary minus on usize and unused variable errors in .fixed file --- .../transmutes_expressible_as_ptr_casts.fixed | 30 +++++++++---------- .../ui/transmutes_expressible_as_ptr_casts.rs | 30 +++++++++---------- ...transmutes_expressible_as_ptr_casts.stderr | 4 +-- 3 files changed, 32 insertions(+), 32 deletions(-) diff --git a/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.fixed b/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.fixed index d80c9f62ed06..4d3eba93bb3e 100644 --- a/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.fixed +++ b/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.fixed @@ -15,54 +15,54 @@ fn main() { // 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 { - -1 as *const i32 + let _ptr_i32_transmute = unsafe { + usize::MAX as *const i32 }; - let ptr_i32 = -1isize 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 { + let _ptr_i8_transmute = unsafe { ptr_i32 as *const i8 }; - let ptr_i8 = 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 { + let _ptr_to_unsized_transmute = unsafe { slice_ptr as *const [u16] }; - let ptr_to_unsized = 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 { + let _usize_from_int_ptr_transmute = unsafe { ptr_i32 as usize }; - let usize_from_int_ptr = 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 { + let _array_ptr_transmute = unsafe { array_ref as *const [i32; 4] }; - let array_ptr = 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 { + let _usize_ptr_transmute = unsafe { foo as *const usize }; - let usize_ptr_transmute = 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 { + let _usize_from_fn_ptr_transmute = unsafe { foo as usize }; - let usize_from_fn_ptr = foo as *const 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 diff --git a/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.rs b/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.rs index 4f27a3a88ba7..874812106433 100644 --- a/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.rs +++ b/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.rs @@ -15,54 +15,54 @@ fn main() { // 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::(-1) + let _ptr_i32_transmute = unsafe { + transmute::(usize::MAX) }; - let ptr_i32 = -1isize 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 { + let _ptr_i8_transmute = unsafe { transmute::<*const i32, *const i8>(ptr_i32) }; - let ptr_i8 = 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 { + let _ptr_to_unsized_transmute = unsafe { transmute::<*const [i32], *const [u16]>(slice_ptr) }; - let ptr_to_unsized = 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 { + let _usize_from_int_ptr_transmute = unsafe { transmute::<*const i32, usize>(ptr_i32) }; - let usize_from_int_ptr = 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 { + let _array_ptr_transmute = unsafe { transmute::<&[i32; 4], *const [i32; 4]>(array_ref) }; - let array_ptr = 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 { + let _usize_ptr_transmute = unsafe { transmute:: u8, *const usize>(foo) }; - let usize_ptr_transmute = 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 { + let _usize_from_fn_ptr_transmute = unsafe { transmute:: u8, usize>(foo) }; - let usize_from_fn_ptr = foo as *const 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 diff --git a/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.stderr b/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.stderr index 721888802157..72fb8689d657 100644 --- a/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.stderr +++ b/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.stderr @@ -1,8 +1,8 @@ error: transmute from an integer to a pointer --> $DIR/transmutes_expressible_as_ptr_casts.rs:19:9 | -LL | transmute::(-1) - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `-1 as *const i32` +LL | transmute::(usize::MAX) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `usize::MAX as *const i32` | = note: `-D clippy::useless-transmute` implied by `-D warnings` From d2e7293078076c801a3774cdb0c805da3e1ba154 Mon Sep 17 00:00:00 2001 From: Ryan1729 Date: Sun, 9 Aug 2020 00:39:14 -0600 Subject: [PATCH 25/37] add allow unused_unsafe and allow dead_code --- .../ui/transmutes_expressible_as_ptr_casts.fixed | 3 ++- .../ui/transmutes_expressible_as_ptr_casts.rs | 3 ++- .../transmutes_expressible_as_ptr_casts.stderr | 16 ++++++++-------- 3 files changed, 12 insertions(+), 10 deletions(-) diff --git a/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.fixed b/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.fixed index 4d3eba93bb3e..98288dde6d84 100644 --- a/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.fixed +++ b/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.fixed @@ -4,12 +4,13 @@ // 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. diff --git a/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.rs b/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.rs index 874812106433..fd5055c08f63 100644 --- a/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.rs +++ b/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.rs @@ -4,12 +4,13 @@ // 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. diff --git a/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.stderr b/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.stderr index 72fb8689d657..46597acc6c0d 100644 --- a/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.stderr +++ b/src/tools/clippy/tests/ui/transmutes_expressible_as_ptr_casts.stderr @@ -1,5 +1,5 @@ error: transmute from an integer to a pointer - --> $DIR/transmutes_expressible_as_ptr_casts.rs:19:9 + --> $DIR/transmutes_expressible_as_ptr_casts.rs:20:9 | LL | transmute::(usize::MAX) | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `usize::MAX as *const i32` @@ -7,7 +7,7 @@ LL | transmute::(usize::MAX) = note: `-D clippy::useless-transmute` implied by `-D warnings` error: transmute from a pointer to a pointer - --> $DIR/transmutes_expressible_as_ptr_casts.rs:25:9 + --> $DIR/transmutes_expressible_as_ptr_casts.rs:26:9 | LL | transmute::<*const i32, *const i8>(ptr_i32) | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `ptr_i32 as *const i8` @@ -15,13 +15,13 @@ LL | transmute::<*const i32, *const i8>(ptr_i32) = note: `-D clippy::transmute-ptr-to-ptr` implied by `-D warnings` error: transmute from a pointer to a pointer - --> $DIR/transmutes_expressible_as_ptr_casts.rs:33:9 + --> $DIR/transmutes_expressible_as_ptr_casts.rs:34:9 | LL | transmute::<*const [i32], *const [u16]>(slice_ptr) | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `slice_ptr as *const [u16]` error: transmute from `*const i32` to `usize` which could be expressed as a pointer cast instead - --> $DIR/transmutes_expressible_as_ptr_casts.rs:41:9 + --> $DIR/transmutes_expressible_as_ptr_casts.rs:42:9 | LL | transmute::<*const i32, usize>(ptr_i32) | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `ptr_i32 as usize` @@ -29,25 +29,25 @@ LL | transmute::<*const i32, usize>(ptr_i32) = note: `-D clippy::transmutes-expressible-as-ptr-casts` implied by `-D warnings` error: transmute from a reference to a pointer - --> $DIR/transmutes_expressible_as_ptr_casts.rs:49:9 + --> $DIR/transmutes_expressible_as_ptr_casts.rs:50:9 | LL | transmute::<&[i32; 4], *const [i32; 4]>(array_ref) | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `array_ref as *const [i32; 4]` error: transmute from `fn(usize) -> u8 {main::foo}` to `*const usize` which could be expressed as a pointer cast instead - --> $DIR/transmutes_expressible_as_ptr_casts.rs:57:9 + --> $DIR/transmutes_expressible_as_ptr_casts.rs:58:9 | LL | transmute:: u8, *const usize>(foo) | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `foo as *const usize` error: transmute from `fn(usize) -> u8 {main::foo}` to `usize` which could be expressed as a pointer cast instead - --> $DIR/transmutes_expressible_as_ptr_casts.rs:63:9 + --> $DIR/transmutes_expressible_as_ptr_casts.rs:64:9 | LL | transmute:: u8, usize>(foo) | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `foo as usize` error: transmute from a reference to a pointer - --> $DIR/transmutes_expressible_as_ptr_casts.rs:76:14 + --> $DIR/transmutes_expressible_as_ptr_casts.rs:77:14 | LL | unsafe { transmute::<&[i32; 1], *const u8>(in_param) } | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `in_param as *const [i32; 1] as *const u8` From 2627eedde97e3e94e786faf8dfe612d65d8a6fa6 Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Sat, 8 Aug 2020 21:05:50 -0400 Subject: [PATCH 26/37] Avoid deleting temporary files on error Previously if the compiler error'd, fatally, then temporary directories which should be preserved by -Csave-temps would be deleted due to fatal compiler errors being implemented as panics. --- Cargo.lock | 1 + src/librustc_codegen_ssa/back/link.rs | 39 +++++++++--------------- src/librustc_data_structures/Cargo.toml | 1 + src/librustc_data_structures/lib.rs | 1 + src/librustc_data_structures/temp_dir.rs | 34 +++++++++++++++++++++ src/librustc_interface/passes.rs | 2 ++ 6 files changed, 54 insertions(+), 24 deletions(-) create mode 100644 src/librustc_data_structures/temp_dir.rs diff --git a/Cargo.lock b/Cargo.lock index a3b37465f1a7..684cd82c8373 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3363,6 +3363,7 @@ dependencies = [ "smallvec 1.4.0", "stable_deref_trait", "stacker", + "tempfile", "tracing", "winapi 0.3.8", ] diff --git a/src/librustc_codegen_ssa/back/link.rs b/src/librustc_codegen_ssa/back/link.rs index 9191c68d4537..e06edb2fe6cc 100644 --- a/src/librustc_codegen_ssa/back/link.rs +++ b/src/librustc_codegen_ssa/back/link.rs @@ -1,4 +1,5 @@ use rustc_data_structures::fx::FxHashSet; +use rustc_data_structures::temp_dir::MaybeTempDir; use rustc_fs_util::fix_windows_verbatim_for_gcc; use rustc_hir::def_id::CrateNum; use rustc_middle::middle::cstore::{EncodedMetadata, LibSource, NativeLib}; @@ -23,7 +24,7 @@ use super::rpath::{self, RPathConfig}; use crate::{looks_like_rust_object_file, CodegenResults, CrateInfo, METADATA_FILENAME}; use cc::windows_registry; -use tempfile::{Builder as TempFileBuilder, TempDir}; +use tempfile::Builder as TempFileBuilder; use std::ffi::OsString; use std::path::{Path, PathBuf}; @@ -70,27 +71,21 @@ pub fn link_binary<'a, B: ArchiveBuilder<'a>>( } }); - let tmpdir = TempFileBuilder::new() - .prefix("rustc") - .tempdir() - .unwrap_or_else(|err| sess.fatal(&format!("couldn't create a temp dir: {}", err))); - if outputs.outputs.should_codegen() { + let tmpdir = TempFileBuilder::new() + .prefix("rustc") + .tempdir() + .unwrap_or_else(|err| sess.fatal(&format!("couldn't create a temp dir: {}", err))); + let path = MaybeTempDir::new(tmpdir, sess.opts.cg.save_temps); let out_filename = out_filename(sess, crate_type, outputs, crate_name); match crate_type { CrateType::Rlib => { let _timer = sess.timer("link_rlib"); - link_rlib::( - sess, - codegen_results, - RlibFlavor::Normal, - &out_filename, - &tmpdir, - ) - .build(); + link_rlib::(sess, codegen_results, RlibFlavor::Normal, &out_filename, &path) + .build(); } CrateType::Staticlib => { - link_staticlib::(sess, codegen_results, &out_filename, &tmpdir); + link_staticlib::(sess, codegen_results, &out_filename, &path); } _ => { link_natively::( @@ -98,7 +93,7 @@ pub fn link_binary<'a, B: ArchiveBuilder<'a>>( crate_type, &out_filename, codegen_results, - tmpdir.path(), + path.as_ref(), target_cpu, ); } @@ -107,10 +102,6 @@ pub fn link_binary<'a, B: ArchiveBuilder<'a>>( sess.parse_sess.span_diagnostic.emit_artifact_notification(&out_filename, "link"); } } - - if sess.opts.cg.save_temps { - let _ = tmpdir.into_path(); - } } // Remove the temporary object file and metadata if we aren't saving temps @@ -279,8 +270,8 @@ pub fn each_linked_rlib( /// building an `.rlib` (stomping over one another), or writing an `.rmeta` into a /// directory being searched for `extern crate` (observing an incomplete file). /// The returned path is the temporary file containing the complete metadata. -pub fn emit_metadata(sess: &Session, metadata: &EncodedMetadata, tmpdir: &TempDir) -> PathBuf { - let out_filename = tmpdir.path().join(METADATA_FILENAME); +pub fn emit_metadata(sess: &Session, metadata: &EncodedMetadata, tmpdir: &MaybeTempDir) -> PathBuf { + let out_filename = tmpdir.as_ref().join(METADATA_FILENAME); let result = fs::write(&out_filename, &metadata.raw_data); if let Err(e) = result { @@ -301,7 +292,7 @@ fn link_rlib<'a, B: ArchiveBuilder<'a>>( codegen_results: &CodegenResults, flavor: RlibFlavor, out_filename: &Path, - tmpdir: &TempDir, + tmpdir: &MaybeTempDir, ) -> B { info!("preparing rlib to {:?}", out_filename); let mut ab = ::new(sess, out_filename, None); @@ -406,7 +397,7 @@ fn link_staticlib<'a, B: ArchiveBuilder<'a>>( sess: &'a Session, codegen_results: &CodegenResults, out_filename: &Path, - tempdir: &TempDir, + tempdir: &MaybeTempDir, ) { let mut ab = link_rlib::(sess, codegen_results, RlibFlavor::StaticlibBase, out_filename, tempdir); diff --git a/src/librustc_data_structures/Cargo.toml b/src/librustc_data_structures/Cargo.toml index 811d1e49626c..65812cc4e687 100644 --- a/src/librustc_data_structures/Cargo.toml +++ b/src/librustc_data_structures/Cargo.toml @@ -30,6 +30,7 @@ bitflags = "1.2.1" measureme = "0.7.1" libc = "0.2" stacker = "0.1.9" +tempfile = "3.0.5" [dependencies.parking_lot] version = "0.10" diff --git a/src/librustc_data_structures/lib.rs b/src/librustc_data_structures/lib.rs index 0b2e7cda1b4c..3884fc051056 100644 --- a/src/librustc_data_structures/lib.rs +++ b/src/librustc_data_structures/lib.rs @@ -95,6 +95,7 @@ pub mod vec_linked_list; pub mod work_queue; pub use atomic_ref::AtomicRef; pub mod frozen; +pub mod temp_dir; pub struct OnDrop(pub F); diff --git a/src/librustc_data_structures/temp_dir.rs b/src/librustc_data_structures/temp_dir.rs new file mode 100644 index 000000000000..0d9b3e3ca25c --- /dev/null +++ b/src/librustc_data_structures/temp_dir.rs @@ -0,0 +1,34 @@ +use std::mem::ManuallyDrop; +use std::path::Path; +use tempfile::TempDir; + +/// This is used to avoid TempDir being dropped on error paths unintentionally. +#[derive(Debug)] +pub struct MaybeTempDir { + dir: ManuallyDrop, + // Whether the TempDir should be deleted on drop. + keep: bool, +} + +impl Drop for MaybeTempDir { + fn drop(&mut self) { + // Safety: We are in the destructor, and no further access will + // occur. + let dir = unsafe { ManuallyDrop::take(&mut self.dir) }; + if self.keep { + dir.into_path(); + } + } +} + +impl AsRef for MaybeTempDir { + fn as_ref(&self) -> &Path { + self.dir.path() + } +} + +impl MaybeTempDir { + pub fn new(dir: TempDir, keep_on_drop: bool) -> MaybeTempDir { + MaybeTempDir { dir: ManuallyDrop::new(dir), keep: keep_on_drop } + } +} diff --git a/src/librustc_interface/passes.rs b/src/librustc_interface/passes.rs index 6c0343330c8c..701fca8e4b53 100644 --- a/src/librustc_interface/passes.rs +++ b/src/librustc_interface/passes.rs @@ -9,6 +9,7 @@ use rustc_ast::{self, ast, visit}; use rustc_codegen_ssa::back::link::emit_metadata; use rustc_codegen_ssa::traits::CodegenBackend; use rustc_data_structures::sync::{par_iter, Lrc, OnceCell, ParallelIterator, WorkerLocal}; +use rustc_data_structures::temp_dir::MaybeTempDir; use rustc_data_structures::{box_region_allow_access, declare_box_region_type, parallel}; use rustc_errors::{ErrorReported, PResult}; use rustc_expand::base::ExtCtxt; @@ -974,6 +975,7 @@ fn encode_and_write_metadata( .prefix("rmeta") .tempdir_in(out_filename.parent().unwrap()) .unwrap_or_else(|err| tcx.sess.fatal(&format!("couldn't create a temp dir: {}", err))); + let metadata_tmpdir = MaybeTempDir::new(metadata_tmpdir, tcx.sess.opts.cg.save_temps); let metadata_filename = emit_metadata(tcx.sess, &metadata, &metadata_tmpdir); if let Err(e) = fs::rename(&metadata_filename, &out_filename) { tcx.sess.fatal(&format!("failed to write {}: {}", out_filename.display(), e)); From bff69c9525b8c498fb6485ac38b4b3497bb37c1a Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 9 Aug 2020 15:37:32 +0200 Subject: [PATCH 27/37] move const_eval error reporting logic into rustc_mir::const_eval::error --- src/librustc_middle/mir/interpret/error.rs | 169 +------------------ src/librustc_middle/mir/interpret/mod.rs | 6 +- src/librustc_mir/const_eval/error.rs | 170 ++++++++++++++++++-- src/librustc_mir/const_eval/eval_queries.rs | 9 +- src/librustc_mir/interpret/eval_context.rs | 35 +++- src/librustc_mir/interpret/mod.rs | 2 +- src/librustc_mir/transform/const_prop.rs | 4 +- 7 files changed, 201 insertions(+), 194 deletions(-) diff --git a/src/librustc_middle/mir/interpret/error.rs b/src/librustc_middle/mir/interpret/error.rs index c904cd83a78b..5be09c0e9bc2 100644 --- a/src/librustc_middle/mir/interpret/error.rs +++ b/src/librustc_middle/mir/interpret/error.rs @@ -1,17 +1,13 @@ use super::{AllocId, Pointer, RawConst, Scalar}; use crate::mir::interpret::ConstValue; -use crate::ty::layout::LayoutError; -use crate::ty::query::TyCtxtAt; -use crate::ty::{self, layout, tls, FnSig, Ty}; +use crate::ty::{layout, query::TyCtxtAt, tls, FnSig, Ty}; use rustc_data_structures::sync::Lock; use rustc_errors::{pluralize, struct_span_err, DiagnosticBuilder, ErrorReported}; -use rustc_hir as hir; -use rustc_hir::definitions::DefPathData; use rustc_macros::HashStable; use rustc_session::CtfeBacktrace; -use rustc_span::{def_id::DefId, Pos, Span}; +use rustc_span::def_id::DefId; use rustc_target::abi::{Align, Size}; use std::{any::Any, backtrace::Backtrace, fmt, mem}; @@ -34,167 +30,6 @@ CloneTypeFoldableAndLiftImpls! { pub type ConstEvalRawResult<'tcx> = Result, ErrorHandled>; pub type ConstEvalResult<'tcx> = Result, ErrorHandled>; -#[derive(Debug)] -pub struct ConstEvalErr<'tcx> { - pub span: Span, - pub error: crate::mir::interpret::InterpError<'tcx>, - pub stacktrace: Vec>, -} - -#[derive(Debug)] -pub struct FrameInfo<'tcx> { - pub instance: ty::Instance<'tcx>, - pub span: Span, - pub lint_root: Option, -} - -impl<'tcx> fmt::Display for FrameInfo<'tcx> { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - ty::tls::with(|tcx| { - if tcx.def_key(self.instance.def_id()).disambiguated_data.data - == DefPathData::ClosureExpr - { - write!(f, "inside closure")?; - } else { - write!(f, "inside `{}`", self.instance)?; - } - if !self.span.is_dummy() { - let lo = tcx.sess.source_map().lookup_char_pos(self.span.lo()); - write!(f, " at {}:{}:{}", lo.file.name, lo.line, lo.col.to_usize() + 1)?; - } - Ok(()) - }) - } -} - -impl<'tcx> ConstEvalErr<'tcx> { - pub fn struct_error( - &self, - tcx: TyCtxtAt<'tcx>, - message: &str, - emit: impl FnOnce(DiagnosticBuilder<'_>), - ) -> ErrorHandled { - self.struct_generic(tcx, message, emit, None) - } - - pub fn report_as_error(&self, tcx: TyCtxtAt<'tcx>, message: &str) -> ErrorHandled { - self.struct_error(tcx, message, |mut e| e.emit()) - } - - pub fn report_as_lint( - &self, - tcx: TyCtxtAt<'tcx>, - message: &str, - lint_root: hir::HirId, - span: Option, - ) -> ErrorHandled { - self.struct_generic( - tcx, - message, - |mut lint: DiagnosticBuilder<'_>| { - // Apply the span. - if let Some(span) = span { - let primary_spans = lint.span.primary_spans().to_vec(); - // point at the actual error as the primary span - lint.replace_span_with(span); - // point to the `const` statement as a secondary span - // they don't have any label - for sp in primary_spans { - if sp != span { - lint.span_label(sp, ""); - } - } - } - lint.emit(); - }, - Some(lint_root), - ) - } - - /// Create a diagnostic for this const eval error. - /// - /// Sets the message passed in via `message` and adds span labels with detailed error - /// information before handing control back to `emit` to do any final processing. - /// It's the caller's responsibility to call emit(), stash(), etc. within the `emit` - /// function to dispose of the diagnostic properly. - /// - /// If `lint_root.is_some()` report it as a lint, else report it as a hard error. - /// (Except that for some errors, we ignore all that -- see `must_error` below.) - fn struct_generic( - &self, - tcx: TyCtxtAt<'tcx>, - message: &str, - emit: impl FnOnce(DiagnosticBuilder<'_>), - lint_root: Option, - ) -> ErrorHandled { - let must_error = match self.error { - err_inval!(Layout(LayoutError::Unknown(_))) | err_inval!(TooGeneric) => { - return ErrorHandled::TooGeneric; - } - err_inval!(TypeckError(error_reported)) => { - return ErrorHandled::Reported(error_reported); - } - // We must *always* hard error on these, even if the caller wants just a lint. - err_inval!(Layout(LayoutError::SizeOverflow(_))) => true, - _ => false, - }; - trace!("reporting const eval failure at {:?}", self.span); - - let err_msg = match &self.error { - InterpError::MachineStop(msg) => { - // A custom error (`ConstEvalErrKind` in `librustc_mir/interp/const_eval/error.rs`). - // Should be turned into a string by now. - msg.downcast_ref::().expect("invalid MachineStop payload").clone() - } - err => err.to_string(), - }; - - let finish = |mut err: DiagnosticBuilder<'_>, span_msg: Option| { - if let Some(span_msg) = span_msg { - err.span_label(self.span, span_msg); - } - // Add spans for the stacktrace. Don't print a single-line backtrace though. - if self.stacktrace.len() > 1 { - for frame_info in &self.stacktrace { - err.span_label(frame_info.span, frame_info.to_string()); - } - } - // Let the caller finish the job. - emit(err) - }; - - if must_error { - // The `message` makes little sense here, this is a more serious error than the - // caller thinks anyway. - // See . - finish(struct_error(tcx, &err_msg), None); - ErrorHandled::Reported(ErrorReported) - } else { - // Regular case. - if let Some(lint_root) = lint_root { - // Report as lint. - let hir_id = self - .stacktrace - .iter() - .rev() - .find_map(|frame| frame.lint_root) - .unwrap_or(lint_root); - tcx.struct_span_lint_hir( - rustc_session::lint::builtin::CONST_ERR, - hir_id, - tcx.span, - |lint| finish(lint.build(message), Some(err_msg)), - ); - ErrorHandled::Linted - } else { - // Report as hard error. - finish(struct_error(tcx, message), Some(err_msg)); - ErrorHandled::Reported(ErrorReported) - } - } - } -} - pub fn struct_error<'tcx>(tcx: TyCtxtAt<'tcx>, msg: &str) -> DiagnosticBuilder<'tcx> { struct_span_err!(tcx.sess, tcx.span, E0080, "{}", msg) } diff --git a/src/librustc_middle/mir/interpret/mod.rs b/src/librustc_middle/mir/interpret/mod.rs index 5e57b60894a5..2507f2184fff 100644 --- a/src/librustc_middle/mir/interpret/mod.rs +++ b/src/librustc_middle/mir/interpret/mod.rs @@ -117,9 +117,9 @@ use crate::ty::subst::GenericArgKind; use crate::ty::{self, Instance, Ty, TyCtxt}; pub use self::error::{ - struct_error, CheckInAllocMsg, ConstEvalErr, ConstEvalRawResult, ConstEvalResult, ErrorHandled, - FrameInfo, InterpError, InterpErrorInfo, InterpResult, InvalidProgramInfo, MachineStopType, - ResourceExhaustionInfo, UndefinedBehaviorInfo, UninitBytesAccess, UnsupportedOpInfo, + struct_error, CheckInAllocMsg, ConstEvalRawResult, ConstEvalResult, ErrorHandled, InterpError, + InterpErrorInfo, InterpResult, InvalidProgramInfo, MachineStopType, ResourceExhaustionInfo, + UndefinedBehaviorInfo, UninitBytesAccess, UnsupportedOpInfo, }; pub use self::value::{get_slice_bytes, ConstValue, RawConst, Scalar, ScalarMaybeUninit}; diff --git a/src/librustc_mir/const_eval/error.rs b/src/librustc_mir/const_eval/error.rs index 8a72be33b842..044d27a6a9dc 100644 --- a/src/librustc_mir/const_eval/error.rs +++ b/src/librustc_mir/const_eval/error.rs @@ -1,12 +1,16 @@ use std::error::Error; use std::fmt; +use rustc_errors::{DiagnosticBuilder, ErrorReported}; +use rustc_hir as hir; use rustc_middle::mir::AssertKind; -use rustc_middle::ty::ConstInt; +use rustc_middle::ty::{layout::LayoutError, query::TyCtxtAt, ConstInt}; use rustc_span::{Span, Symbol}; use super::InterpCx; -use crate::interpret::{ConstEvalErr, InterpErrorInfo, Machine}; +use crate::interpret::{ + struct_error, ErrorHandled, FrameInfo, InterpError, InterpErrorInfo, Machine, +}; /// The CTFE machine has some custom error kinds. #[derive(Clone, Debug)] @@ -48,15 +52,155 @@ impl fmt::Display for ConstEvalErrKind { impl Error for ConstEvalErrKind {} -/// Turn an interpreter error into something to report to the user. -/// As a side-effect, if RUSTC_CTFE_BACKTRACE is set, this prints the backtrace. -/// Should be called only if the error is actually going to to be reported! -pub fn error_to_const_error<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>>( - ecx: &InterpCx<'mir, 'tcx, M>, - error: InterpErrorInfo<'tcx>, - span: Option, -) -> ConstEvalErr<'tcx> { - error.print_backtrace(); - let stacktrace = ecx.generate_stacktrace(); - ConstEvalErr { error: error.kind, stacktrace, span: span.unwrap_or_else(|| ecx.cur_span()) } +/// When const-evaluation errors, this type is constructed with the resulting information, +/// and then used to emit the error as a lint or hard error. +#[derive(Debug)] +pub struct ConstEvalErr<'tcx> { + pub span: Span, + pub error: InterpError<'tcx>, + pub stacktrace: Vec>, +} + +impl<'tcx> ConstEvalErr<'tcx> { + /// Turn an interpreter error into something to report to the user. + /// As a side-effect, if RUSTC_CTFE_BACKTRACE is set, this prints the backtrace. + /// Should be called only if the error is actually going to to be reported! + pub fn new<'mir, M: Machine<'mir, 'tcx>>( + ecx: &InterpCx<'mir, 'tcx, M>, + error: InterpErrorInfo<'tcx>, + span: Option, + ) -> ConstEvalErr<'tcx> + where + 'tcx: 'mir, + { + error.print_backtrace(); + let stacktrace = ecx.generate_stacktrace(); + ConstEvalErr { error: error.kind, stacktrace, span: span.unwrap_or_else(|| ecx.cur_span()) } + } + + pub fn struct_error( + &self, + tcx: TyCtxtAt<'tcx>, + message: &str, + emit: impl FnOnce(DiagnosticBuilder<'_>), + ) -> ErrorHandled { + self.struct_generic(tcx, message, emit, None) + } + + pub fn report_as_error(&self, tcx: TyCtxtAt<'tcx>, message: &str) -> ErrorHandled { + self.struct_error(tcx, message, |mut e| e.emit()) + } + + pub fn report_as_lint( + &self, + tcx: TyCtxtAt<'tcx>, + message: &str, + lint_root: hir::HirId, + span: Option, + ) -> ErrorHandled { + self.struct_generic( + tcx, + message, + |mut lint: DiagnosticBuilder<'_>| { + // Apply the span. + if let Some(span) = span { + let primary_spans = lint.span.primary_spans().to_vec(); + // point at the actual error as the primary span + lint.replace_span_with(span); + // point to the `const` statement as a secondary span + // they don't have any label + for sp in primary_spans { + if sp != span { + lint.span_label(sp, ""); + } + } + } + lint.emit(); + }, + Some(lint_root), + ) + } + + /// Create a diagnostic for this const eval error. + /// + /// Sets the message passed in via `message` and adds span labels with detailed error + /// information before handing control back to `emit` to do any final processing. + /// It's the caller's responsibility to call emit(), stash(), etc. within the `emit` + /// function to dispose of the diagnostic properly. + /// + /// If `lint_root.is_some()` report it as a lint, else report it as a hard error. + /// (Except that for some errors, we ignore all that -- see `must_error` below.) + fn struct_generic( + &self, + tcx: TyCtxtAt<'tcx>, + message: &str, + emit: impl FnOnce(DiagnosticBuilder<'_>), + lint_root: Option, + ) -> ErrorHandled { + let must_error = match self.error { + err_inval!(Layout(LayoutError::Unknown(_))) | err_inval!(TooGeneric) => { + return ErrorHandled::TooGeneric; + } + err_inval!(TypeckError(error_reported)) => { + return ErrorHandled::Reported(error_reported); + } + // We must *always* hard error on these, even if the caller wants just a lint. + err_inval!(Layout(LayoutError::SizeOverflow(_))) => true, + _ => false, + }; + trace!("reporting const eval failure at {:?}", self.span); + + let err_msg = match &self.error { + InterpError::MachineStop(msg) => { + // A custom error (`ConstEvalErrKind` in `librustc_mir/interp/const_eval/error.rs`). + // Should be turned into a string by now. + msg.downcast_ref::().expect("invalid MachineStop payload").clone() + } + err => err.to_string(), + }; + + let finish = |mut err: DiagnosticBuilder<'_>, span_msg: Option| { + if let Some(span_msg) = span_msg { + err.span_label(self.span, span_msg); + } + // Add spans for the stacktrace. Don't print a single-line backtrace though. + if self.stacktrace.len() > 1 { + for frame_info in &self.stacktrace { + err.span_label(frame_info.span, frame_info.to_string()); + } + } + // Let the caller finish the job. + emit(err) + }; + + if must_error { + // The `message` makes little sense here, this is a more serious error than the + // caller thinks anyway. + // See . + finish(struct_error(tcx, &err_msg), None); + ErrorHandled::Reported(ErrorReported) + } else { + // Regular case. + if let Some(lint_root) = lint_root { + // Report as lint. + let hir_id = self + .stacktrace + .iter() + .rev() + .find_map(|frame| frame.lint_root) + .unwrap_or(lint_root); + tcx.struct_span_lint_hir( + rustc_session::lint::builtin::CONST_ERR, + hir_id, + tcx.span, + |lint| finish(lint.build(message), Some(err_msg)), + ); + ErrorHandled::Linted + } else { + // Report as hard error. + finish(struct_error(tcx, message), Some(err_msg)); + ErrorHandled::Reported(ErrorReported) + } + } + } } diff --git a/src/librustc_mir/const_eval/eval_queries.rs b/src/librustc_mir/const_eval/eval_queries.rs index 42fba8982d23..7fbe5c409d3c 100644 --- a/src/librustc_mir/const_eval/eval_queries.rs +++ b/src/librustc_mir/const_eval/eval_queries.rs @@ -1,13 +1,14 @@ -use super::{error_to_const_error, CompileTimeEvalContext, CompileTimeInterpreter, MemoryExtra}; +use super::{CompileTimeEvalContext, CompileTimeInterpreter, ConstEvalErr, MemoryExtra}; use crate::interpret::eval_nullary_intrinsic; use crate::interpret::{ intern_const_alloc_recursive, Allocation, ConstValue, GlobalId, Immediate, InternKind, InterpCx, InterpResult, MPlaceTy, MemoryKind, OpTy, RawConst, RefTracking, Scalar, ScalarMaybeUninit, StackPopCleanup, }; + use rustc_hir::def::DefKind; use rustc_middle::mir; -use rustc_middle::mir::interpret::{ConstEvalErr, ErrorHandled}; +use rustc_middle::mir::interpret::ErrorHandled; use rustc_middle::traits::Reveal; use rustc_middle::ty::{self, subst::Subst, TyCtxt}; use rustc_span::source_map::Span; @@ -213,7 +214,7 @@ fn validate_and_turn_into_const<'tcx>( })(); val.map_err(|error| { - let err = error_to_const_error(&ecx, error, None); + let err = ConstEvalErr::new(&ecx, error, None); err.struct_error(ecx.tcx, "it is undefined behavior to use this value", |mut diag| { diag.note(note_on_undefined_behavior_error()); diag.emit(); @@ -312,7 +313,7 @@ pub fn const_eval_raw_provider<'tcx>( res.and_then(|body| eval_body_using_ecx(&mut ecx, cid, &body)) .map(|place| RawConst { alloc_id: place.ptr.assert_ptr().alloc_id, ty: place.layout.ty }) .map_err(|error| { - let err = error_to_const_error(&ecx, error, None); + let err = ConstEvalErr::new(&ecx, error, None); // errors in statics are always emitted as fatal errors if is_static { // Ensure that if the above error was either `TooGeneric` or `Reported` diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs index 1e9be0978150..fc192cd9b3cb 100644 --- a/src/librustc_mir/interpret/eval_context.rs +++ b/src/librustc_mir/interpret/eval_context.rs @@ -1,22 +1,22 @@ use std::cell::Cell; +use std::fmt; use std::mem; use rustc_data_structures::fx::FxHashMap; use rustc_data_structures::stable_hasher::{HashStable, StableHasher}; -use rustc_hir::def::DefKind; -use rustc_hir::def_id::DefId; +use rustc_hir::{self as hir, def::DefKind, def_id::DefId, definitions::DefPathData}; use rustc_index::vec::IndexVec; use rustc_macros::HashStable; use rustc_middle::ich::StableHashingContext; use rustc_middle::mir; use rustc_middle::mir::interpret::{ - sign_extend, truncate, FrameInfo, GlobalId, InterpResult, Pointer, Scalar, + sign_extend, truncate, GlobalId, InterpResult, Pointer, Scalar, }; use rustc_middle::ty::layout::{self, TyAndLayout}; use rustc_middle::ty::{ self, query::TyCtxtAt, subst::SubstsRef, ParamEnv, Ty, TyCtxt, TypeFoldable, }; -use rustc_span::{source_map::DUMMY_SP, Span}; +use rustc_span::{source_map::DUMMY_SP, Pos, Span}; use rustc_target::abi::{Align, HasDataLayout, LayoutOf, Size, TargetDataLayout}; use super::{ @@ -88,6 +88,14 @@ pub struct Frame<'mir, 'tcx, Tag = (), Extra = ()> { pub loc: Option, } +/// What we store about a frame in an interpreter backtrace. +#[derive(Debug)] +pub struct FrameInfo<'tcx> { + pub instance: ty::Instance<'tcx>, + pub span: Span, + pub lint_root: Option, +} + #[derive(Clone, Eq, PartialEq, Debug, HashStable)] // Miri debug-prints these pub enum StackPopCleanup { /// Jump to the next block in the caller, or cause UB if None (that's a function @@ -185,6 +193,25 @@ impl<'mir, 'tcx, Tag, Extra> Frame<'mir, 'tcx, Tag, Extra> { } } +impl<'tcx> fmt::Display for FrameInfo<'tcx> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + ty::tls::with(|tcx| { + if tcx.def_key(self.instance.def_id()).disambiguated_data.data + == DefPathData::ClosureExpr + { + write!(f, "inside closure")?; + } else { + write!(f, "inside `{}`", self.instance)?; + } + if !self.span.is_dummy() { + let lo = tcx.sess.source_map().lookup_char_pos(self.span.lo()); + write!(f, " at {}:{}:{}", lo.file.name, lo.line, lo.col.to_usize() + 1)?; + } + Ok(()) + }) + } +} + impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> HasDataLayout for InterpCx<'mir, 'tcx, M> { #[inline] fn data_layout(&self) -> &TargetDataLayout { diff --git a/src/librustc_mir/interpret/mod.rs b/src/librustc_mir/interpret/mod.rs index ebb061f48518..5218b03d65ec 100644 --- a/src/librustc_mir/interpret/mod.rs +++ b/src/librustc_mir/interpret/mod.rs @@ -18,7 +18,7 @@ mod visitor; pub use rustc_middle::mir::interpret::*; // have all the `interpret` symbols in one place: here -pub use self::eval_context::{Frame, InterpCx, LocalState, LocalValue, StackPopCleanup}; +pub use self::eval_context::{Frame, FrameInfo, InterpCx, LocalState, LocalValue, StackPopCleanup}; pub use self::intern::{intern_const_alloc_recursive, InternKind}; pub use self::machine::{compile_time_machine, AllocMap, Machine, MayLeak, StackPopJump}; pub use self::memory::{get_static, AllocCheck, FnVal, Memory, MemoryKind}; diff --git a/src/librustc_mir/transform/const_prop.rs b/src/librustc_mir/transform/const_prop.rs index db0b0415728a..bff83b3c6db7 100644 --- a/src/librustc_mir/transform/const_prop.rs +++ b/src/librustc_mir/transform/const_prop.rs @@ -26,7 +26,7 @@ use rustc_span::{def_id::DefId, Span}; use rustc_target::abi::{HasDataLayout, LayoutOf, Size, TargetDataLayout}; use rustc_trait_selection::traits; -use crate::const_eval::error_to_const_error; +use crate::const_eval::ConstEvalErr; use crate::interpret::{ self, compile_time_machine, truncate, AllocId, Allocation, Frame, ImmTy, Immediate, InterpCx, LocalState, LocalValue, MemPlace, Memory, MemoryKind, OpTy, Operand as InterpOperand, PlaceTy, @@ -451,7 +451,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { Ok(op) => Some(op), Err(error) => { let tcx = self.ecx.tcx.at(c.span); - let err = error_to_const_error(&self.ecx, error, Some(c.span)); + let err = ConstEvalErr::new(&self.ecx, error, Some(c.span)); if let Some(lint_root) = self.lint_root(source_info) { let lint_only = match c.literal.val { // Promoteds must lint and not error as the user didn't ask for them From fd41bdeff0ff56ad2e46b00ca55daafb68a8ea08 Mon Sep 17 00:00:00 2001 From: David Wood Date: Sun, 9 Aug 2020 14:53:33 +0100 Subject: [PATCH 28/37] instance: only polymorphize upvar substs This commit restricts the substitution polymorphization added in #75255 to only apply to the tupled upvar substitution, rather than all substitutions, fixing a bunch of regressions when polymorphization is enabled. Signed-off-by: David Wood --- src/librustc_middle/ty/flags.rs | 6 --- src/librustc_middle/ty/fold.rs | 6 --- src/librustc_middle/ty/instance.rs | 53 ++++++++++++------- src/librustc_middle/ty/mod.rs | 4 -- .../polymorphization/pr-75255.rs | 52 ------------------ 5 files changed, 33 insertions(+), 88 deletions(-) delete mode 100644 src/test/codegen-units/polymorphization/pr-75255.rs diff --git a/src/librustc_middle/ty/flags.rs b/src/librustc_middle/ty/flags.rs index 81f7474962c8..27f50c240db6 100644 --- a/src/librustc_middle/ty/flags.rs +++ b/src/librustc_middle/ty/flags.rs @@ -85,8 +85,6 @@ impl FlagComputation { } &ty::Generator(_, ref substs, _) => { - self.add_flags(TypeFlags::MAY_POLYMORPHIZE); - let substs = substs.as_generator(); let should_remove_further_specializable = !self.flags.contains(TypeFlags::STILL_FURTHER_SPECIALIZABLE); @@ -109,8 +107,6 @@ impl FlagComputation { } &ty::Closure(_, substs) => { - self.add_flags(TypeFlags::MAY_POLYMORPHIZE); - let substs = substs.as_closure(); let should_remove_further_specializable = !self.flags.contains(TypeFlags::STILL_FURTHER_SPECIALIZABLE); @@ -196,8 +192,6 @@ impl FlagComputation { } &ty::FnDef(_, substs) => { - self.add_flags(TypeFlags::MAY_POLYMORPHIZE); - self.add_substs(substs); } diff --git a/src/librustc_middle/ty/fold.rs b/src/librustc_middle/ty/fold.rs index 87434f3f2677..492f8ce9ef1a 100644 --- a/src/librustc_middle/ty/fold.rs +++ b/src/librustc_middle/ty/fold.rs @@ -150,12 +150,6 @@ pub trait TypeFoldable<'tcx>: fmt::Debug + Clone { self.has_type_flags(TypeFlags::STILL_FURTHER_SPECIALIZABLE) } - /// Does this value contain closures, generators or functions such that it may require - /// polymorphization? - fn may_polymorphize(&self) -> bool { - self.has_type_flags(TypeFlags::MAY_POLYMORPHIZE) - } - /// A visitor that does not recurse into types, works like `fn walk_shallow` in `Ty`. fn visit_tys_shallow(&self, visit: impl FnMut(Ty<'tcx>) -> bool) -> bool { pub struct Visitor(F); diff --git a/src/librustc_middle/ty/instance.rs b/src/librustc_middle/ty/instance.rs index cf876db26bc7..2def000da648 100644 --- a/src/librustc_middle/ty/instance.rs +++ b/src/librustc_middle/ty/instance.rs @@ -492,6 +492,20 @@ fn polymorphize<'tcx>( let unused = tcx.unused_generic_params(def_id); debug!("polymorphize: unused={:?}", unused); + // If this is a closure or generator then we need to handle the case where another closure + // from the function is captured as an upvar and hasn't been polymorphized. In this case, + // the unpolymorphized upvar closure would result in a polymorphized closure producing + // multiple mono items (and eventually symbol clashes). + let upvars_ty = if tcx.is_closure(def_id) { + Some(substs.as_closure().tupled_upvars_ty()) + } else if tcx.type_of(def_id).is_generator() { + Some(substs.as_generator().tupled_upvars_ty()) + } else { + None + }; + let has_upvars = upvars_ty.map(|ty| ty.tuple_fields().count() > 0).unwrap_or(false); + debug!("polymorphize: upvars_ty={:?} has_upvars={:?}", upvars_ty, has_upvars); + struct PolymorphizationFolder<'tcx> { tcx: TyCtxt<'tcx>, }; @@ -512,14 +526,6 @@ fn polymorphize<'tcx>( self.tcx.mk_closure(def_id, polymorphized_substs) } } - ty::FnDef(def_id, substs) => { - let polymorphized_substs = polymorphize(self.tcx, def_id, substs); - if substs == polymorphized_substs { - ty - } else { - self.tcx.mk_fn_def(def_id, polymorphized_substs) - } - } ty::Generator(def_id, substs, movability) => { let polymorphized_substs = polymorphize(self.tcx, def_id, substs); if substs == polymorphized_substs { @@ -537,24 +543,31 @@ fn polymorphize<'tcx>( let is_unused = unused.contains(param.index).unwrap_or(false); debug!("polymorphize: param={:?} is_unused={:?}", param, is_unused); match param.kind { - // If parameter is a const or type parameter.. + // Upvar case: If parameter is a type parameter.. + ty::GenericParamDefKind::Type { .. } if + // ..and has upvars.. + has_upvars && + // ..and this param has the same type as the tupled upvars.. + upvars_ty == Some(substs[param.index as usize].expect_ty()) => { + // ..then double-check that polymorphization marked it used.. + debug_assert!(!is_unused); + // ..and polymorphize any closures/generators captured as upvars. + let upvars_ty = upvars_ty.unwrap(); + let polymorphized_upvars_ty = upvars_ty.fold_with( + &mut PolymorphizationFolder { tcx }); + debug!("polymorphize: polymorphized_upvars_ty={:?}", polymorphized_upvars_ty); + ty::GenericArg::from(polymorphized_upvars_ty) + }, + + // Simple case: If parameter is a const or type parameter.. ty::GenericParamDefKind::Const | ty::GenericParamDefKind::Type { .. } if // ..and is within range and unused.. unused.contains(param.index).unwrap_or(false) => // ..then use the identity for this parameter. tcx.mk_param_from_def(param), - // If the parameter does not contain any closures or generators, then use the - // substitution directly. - _ if !substs.may_polymorphize() => substs[param.index as usize], - - // Otherwise, use the substitution after polymorphizing. - _ => { - let arg = substs[param.index as usize]; - let polymorphized_arg = arg.fold_with(&mut PolymorphizationFolder { tcx }); - debug!("polymorphize: arg={:?} polymorphized_arg={:?}", arg, polymorphized_arg); - ty::GenericArg::from(polymorphized_arg) - } + // Otherwise, use the parameter as before. + _ => substs[param.index as usize], } }) } diff --git a/src/librustc_middle/ty/mod.rs b/src/librustc_middle/ty/mod.rs index 0102225b9b56..6798addb8aaa 100644 --- a/src/librustc_middle/ty/mod.rs +++ b/src/librustc_middle/ty/mod.rs @@ -575,10 +575,6 @@ bitflags! { /// Does this value have parameters/placeholders/inference variables which could be /// replaced later, in a way that would change the results of `impl` specialization? const STILL_FURTHER_SPECIALIZABLE = 1 << 17; - - /// Does this value contain closures, generators or functions such that it may require - /// polymorphization? - const MAY_POLYMORPHIZE = 1 << 18; } } diff --git a/src/test/codegen-units/polymorphization/pr-75255.rs b/src/test/codegen-units/polymorphization/pr-75255.rs deleted file mode 100644 index af47b440640a..000000000000 --- a/src/test/codegen-units/polymorphization/pr-75255.rs +++ /dev/null @@ -1,52 +0,0 @@ -// compile-flags:-Zpolymorphize=on -Zprint-mono-items=lazy -Copt-level=1 -// ignore-tidy-linelength - -#![crate_type = "rlib"] - -// Test that only one copy of `Iter::map` and `iter::repeat` are generated. - -fn unused() -> u64 { - 42 -} - -fn foo() { - let x = [1, 2, 3, std::mem::size_of::()]; - x.iter().map(|_| ()); -} - -//~ MONO_ITEM fn core::iter[0]::adapters[0]::{{impl}}[29]::new[0], pr_75255::foo[0]::{{closure}}[0]> @@ pr_75255-cgu.0[External] -//~ MONO_ITEM fn core::iter[0]::traits[0]::iterator[0]::Iterator[0]::map[0], (), pr_75255::foo[0]::{{closure}}[0]> @@ pr_75255-cgu.1[Internal] - -fn bar() { - std::iter::repeat(unused::); -} - -//~ MONO_ITEM fn core::iter[0]::sources[0]::repeat[0] u64> @@ pr_75255-cgu.1[Internal] - -pub fn dispatch() { - foo::(); - foo::>(); - - bar::(); - bar::>(); -} - -// These are all the items that aren't relevant to the test. -//~ MONO_ITEM fn core::mem[0]::size_of[0] @@ pr_75255-cgu.1[Internal] -//~ MONO_ITEM fn core::mem[0]::size_of[0]> @@ pr_75255-cgu.1[Internal] -//~ MONO_ITEM fn core::mem[0]::size_of[0] @@ pr_75255-cgu.1[Internal] -//~ MONO_ITEM fn core::ptr[0]::const_ptr[0]::{{impl}}[0]::add[0] @@ pr_75255-cgu.1[Internal] -//~ MONO_ITEM fn core::ptr[0]::const_ptr[0]::{{impl}}[0]::is_null[0] @@ pr_75255-cgu.1[Internal] -//~ MONO_ITEM fn core::ptr[0]::const_ptr[0]::{{impl}}[0]::offset[0] @@ pr_75255-cgu.1[Internal] -//~ MONO_ITEM fn core::ptr[0]::const_ptr[0]::{{impl}}[0]::wrapping_add[0] @@ pr_75255-cgu.1[Internal] -//~ MONO_ITEM fn core::ptr[0]::const_ptr[0]::{{impl}}[0]::wrapping_offset[0] @@ pr_75255-cgu.1[Internal] -//~ MONO_ITEM fn core::ptr[0]::non_null[0]::{{impl}}[3]::new_unchecked[0] @@ pr_75255-cgu.1[Internal] -//~ MONO_ITEM fn core::ptr[0]::null[0] @@ pr_75255-cgu.1[Internal] -//~ MONO_ITEM fn core::slice[0]::{{impl}}[0]::as_ptr[0] @@ pr_75255-cgu.1[Internal] -//~ MONO_ITEM fn core::slice[0]::{{impl}}[0]::iter[0] @@ pr_75255-cgu.1[Internal] -//~ MONO_ITEM fn core::slice[0]::{{impl}}[0]::len[0] @@ pr_75255-cgu.1[Internal] -//~ MONO_ITEM fn pr_75255::dispatch[0] @@ pr_75255-cgu.1[External] -//~ MONO_ITEM fn pr_75255::foo[0] @@ pr_75255-cgu.1[Internal] -//~ MONO_ITEM fn pr_75255::foo[0]> @@ pr_75255-cgu.1[Internal] -//~ MONO_ITEM fn pr_75255::bar[0] @@ pr_75255-cgu.1[Internal] -//~ MONO_ITEM fn pr_75255::bar[0]> @@ pr_75255-cgu.1[Internal] From a0e057540eb996ea43ba5e0dce2b69fa972f718f Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 9 Aug 2020 17:47:29 +0200 Subject: [PATCH 29/37] evaluate required_consts when pushing stack frame in Miri engine --- src/librustc_mir/interpret/eval_context.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs index fc192cd9b3cb..7c2f749c1567 100644 --- a/src/librustc_mir/interpret/eval_context.rs +++ b/src/librustc_mir/interpret/eval_context.rs @@ -652,6 +652,13 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { let frame = M::init_frame_extra(self, pre_frame)?; self.stack_mut().push(frame); + // Make sure all the constants required by this frame evaluate successfully (post-monomorphization check). + for const_ in &body.required_consts { + let const_ = + self.subst_from_current_frame_and_normalize_erasing_regions(const_.literal); + self.const_to_op(const_, None)?; + } + // Locals are initially uninitialized. let dummy = LocalState { value: LocalValue::Uninitialized, layout: Cell::new(None) }; let mut locals = IndexVec::from_elem(dummy, &body.local_decls); From 1fa720316101b0f9c9537042ff487064ed3b6dda Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 9 Aug 2020 18:54:56 +0200 Subject: [PATCH 30/37] bless MIR --- ...allocation2.main.ConstProp.after.mir.32bit | 30 ++++++++--------- ...allocation2.main.ConstProp.after.mir.64bit | 32 +++++++++---------- 2 files changed, 31 insertions(+), 31 deletions(-) diff --git a/src/test/mir-opt/const_allocation2.main.ConstProp.after.mir.32bit b/src/test/mir-opt/const_allocation2.main.ConstProp.after.mir.32bit index d386d2478292..71d55dbb96e1 100644 --- a/src/test/mir-opt/const_allocation2.main.ConstProp.after.mir.32bit +++ b/src/test/mir-opt/const_allocation2.main.ConstProp.after.mir.32bit @@ -30,41 +30,41 @@ fn main() -> () { } alloc0 (static: FOO, size: 8, align: 4) { - ╾─alloc21─╼ 03 00 00 00 │ ╾──╼.... + ╾─alloc23─╼ 03 00 00 00 │ ╾──╼.... } -alloc21 (size: 48, align: 4) { - 0x00 │ 00 00 00 00 __ __ __ __ ╾─alloc4──╼ 00 00 00 00 │ ....░░░░╾──╼.... - 0x10 │ 00 00 00 00 __ __ __ __ ╾─alloc9──╼ 02 00 00 00 │ ....░░░░╾──╼.... - 0x20 │ 01 00 00 00 2a 00 00 00 ╾─alloc19─╼ 03 00 00 00 │ ....*...╾──╼.... +alloc23 (size: 48, align: 4) { + 0x00 │ 00 00 00 00 __ __ __ __ ╾─alloc8──╼ 00 00 00 00 │ ....░░░░╾──╼.... + 0x10 │ 00 00 00 00 __ __ __ __ ╾─alloc13─╼ 02 00 00 00 │ ....░░░░╾──╼.... + 0x20 │ 01 00 00 00 2a 00 00 00 ╾─alloc21─╼ 03 00 00 00 │ ....*...╾──╼.... } -alloc4 (size: 0, align: 4) {} +alloc8 (size: 0, align: 4) {} -alloc9 (size: 8, align: 4) { - ╾─alloc7──╼ ╾─alloc8──╼ │ ╾──╼╾──╼ +alloc13 (size: 8, align: 4) { + ╾─alloc11─╼ ╾─alloc12─╼ │ ╾──╼╾──╼ } -alloc7 (size: 1, align: 1) { +alloc11 (size: 1, align: 1) { 05 │ . } -alloc8 (size: 1, align: 1) { +alloc12 (size: 1, align: 1) { 06 │ . } -alloc19 (size: 12, align: 4) { - ╾─a15+0x3─╼ ╾─alloc16─╼ ╾─a18+0x2─╼ │ ╾──╼╾──╼╾──╼ +alloc21 (size: 12, align: 4) { + ╾─a17+0x3─╼ ╾─alloc18─╼ ╾─a20+0x2─╼ │ ╾──╼╾──╼╾──╼ } -alloc15 (size: 4, align: 1) { +alloc17 (size: 4, align: 1) { 2a 45 15 6f │ *E.o } -alloc16 (size: 1, align: 1) { +alloc18 (size: 1, align: 1) { 2a │ * } -alloc18 (size: 4, align: 1) { +alloc20 (size: 4, align: 1) { 2a 45 15 6f │ *E.o } diff --git a/src/test/mir-opt/const_allocation2.main.ConstProp.after.mir.64bit b/src/test/mir-opt/const_allocation2.main.ConstProp.after.mir.64bit index d7acd0f0f433..79bad7ea9262 100644 --- a/src/test/mir-opt/const_allocation2.main.ConstProp.after.mir.64bit +++ b/src/test/mir-opt/const_allocation2.main.ConstProp.after.mir.64bit @@ -30,44 +30,44 @@ fn main() -> () { } alloc0 (static: FOO, size: 16, align: 8) { - ╾───────alloc21───────╼ 03 00 00 00 00 00 00 00 │ ╾──────╼........ + ╾───────alloc23───────╼ 03 00 00 00 00 00 00 00 │ ╾──────╼........ } -alloc21 (size: 72, align: 8) { - 0x00 │ 00 00 00 00 __ __ __ __ ╾───────alloc4────────╼ │ ....░░░░╾──────╼ +alloc23 (size: 72, align: 8) { + 0x00 │ 00 00 00 00 __ __ __ __ ╾───────alloc8────────╼ │ ....░░░░╾──────╼ 0x10 │ 00 00 00 00 00 00 00 00 00 00 00 00 __ __ __ __ │ ............░░░░ - 0x20 │ ╾───────alloc9────────╼ 02 00 00 00 00 00 00 00 │ ╾──────╼........ - 0x30 │ 01 00 00 00 2a 00 00 00 ╾───────alloc19───────╼ │ ....*...╾──────╼ + 0x20 │ ╾───────alloc13───────╼ 02 00 00 00 00 00 00 00 │ ╾──────╼........ + 0x30 │ 01 00 00 00 2a 00 00 00 ╾───────alloc21───────╼ │ ....*...╾──────╼ 0x40 │ 03 00 00 00 00 00 00 00 │ ........ } -alloc4 (size: 0, align: 8) {} +alloc8 (size: 0, align: 8) {} -alloc9 (size: 16, align: 8) { - ╾───────alloc7────────╼ ╾───────alloc8────────╼ │ ╾──────╼╾──────╼ +alloc13 (size: 16, align: 8) { + ╾───────alloc11───────╼ ╾───────alloc12───────╼ │ ╾──────╼╾──────╼ } -alloc7 (size: 1, align: 1) { +alloc11 (size: 1, align: 1) { 05 │ . } -alloc8 (size: 1, align: 1) { +alloc12 (size: 1, align: 1) { 06 │ . } -alloc19 (size: 24, align: 8) { - 0x00 │ ╾─────alloc15+0x3─────╼ ╾───────alloc16───────╼ │ ╾──────╼╾──────╼ - 0x10 │ ╾─────alloc18+0x2─────╼ │ ╾──────╼ +alloc21 (size: 24, align: 8) { + 0x00 │ ╾─────alloc17+0x3─────╼ ╾───────alloc18───────╼ │ ╾──────╼╾──────╼ + 0x10 │ ╾─────alloc20+0x2─────╼ │ ╾──────╼ } -alloc15 (size: 4, align: 1) { +alloc17 (size: 4, align: 1) { 2a 45 15 6f │ *E.o } -alloc16 (size: 1, align: 1) { +alloc18 (size: 1, align: 1) { 2a │ * } -alloc18 (size: 4, align: 1) { +alloc20 (size: 4, align: 1) { 2a 45 15 6f │ *E.o } From 7dba693f0e1197098ff10ac8812e1681cc79297e Mon Sep 17 00:00:00 2001 From: Denis Vasilik Date: Sun, 9 Aug 2020 23:06:44 +0200 Subject: [PATCH 31/37] Use intra-doc links --- library/core/src/borrow.rs | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/library/core/src/borrow.rs b/library/core/src/borrow.rs index 3e533255becb..8d52fb2ad8f4 100644 --- a/library/core/src/borrow.rs +++ b/library/core/src/borrow.rs @@ -40,14 +40,13 @@ /// provide a reference to related type `T`, it is often better to use /// [`AsRef`] as more types can safely implement it. /// -/// [`AsRef`]: ../../std/convert/trait.AsRef.html -/// [`BorrowMut`]: trait.BorrowMut.html +/// [`AsRef`]: crate::convert::AsRef +/// [`BorrowMut`]: BorrowMut /// [`Box`]: ../../std/boxed/struct.Box.html /// [`Mutex`]: ../../std/sync/struct.Mutex.html /// [`Rc`]: ../../std/rc/struct.Rc.html -/// [`str`]: ../../std/primitive.str.html /// [`String`]: ../../std/string/struct.String.html -/// [`borrow`]: #tymethod.borrow +/// [`borrow`]: Borrow::borrow /// /// # Examples /// @@ -152,10 +151,9 @@ /// If it wants to allow others access to the underlying `str`, it can do /// that via `AsRef` which doesn’t carry any extra requirements. /// -/// [`Hash`]: ../../std/hash/trait.Hash.html +/// [`Hash`]: crate::hash::Hash /// [`HashMap`]: ../../std/collections/struct.HashMap.html /// [`String`]: ../../std/string/struct.String.html -/// [`str`]: ../../std/primitive.str.html #[stable(feature = "rust1", since = "1.0.0")] pub trait Borrow { /// Immutably borrows from an owned value. @@ -187,7 +185,7 @@ pub trait Borrow { /// an underlying type by providing a mutable reference. See [`Borrow`] /// for more information on borrowing as another type. /// -/// [`Borrow`]: trait.Borrow.html +/// [`Borrow`]: Borrow #[stable(feature = "rust1", since = "1.0.0")] pub trait BorrowMut: Borrow { /// Mutably borrows from an owned value. From fd3851aadb3f5326480f9e9ced57ade2db86480b Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 10 Aug 2020 10:58:53 +0200 Subject: [PATCH 32/37] add test for unused erroneous const in CTFE --- .../ui/consts/const-eval/erroneous-const.rs | 20 +++++++++ .../consts/const-eval/erroneous-const.stderr | 43 +++++++++++++++++++ 2 files changed, 63 insertions(+) create mode 100644 src/test/ui/consts/const-eval/erroneous-const.rs create mode 100644 src/test/ui/consts/const-eval/erroneous-const.stderr diff --git a/src/test/ui/consts/const-eval/erroneous-const.rs b/src/test/ui/consts/const-eval/erroneous-const.rs new file mode 100644 index 000000000000..93c4e9372e8f --- /dev/null +++ b/src/test/ui/consts/const-eval/erroneous-const.rs @@ -0,0 +1,20 @@ +//! Make sure we error on erroneous consts even if they are unused. +#![warn(const_err, unconditional_panic)] + +struct PrintName(T); +impl PrintName { + const VOID: () = [()][2]; //~WARN any use of this value will cause an error + //~^ WARN this operation will panic at runtime +} + +const fn no_codegen() { + if false { //~ERROR evaluation of constant value failed + let _ = PrintName::::VOID; + } +} + +pub static FOO: () = no_codegen::(); //~ERROR could not evaluate static initializer + +fn main() { + FOO +} diff --git a/src/test/ui/consts/const-eval/erroneous-const.stderr b/src/test/ui/consts/const-eval/erroneous-const.stderr new file mode 100644 index 000000000000..da7e7247d50f --- /dev/null +++ b/src/test/ui/consts/const-eval/erroneous-const.stderr @@ -0,0 +1,43 @@ +warning: this operation will panic at runtime + --> $DIR/erroneous-const.rs:6:22 + | +LL | const VOID: () = [()][2]; + | ^^^^^^^ index out of bounds: the len is 1 but the index is 2 + | +note: the lint level is defined here + --> $DIR/erroneous-const.rs:2:20 + | +LL | #![warn(const_err, unconditional_panic)] + | ^^^^^^^^^^^^^^^^^^^ + +warning: any use of this value will cause an error + --> $DIR/erroneous-const.rs:6:22 + | +LL | const VOID: () = [()][2]; + | -----------------^^^^^^^- + | | + | index out of bounds: the len is 1 but the index is 2 + | +note: the lint level is defined here + --> $DIR/erroneous-const.rs:2:9 + | +LL | #![warn(const_err, unconditional_panic)] + | ^^^^^^^^^ + +error[E0080]: evaluation of constant value failed + --> $DIR/erroneous-const.rs:11:5 + | +LL | / if false { +LL | | let _ = PrintName::::VOID; +LL | | } + | |_____^ referenced constant has errors + +error[E0080]: could not evaluate static initializer + --> $DIR/erroneous-const.rs:16:22 + | +LL | pub static FOO: () = no_codegen::(); + | ^^^^^^^^^^^^^^^^^^^ referenced constant has errors + +error: aborting due to 2 previous errors; 2 warnings emitted + +For more information about this error, try `rustc --explain E0080`. From 4ed0c6a464e89116d862773320aeab50464acd5f Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Mon, 10 Aug 2020 08:05:43 -0400 Subject: [PATCH 33/37] Use existing `infcx` when emitting trait impl diagnostic Fixes #75361 Fixes #74918 Previously, we were creating a new `InferCtxt`, which caused an ICE when used with type variables from the existing `InferCtxt` --- .../trait_impl_difference.rs | 8 ++--- .../issue-74918-missing-lifetime.rs | 28 +++++++++++++++++ .../issue-74918-missing-lifetime.stderr | 30 +++++++++++++++++++ .../issue-75361-mismatched-impl.rs | 24 +++++++++++++++ .../issue-75361-mismatched-impl.stderr | 19 ++++++++++++ 5 files changed, 104 insertions(+), 5 deletions(-) create mode 100644 src/test/ui/mismatched_types/issue-74918-missing-lifetime.rs create mode 100644 src/test/ui/mismatched_types/issue-74918-missing-lifetime.stderr create mode 100644 src/test/ui/mismatched_types/issue-75361-mismatched-impl.rs create mode 100644 src/test/ui/mismatched_types/issue-75361-mismatched-impl.stderr diff --git a/src/librustc_infer/infer/error_reporting/nice_region_error/trait_impl_difference.rs b/src/librustc_infer/infer/error_reporting/nice_region_error/trait_impl_difference.rs index 45aee2b39654..1ddf88c03066 100644 --- a/src/librustc_infer/infer/error_reporting/nice_region_error/trait_impl_difference.rs +++ b/src/librustc_infer/infer/error_reporting/nice_region_error/trait_impl_difference.rs @@ -2,7 +2,7 @@ use crate::infer::error_reporting::nice_region_error::NiceRegionError; use crate::infer::lexical_region_resolve::RegionResolutionError; -use crate::infer::{Subtype, TyCtxtInferExt, ValuePairs}; +use crate::infer::{Subtype, ValuePairs}; use crate::traits::ObligationCauseCode::CompareImplMethodObligation; use rustc_errors::ErrorReported; use rustc_hir as hir; @@ -53,7 +53,6 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> { } fn emit_err(&self, sp: Span, expected: Ty<'tcx>, found: Ty<'tcx>, trait_def_id: DefId) { - let tcx = self.tcx(); let trait_sp = self.tcx().def_span(trait_def_id); let mut err = self .tcx() @@ -85,9 +84,8 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> { ); } - if let Some((expected, found)) = tcx - .infer_ctxt() - .enter(|infcx| infcx.expected_found_str_ty(&ExpectedFound { expected, found })) + if let Some((expected, found)) = + self.infcx.expected_found_str_ty(&ExpectedFound { expected, found }) { // Highlighted the differences when showing the "expected/found" note. err.note_expected_found(&"", expected, &"", found); diff --git a/src/test/ui/mismatched_types/issue-74918-missing-lifetime.rs b/src/test/ui/mismatched_types/issue-74918-missing-lifetime.rs new file mode 100644 index 000000000000..0e3ea4bc8c9d --- /dev/null +++ b/src/test/ui/mismatched_types/issue-74918-missing-lifetime.rs @@ -0,0 +1,28 @@ +// Regression test for issue #74918 +// Tests that we don't ICE after emitting an error + +struct ChunkingIterator> { + source: S, +} + +impl> Iterator for ChunkingIterator { + type Item = IteratorChunk; //~ ERROR missing lifetime + + fn next(&mut self) -> Option> { //~ ERROR `impl` + todo!() + } +} + +struct IteratorChunk<'a, T, S: Iterator> { + source: &'a mut S, +} + +impl> Iterator for IteratorChunk<'_, T, S> { + type Item = T; + + fn next(&mut self) -> Option { + todo!() + } +} + +fn main() {} diff --git a/src/test/ui/mismatched_types/issue-74918-missing-lifetime.stderr b/src/test/ui/mismatched_types/issue-74918-missing-lifetime.stderr new file mode 100644 index 000000000000..da3056eac900 --- /dev/null +++ b/src/test/ui/mismatched_types/issue-74918-missing-lifetime.stderr @@ -0,0 +1,30 @@ +error[E0106]: missing lifetime specifier + --> $DIR/issue-74918-missing-lifetime.rs:9:31 + | +LL | type Item = IteratorChunk; + | ^ expected named lifetime parameter + | +help: consider introducing a named lifetime parameter + | +LL | type Item<'a> = IteratorChunk<<'a>T, S>; + | ^^^^ ^^^^ + +error: `impl` item signature doesn't match `trait` item signature + --> $DIR/issue-74918-missing-lifetime.rs:11:5 + | +LL | fn next(&mut self) -> Option> { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ found `fn(&mut ChunkingIterator) -> std::option::Option>` + | + ::: $SRC_DIR/core/src/iter/traits/iterator.rs:LL:COL + | +LL | fn next(&mut self) -> Option; + | ----------------------------------------- expected `fn(&mut ChunkingIterator) -> std::option::Option>` + | + = note: expected `fn(&mut ChunkingIterator) -> std::option::Option>` + found `fn(&mut ChunkingIterator) -> std::option::Option>` + = help: the lifetime requirements from the `impl` do not correspond to the requirements in the `trait` + = help: verify the lifetime relationships in the `trait` and `impl` between the `self` argument, the other inputs and its output + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0106`. diff --git a/src/test/ui/mismatched_types/issue-75361-mismatched-impl.rs b/src/test/ui/mismatched_types/issue-75361-mismatched-impl.rs new file mode 100644 index 000000000000..4410514476dc --- /dev/null +++ b/src/test/ui/mismatched_types/issue-75361-mismatched-impl.rs @@ -0,0 +1,24 @@ +// Regresison test for issue #75361 +// Tests that we don't ICE on mismatched types with inference variables + + +trait MyTrait { + type Item; +} + +pub trait Graph { + type EdgeType; + + fn adjacent_edges(&self) -> Box>; +} + +impl Graph for T { + type EdgeType = T; + + fn adjacent_edges(&self) -> Box + '_> { //~ ERROR `impl` + panic!() + } + +} + +fn main() {} diff --git a/src/test/ui/mismatched_types/issue-75361-mismatched-impl.stderr b/src/test/ui/mismatched_types/issue-75361-mismatched-impl.stderr new file mode 100644 index 000000000000..5be7f5271dee --- /dev/null +++ b/src/test/ui/mismatched_types/issue-75361-mismatched-impl.stderr @@ -0,0 +1,19 @@ +error: `impl` item signature doesn't match `trait` item signature + --> $DIR/issue-75361-mismatched-impl.rs:18:3 + | +LL | fn adjacent_edges(&self) -> Box>; + | --------------------------------------------------------------------- expected `fn(&T) -> std::boxed::Box<(dyn MyTrait + 'static)>` +... +LL | fn adjacent_edges(&self) -> Box + '_> { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ found `fn(&T) -> std::boxed::Box>` + | + = note: expected `fn(&T) -> std::boxed::Box<(dyn MyTrait + 'static)>` + found `fn(&T) -> std::boxed::Box>` +help: the lifetime requirements from the `impl` do not correspond to the requirements in the `trait` + --> $DIR/issue-75361-mismatched-impl.rs:12:55 + | +LL | fn adjacent_edges(&self) -> Box>; + | ^^^^^^^^^^^^^^ consider borrowing this type parameter in the trait + +error: aborting due to previous error + From a34bc7961ad6745d25be4e246ae99e16cc30a422 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Mon, 10 Aug 2020 16:42:11 +0200 Subject: [PATCH 34/37] Add help button --- src/librustdoc/html/layout.rs | 1 + src/librustdoc/html/static/main.js | 4 +++- src/librustdoc/html/static/rustdoc.css | 18 ++++++++++++++---- src/librustdoc/html/static/themes/ayu.css | 5 +++-- src/librustdoc/html/static/themes/dark.css | 5 +++-- src/librustdoc/html/static/themes/light.css | 5 +++-- 6 files changed, 27 insertions(+), 11 deletions(-) diff --git a/src/librustdoc/html/layout.rs b/src/librustdoc/html/layout.rs index cc6b38ebcdb7..8bef79e2521c 100644 --- a/src/librustdoc/html/layout.rs +++ b/src/librustdoc/html/layout.rs @@ -95,6 +95,7 @@ pub fn render( placeholder=\"Click or press ‘S’ to search, ‘?’ for more options…\" \ type=\"search\">\
\ + ? \ div { display: inline-flex; - width: calc(100% - 34px); + width: calc(100% - 63px); } #crate-search { margin-top: 5px; @@ -1250,14 +1250,24 @@ h4 > .important-traits { outline: none; } -#settings-menu { +#settings-menu, .help-button { position: absolute; - right: 0; top: 10px; +} + +#settings-menu { + right: 0; outline: none; } -#theme-picker, #settings-menu { +.help-button { + right: 30px; + font-family: "Fira Sans",sans-serif; + text-align: center; + font-size: 17px; +} + +#theme-picker, #settings-menu, .help-button { padding: 4px; width: 27px; height: 29px; diff --git a/src/librustdoc/html/static/themes/ayu.css b/src/librustdoc/html/static/themes/ayu.css index f4710f6ae873..94ee8a6f5bdb 100644 --- a/src/librustdoc/html/static/themes/ayu.css +++ b/src/librustdoc/html/static/themes/ayu.css @@ -489,7 +489,7 @@ kbd { box-shadow-color: #c6cbd1; } -#theme-picker, #settings-menu { +#theme-picker, #settings-menu, .help-button { border-color: #5c6773; background-color: #0f1419; } @@ -499,7 +499,8 @@ kbd { } #theme-picker:hover, #theme-picker:focus, -#settings-menu:hover, #settings-menu:focus { +#settings-menu:hover, #settings-menu:focus, +.help-button:hover, .help-button:focus { border-color: #e0e0e0; } diff --git a/src/librustdoc/html/static/themes/dark.css b/src/librustdoc/html/static/themes/dark.css index b3b586ba362f..c210216194ec 100644 --- a/src/librustdoc/html/static/themes/dark.css +++ b/src/librustdoc/html/static/themes/dark.css @@ -383,13 +383,14 @@ kbd { box-shadow-color: #c6cbd1; } -#theme-picker, #settings-menu { +#theme-picker, #settings-menu, .help-button { border-color: #e0e0e0; background: #f0f0f0; } #theme-picker:hover, #theme-picker:focus, -#settings-menu:hover, #settings-menu:focus { +#settings-menu:hover, #settings-menu:focus, +.help-button:hover, .help-button:focus { border-color: #ffb900; } diff --git a/src/librustdoc/html/static/themes/light.css b/src/librustdoc/html/static/themes/light.css index b0c5715604ba..150460f5c5d2 100644 --- a/src/librustdoc/html/static/themes/light.css +++ b/src/librustdoc/html/static/themes/light.css @@ -377,13 +377,14 @@ kbd { box-shadow-color: #c6cbd1; } -#theme-picker, #settings-menu { +#theme-picker, #settings-menu, .help-button { border-color: #e0e0e0; background-color: #fff; } #theme-picker:hover, #theme-picker:focus, -#settings-menu:hover, #settings-menu:focus { +#settings-menu:hover, #settings-menu:focus, +.help-button:hover, .help-button:focus { border-color: #717171; } From d7e72710853983372bf64aef905f1c273b75d365 Mon Sep 17 00:00:00 2001 From: Denis Vasilik Date: Mon, 10 Aug 2020 20:29:20 +0200 Subject: [PATCH 35/37] Remove AsRef link as it is in the prelude --- library/core/src/borrow.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/library/core/src/borrow.rs b/library/core/src/borrow.rs index 8d52fb2ad8f4..6f5a6aa7c79c 100644 --- a/library/core/src/borrow.rs +++ b/library/core/src/borrow.rs @@ -40,7 +40,6 @@ /// provide a reference to related type `T`, it is often better to use /// [`AsRef`] as more types can safely implement it. /// -/// [`AsRef`]: crate::convert::AsRef /// [`BorrowMut`]: BorrowMut /// [`Box`]: ../../std/boxed/struct.Box.html /// [`Mutex`]: ../../std/sync/struct.Mutex.html From f260462c32f25d34778c7702392061da2b497abb Mon Sep 17 00:00:00 2001 From: Denis Vasilik Date: Mon, 10 Aug 2020 23:14:43 +0200 Subject: [PATCH 36/37] Remove links that are in scope --- library/core/src/cmp.rs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/library/core/src/cmp.rs b/library/core/src/cmp.rs index 79085740119b..a61ad087ffbf 100644 --- a/library/core/src/cmp.rs +++ b/library/core/src/cmp.rs @@ -17,12 +17,6 @@ //! //! For more details, see the respective documentation of each item in the list. //! -//! [`Eq`]: trait.Eq.html -//! [`PartialEq`]: trait.PartialEq.html -//! [`Ord`]: trait.Ord.html -//! [`PartialOrd`]: trait.PartialOrd.html -//! [`Ordering`]: enum.Ordering.html -//! [`Reverse`]: struct.Reverse.html //! [`max`]: fn.max.html //! [`min`]: fn.min.html From eea85814e18f7558bc28c6bdc804d6e31d3c456e Mon Sep 17 00:00:00 2001 From: Denis Vasilik Date: Mon, 10 Aug 2020 23:16:01 +0200 Subject: [PATCH 37/37] Use intra-doc links --- library/core/src/cmp.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library/core/src/cmp.rs b/library/core/src/cmp.rs index a61ad087ffbf..e775ded60f59 100644 --- a/library/core/src/cmp.rs +++ b/library/core/src/cmp.rs @@ -17,8 +17,8 @@ //! //! For more details, see the respective documentation of each item in the list. //! -//! [`max`]: fn.max.html -//! [`min`]: fn.min.html +//! [`max`]: Ord::max +//! [`min`]: Ord::min #![stable(feature = "rust1", since = "1.0.0")]