From f121f094fe4d4f4e92142984e9230373c311e1e6 Mon Sep 17 00:00:00 2001 From: ThinkChaos Date: Tue, 21 Apr 2020 20:03:50 +0200 Subject: [PATCH 1/9] Add `RefCell::take` In the same vein as `Cell::take` and `Option::take`. --- src/libcore/cell.rs | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/src/libcore/cell.rs b/src/libcore/cell.rs index a922d4f118b5a..05c9c97a6126e 100644 --- a/src/libcore/cell.rs +++ b/src/libcore/cell.rs @@ -1023,6 +1023,27 @@ impl RefCell { } } +impl RefCell { + /// Takes the wrapped value, leaving `Default::default()` in its place. + /// + /// # Examples + /// + /// ``` + /// #![feature(refcell_take)] + /// use std::cell::RefCell; + /// + /// let c = RefCell::new(5); + /// let five = c.take(); + /// + /// assert_eq!(five, 5); + /// assert_eq!(c.into_inner(), 0); + /// ``` + #[unstable(feature = "refcell_take", issue = "71395")] + pub fn take(&self) -> T { + self.replace(Default::default()) + } +} + #[stable(feature = "rust1", since = "1.0.0")] unsafe impl Send for RefCell where T: Send {} From 4ea83bfb3d457770f2e54965dd86f672fbbc87c2 Mon Sep 17 00:00:00 2001 From: ThinkChaos Date: Tue, 21 Apr 2020 21:11:32 +0200 Subject: [PATCH 2/9] Use Cell::take in a couple places --- src/libstd/sync/once.rs | 2 +- src/test/run-make/wasm-panic-small/foo.rs | 2 +- .../ui/traits/negative-impls/pin-unsound-issue-66544-clone.rs | 2 +- .../traits/negative-impls/pin-unsound-issue-66544-derefmut.rs | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libstd/sync/once.rs b/src/libstd/sync/once.rs index 1e6b6c430be90..a3ee14e85d222 100644 --- a/src/libstd/sync/once.rs +++ b/src/libstd/sync/once.rs @@ -497,7 +497,7 @@ impl Drop for WaiterQueue<'_> { let mut queue = (state_and_queue & !STATE_MASK) as *const Waiter; while !queue.is_null() { let next = (*queue).next; - let thread = (*queue).thread.replace(None).unwrap(); + let thread = (*queue).thread.take().unwrap(); (*queue).signaled.store(true, Ordering::Release); // ^- FIXME (maybe): This is another case of issue #55005 // `store()` has a potentially dangling ref to `signaled`. diff --git a/src/test/run-make/wasm-panic-small/foo.rs b/src/test/run-make/wasm-panic-small/foo.rs index fd3dddb18eb20..6df52affe3993 100644 --- a/src/test/run-make/wasm-panic-small/foo.rs +++ b/src/test/run-make/wasm-panic-small/foo.rs @@ -23,5 +23,5 @@ pub fn foo() { pub fn foo() -> usize { use std::cell::Cell; thread_local!(static A: Cell> = Cell::new(Vec::new())); - A.try_with(|x| x.replace(Vec::new()).len()).unwrap_or(0) + A.try_with(|x| x.take().len()).unwrap_or(0) } diff --git a/src/test/ui/traits/negative-impls/pin-unsound-issue-66544-clone.rs b/src/test/ui/traits/negative-impls/pin-unsound-issue-66544-clone.rs index 499ac461e59a5..a5b85646581bf 100644 --- a/src/test/ui/traits/negative-impls/pin-unsound-issue-66544-clone.rs +++ b/src/test/ui/traits/negative-impls/pin-unsound-issue-66544-clone.rs @@ -7,7 +7,7 @@ struct MyType<'a>(Cell>>, PhantomPinned); impl<'a> Clone for &'a mut MyType<'a> { //~^ ERROR E0751 fn clone(&self) -> &'a mut MyType<'a> { - self.0.replace(None).unwrap() + self.0.take().unwrap() } } diff --git a/src/test/ui/traits/negative-impls/pin-unsound-issue-66544-derefmut.rs b/src/test/ui/traits/negative-impls/pin-unsound-issue-66544-derefmut.rs index 245be80078056..606cc65a84bc2 100644 --- a/src/test/ui/traits/negative-impls/pin-unsound-issue-66544-derefmut.rs +++ b/src/test/ui/traits/negative-impls/pin-unsound-issue-66544-derefmut.rs @@ -12,7 +12,7 @@ struct MyType<'a>(Cell>>, PhantomPinned); impl<'a> DerefMut for &'a MyType<'a> { //~^ ERROR E0751 fn deref_mut(&mut self) -> &mut MyType<'a> { - self.0.replace(None).unwrap() + self.0.take().unwrap() } } From 60d62bee36074d24f4995287ba3b12adf1df0888 Mon Sep 17 00:00:00 2001 From: Donough Liu Date: Fri, 1 May 2020 06:36:06 +0800 Subject: [PATCH 3/9] Suggest deref when coercing `ty::Ref` to `ty::RawPtr` with arbitrary mutability --- src/librustc_typeck/check/coercion.rs | 15 ++- src/librustc_typeck/check/demand.rs | 125 ++++++++++++++++-------- src/librustc_typeck/check/mod.rs | 4 +- src/test/ui/issues/issue-32122-1.stderr | 2 +- src/test/ui/issues/issue-32122-2.stderr | 2 +- src/test/ui/issues/issue-71676-1.fixed | 54 ++++++++++ src/test/ui/issues/issue-71676-1.rs | 54 ++++++++++ src/test/ui/issues/issue-71676-1.stderr | 55 +++++++++++ src/test/ui/issues/issue-71676-2.rs | 43 ++++++++ src/test/ui/issues/issue-71676-2.stderr | 16 +++ 10 files changed, 323 insertions(+), 47 deletions(-) create mode 100644 src/test/ui/issues/issue-71676-1.fixed create mode 100644 src/test/ui/issues/issue-71676-1.rs create mode 100644 src/test/ui/issues/issue-71676-1.stderr create mode 100644 src/test/ui/issues/issue-71676-2.rs create mode 100644 src/test/ui/issues/issue-71676-2.stderr diff --git a/src/librustc_typeck/check/coercion.rs b/src/librustc_typeck/check/coercion.rs index e3b16eaaef2a2..c336ec1347949 100644 --- a/src/librustc_typeck/check/coercion.rs +++ b/src/librustc_typeck/check/coercion.rs @@ -74,7 +74,7 @@ use rustc_trait_selection::traits::{self, ObligationCause, ObligationCauseCode}; use smallvec::{smallvec, SmallVec}; use std::ops::Deref; -pub struct Coerce<'a, 'tcx> { +struct Coerce<'a, 'tcx> { fcx: &'a FnCtxt<'a, 'tcx>, cause: ObligationCause<'tcx>, use_lub: bool, @@ -126,7 +126,7 @@ fn success<'tcx>( } impl<'f, 'tcx> Coerce<'f, 'tcx> { - pub fn new( + fn new( fcx: &'f FnCtxt<'f, 'tcx>, cause: ObligationCause<'tcx>, allow_two_phase: AllowTwoPhase, @@ -134,7 +134,7 @@ impl<'f, 'tcx> Coerce<'f, 'tcx> { Coerce { fcx, cause, allow_two_phase, use_lub: false } } - pub fn unify(&self, a: Ty<'tcx>, b: Ty<'tcx>) -> InferResult<'tcx, Ty<'tcx>> { + fn unify(&self, a: Ty<'tcx>, b: Ty<'tcx>) -> InferResult<'tcx, Ty<'tcx>> { debug!("unify(a: {:?}, b: {:?}, use_lub: {})", a, b, self.use_lub); self.commit_if_ok(|_| { if self.use_lub { @@ -831,6 +831,15 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { self.probe(|_| coerce.coerce(source, target)).is_ok() } + pub fn deref_steps(&self, expr_ty: Ty<'tcx>, target: Ty<'tcx>) -> Option { + let cause = self.cause(rustc_span::DUMMY_SP, ObligationCauseCode::ExprAssignable); + // We don't ever need two-phase here since we throw out the result of the coercion + let coerce = Coerce::new(self, cause, AllowTwoPhase::No); + coerce + .autoderef(rustc_span::DUMMY_SP, expr_ty) + .find_map(|(ty, steps)| coerce.unify(ty, target).ok().map(|_| steps)) + } + /// Given some expressions, their known unified type and another expression, /// tries to unify the types, potentially inserting coercions on any of the /// provided expressions and returns their LUB (aka "common supertype"). diff --git a/src/librustc_typeck/check/demand.rs b/src/librustc_typeck/check/demand.rs index 65ef9cad87448..2c4ab40ccd443 100644 --- a/src/librustc_typeck/check/demand.rs +++ b/src/librustc_typeck/check/demand.rs @@ -1,4 +1,3 @@ -use crate::check::coercion::Coerce; use crate::check::FnCtxt; use rustc_infer::infer::InferOk; use rustc_trait_selection::infer::InferCtxtExt as _; @@ -9,7 +8,6 @@ use rustc_ast::util::parser::PREC_POSTFIX; use rustc_errors::{Applicability, DiagnosticBuilder}; use rustc_hir as hir; use rustc_hir::{is_range_literal, Node}; -use rustc_middle::traits::ObligationCauseCode; use rustc_middle::ty::adjustment::AllowTwoPhase; use rustc_middle::ty::{self, AssocItem, Ty, TypeAndMut}; use rustc_span::symbol::sym; @@ -376,7 +374,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { expr: &hir::Expr<'_>, checked_ty: Ty<'tcx>, expected: Ty<'tcx>, - ) -> Option<(Span, &'static str, String)> { + ) -> Option<(Span, &'static str, String, Applicability)> { let sm = self.sess().source_map(); let sp = expr.span; if sm.is_imported(sp) { @@ -395,16 +393,21 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // `ExprKind::DropTemps` is semantically irrelevant for these suggestions. let expr = expr.peel_drop_temps(); + let remove_prefix = |s: String, prefix: &str| { + if s.starts_with(prefix) { Some(s[prefix.len()..].to_string()) } else { None } + }; + match (&expr.kind, &expected.kind, &checked_ty.kind) { (_, &ty::Ref(_, exp, _), &ty::Ref(_, check, _)) => match (&exp.kind, &check.kind) { (&ty::Str, &ty::Array(arr, _) | &ty::Slice(arr)) if arr == self.tcx.types.u8 => { if let hir::ExprKind::Lit(_) = expr.kind { if let Ok(src) = sm.span_to_snippet(sp) { - if src.starts_with("b\"") { + if let Some(src) = remove_prefix(src, "b\"") { return Some(( sp, "consider removing the leading `b`", - src[1..].to_string(), + format!("\"{}", src), + Applicability::MachineApplicable, )); } } @@ -413,11 +416,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { (&ty::Array(arr, _) | &ty::Slice(arr), &ty::Str) if arr == self.tcx.types.u8 => { if let hir::ExprKind::Lit(_) = expr.kind { if let Ok(src) = sm.span_to_snippet(sp) { - if src.starts_with('"') { + if let Some(src) = remove_prefix(src, "\"") { return Some(( sp, "consider adding a leading `b`", - format!("b{}", src), + format!("b\"{}", src), + Applicability::MachineApplicable, )); } } @@ -470,7 +474,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { let sugg_expr = if needs_parens { format!("({})", src) } else { src }; if let Some(sugg) = self.can_use_as_ref(expr) { - return Some(sugg); + return Some(( + sugg.0, + sugg.1, + sugg.2, + Applicability::MachineApplicable, + )); } let field_name = if is_struct_pat_shorthand_field { format!("{}: ", sugg_expr) @@ -495,6 +504,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { "consider dereferencing here to assign to the mutable \ borrowed piece of memory", format!("*{}", src), + Applicability::MachineApplicable, )); } } @@ -505,11 +515,13 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { sp, "consider mutably borrowing here", format!("{}&mut {}", field_name, sugg_expr), + Applicability::MachineApplicable, ), hir::Mutability::Not => ( sp, "consider borrowing here", format!("{}&{}", field_name, sugg_expr), + Applicability::MachineApplicable, ), }); } @@ -526,51 +538,84 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // We have `&T`, check if what was expected was `T`. If so, // we may want to suggest removing a `&`. if sm.is_imported(expr.span) { - if let Ok(code) = sm.span_to_snippet(sp) { - if code.starts_with('&') { + if let Ok(src) = sm.span_to_snippet(sp) { + if let Some(src) = remove_prefix(src, "&") { return Some(( sp, "consider removing the borrow", - code[1..].to_string(), + src, + Applicability::MachineApplicable, )); } } return None; } if let Ok(code) = sm.span_to_snippet(expr.span) { - return Some((sp, "consider removing the borrow", code)); + return Some(( + sp, + "consider removing the borrow", + code, + Applicability::MachineApplicable, + )); } } ( _, - &ty::RawPtr(TypeAndMut { ty: _, mutbl: hir::Mutability::Not }), - &ty::Ref(_, _, hir::Mutability::Not), + &ty::RawPtr(TypeAndMut { ty: ty_b, mutbl: mutbl_b }), + &ty::Ref(_, ty_a, mutbl_a), ) => { - let cause = self.cause(rustc_span::DUMMY_SP, ObligationCauseCode::ExprAssignable); - // We don't ever need two-phase here since we throw out the result of the coercion - let coerce = Coerce::new(self, cause, AllowTwoPhase::No); - - if let Some(steps) = - coerce.autoderef(sp, checked_ty).skip(1).find_map(|(referent_ty, steps)| { - coerce - .unify( - coerce.tcx.mk_ptr(ty::TypeAndMut { - mutbl: hir::Mutability::Not, - ty: referent_ty, - }), - expected, - ) - .ok() - .map(|_| steps) - }) - { - // The pointer type implements `Copy` trait so the suggestion is always valid. - if let Ok(code) = sm.span_to_snippet(sp) { - if code.starts_with('&') { - let derefs = "*".repeat(steps - 1); - let message = "consider dereferencing the reference"; - let suggestion = format!("&{}{}", derefs, code[1..].to_string()); - return Some((sp, message, suggestion)); + if let Some(steps) = self.deref_steps(ty_a, ty_b) { + // Only suggest valid if dereferencing needed. + if steps > 0 { + // The pointer type implements `Copy` trait so the suggestion is always valid. + if let Ok(src) = sm.span_to_snippet(sp) { + let derefs = "*".repeat(steps); + match mutbl_b { + hir::Mutability::Mut => match mutbl_a { + hir::Mutability::Mut => { + if let Some(src) = remove_prefix(src, "&mut ") { + return Some(( + sp, + "consider dereferencing", + format!("&mut {}{}", derefs, src), + Applicability::MachineApplicable, + )); + } + } + hir::Mutability::Not => { + if let Some(src) = remove_prefix(src, "&") { + return Some(( + sp, + "consider dereferencing", + format!("&mut {}{}", derefs, src), + Applicability::Unspecified, + )); + } + } + }, + hir::Mutability::Not => match mutbl_a { + hir::Mutability::Mut => { + if let Some(src) = remove_prefix(src, "&mut ") { + return Some(( + sp, + "consider dereferencing", + format!("&{}{}", derefs, src), + Applicability::MachineApplicable, + )); + } + } + hir::Mutability::Not => { + if let Some(src) = remove_prefix(src, "&") { + return Some(( + sp, + "consider dereferencing", + format!("&{}{}", derefs, src), + Applicability::MachineApplicable, + )); + } + } + }, + } } } } @@ -616,7 +661,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } else { format!("*{}", code) }; - return Some((sp, message, suggestion)); + return Some((sp, message, suggestion, Applicability::MachineApplicable)); } } } diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index adbab3d4cb620..68817d3fe0f16 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -5036,8 +5036,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { expected: Ty<'tcx>, found: Ty<'tcx>, ) { - if let Some((sp, msg, suggestion)) = self.check_ref(expr, found, expected) { - err.span_suggestion(sp, msg, suggestion, Applicability::MachineApplicable); + if let Some((sp, msg, suggestion, applicability)) = self.check_ref(expr, found, expected) { + err.span_suggestion(sp, msg, suggestion, applicability); } else if let (ty::FnDef(def_id, ..), true) = (&found.kind, self.suggest_fn_call(err, expr, expected, found)) { diff --git a/src/test/ui/issues/issue-32122-1.stderr b/src/test/ui/issues/issue-32122-1.stderr index 313de275c53ee..dfbd3223efc86 100644 --- a/src/test/ui/issues/issue-32122-1.stderr +++ b/src/test/ui/issues/issue-32122-1.stderr @@ -5,7 +5,7 @@ LL | let _: *const u8 = &a; | --------- ^^ | | | | | expected `u8`, found struct `Foo` - | | help: consider dereferencing the reference: `&*a` + | | help: consider dereferencing: `&*a` | expected due to this | = note: expected raw pointer `*const u8` diff --git a/src/test/ui/issues/issue-32122-2.stderr b/src/test/ui/issues/issue-32122-2.stderr index 959a49507e4f5..2e199e2a19f73 100644 --- a/src/test/ui/issues/issue-32122-2.stderr +++ b/src/test/ui/issues/issue-32122-2.stderr @@ -5,7 +5,7 @@ LL | let _: *const u8 = &a; | --------- ^^ | | | | | expected `u8`, found struct `Emm` - | | help: consider dereferencing the reference: `&***a` + | | help: consider dereferencing: `&***a` | expected due to this | = note: expected raw pointer `*const u8` diff --git a/src/test/ui/issues/issue-71676-1.fixed b/src/test/ui/issues/issue-71676-1.fixed new file mode 100644 index 0000000000000..d2be0db95fb8c --- /dev/null +++ b/src/test/ui/issues/issue-71676-1.fixed @@ -0,0 +1,54 @@ +// run-rustfix +use std::ops::Deref; +use std::ops::DerefMut; +struct Bar(u8); +struct Foo(Bar); +struct Emm(Foo); +impl Deref for Bar{ + type Target = u8; + fn deref(&self) -> &Self::Target { + &self.0 + } +} +impl Deref for Foo { + type Target = Bar; + fn deref(&self) -> &Self::Target { + &self.0 + } +} +impl Deref for Emm { + type Target = Foo; + fn deref(&self) -> &Self::Target { + &self.0 + } +} +impl DerefMut for Bar{ + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.0 + } +} +impl DerefMut for Foo { + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.0 + } +} +impl DerefMut for Emm { + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.0 + } +} +fn main() { + // Suggest dereference with arbitrary mutability + let a = Emm(Foo(Bar(0))); + let _: *const u8 = &***a; //~ ERROR mismatched types + + let mut a = Emm(Foo(Bar(0))); + let _: *mut u8 = &mut ***a; //~ ERROR mismatched types + + let a = Emm(Foo(Bar(0))); + let _: *const u8 = &***a; //~ ERROR mismatched types + + let mut a = Emm(Foo(Bar(0))); + let _: *mut u8 = &mut ***a; //~ ERROR mismatched types + +} diff --git a/src/test/ui/issues/issue-71676-1.rs b/src/test/ui/issues/issue-71676-1.rs new file mode 100644 index 0000000000000..c09ad6dabaa9a --- /dev/null +++ b/src/test/ui/issues/issue-71676-1.rs @@ -0,0 +1,54 @@ +// run-rustfix +use std::ops::Deref; +use std::ops::DerefMut; +struct Bar(u8); +struct Foo(Bar); +struct Emm(Foo); +impl Deref for Bar{ + type Target = u8; + fn deref(&self) -> &Self::Target { + &self.0 + } +} +impl Deref for Foo { + type Target = Bar; + fn deref(&self) -> &Self::Target { + &self.0 + } +} +impl Deref for Emm { + type Target = Foo; + fn deref(&self) -> &Self::Target { + &self.0 + } +} +impl DerefMut for Bar{ + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.0 + } +} +impl DerefMut for Foo { + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.0 + } +} +impl DerefMut for Emm { + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.0 + } +} +fn main() { + // Suggest dereference with arbitrary mutability + let a = Emm(Foo(Bar(0))); + let _: *const u8 = &a; //~ ERROR mismatched types + + let mut a = Emm(Foo(Bar(0))); + let _: *mut u8 = &a; //~ ERROR mismatched types + + let a = Emm(Foo(Bar(0))); + let _: *const u8 = &mut a; //~ ERROR mismatched types + + let mut a = Emm(Foo(Bar(0))); + let _: *mut u8 = &mut a; //~ ERROR mismatched types + +} diff --git a/src/test/ui/issues/issue-71676-1.stderr b/src/test/ui/issues/issue-71676-1.stderr new file mode 100644 index 0000000000000..bbabc2202dc84 --- /dev/null +++ b/src/test/ui/issues/issue-71676-1.stderr @@ -0,0 +1,55 @@ +error[E0308]: mismatched types + --> $DIR/issue-71676-1.rs:43:24 + | +LL | let _: *const u8 = &a; + | --------- ^^ + | | | + | | expected `u8`, found struct `Emm` + | | help: consider dereferencing: `&***a` + | expected due to this + | + = note: expected raw pointer `*const u8` + found reference `&Emm` + +error[E0308]: mismatched types + --> $DIR/issue-71676-1.rs:46:22 + | +LL | let _: *mut u8 = &a; + | ------- ^^ + | | | + | | types differ in mutability + | | help: consider dereferencing: `&mut ***a` + | expected due to this + | + = note: expected raw pointer `*mut u8` + found reference `&Emm` + +error[E0308]: mismatched types + --> $DIR/issue-71676-1.rs:49:24 + | +LL | let _: *const u8 = &mut a; + | --------- ^^^^^^ + | | | + | | expected `u8`, found struct `Emm` + | | help: consider dereferencing: `&***a` + | expected due to this + | + = note: expected raw pointer `*const u8` + found mutable reference `&mut Emm` + +error[E0308]: mismatched types + --> $DIR/issue-71676-1.rs:52:22 + | +LL | let _: *mut u8 = &mut a; + | ------- ^^^^^^ + | | | + | | expected `u8`, found struct `Emm` + | | help: consider dereferencing: `&mut ***a` + | expected due to this + | + = note: expected raw pointer `*mut u8` + found mutable reference `&mut Emm` + +error: aborting due to 4 previous errors + +For more information about this error, try `rustc --explain E0308`. diff --git a/src/test/ui/issues/issue-71676-2.rs b/src/test/ui/issues/issue-71676-2.rs new file mode 100644 index 0000000000000..d11fce567ce51 --- /dev/null +++ b/src/test/ui/issues/issue-71676-2.rs @@ -0,0 +1,43 @@ +use std::ops::Deref; +use std::ops::DerefMut; +struct Bar(u8); +struct Foo(Bar); +struct Emm(Foo); +impl Deref for Bar{ + type Target = u8; + fn deref(&self) -> &Self::Target { + &self.0 + } +} +impl Deref for Foo { + type Target = Bar; + fn deref(&self) -> &Self::Target { + &self.0 + } +} +impl Deref for Emm { + type Target = Foo; + fn deref(&self) -> &Self::Target { + &self.0 + } +} +impl DerefMut for Bar{ + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.0 + } +} +impl DerefMut for Foo { + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.0 + } +} +impl DerefMut for Emm { + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.0 + } +} +fn main() { + // Should not suggest when a is immutable + let a = Emm(Foo(Bar(0))); + let _: *mut u8 = &a; //~ ERROR mismatched types +} diff --git a/src/test/ui/issues/issue-71676-2.stderr b/src/test/ui/issues/issue-71676-2.stderr new file mode 100644 index 0000000000000..273ae9cb4dba8 --- /dev/null +++ b/src/test/ui/issues/issue-71676-2.stderr @@ -0,0 +1,16 @@ +error[E0308]: mismatched types + --> $DIR/issue-71676-2.rs:42:22 + | +LL | let _: *mut u8 = &a; + | ------- ^^ + | | | + | | types differ in mutability + | | help: consider dereferencing: `&mut ***a` + | expected due to this + | + = note: expected raw pointer `*mut u8` + found reference `&Emm` + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0308`. From 80d04cc1ba610d796c84427622ce17eb2ca9c771 Mon Sep 17 00:00:00 2001 From: Donough Liu Date: Fri, 1 May 2020 21:56:10 +0800 Subject: [PATCH 4/9] Add comments for deref_steps() --- src/librustc_typeck/check/coercion.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/librustc_typeck/check/coercion.rs b/src/librustc_typeck/check/coercion.rs index c336ec1347949..7437c87a25763 100644 --- a/src/librustc_typeck/check/coercion.rs +++ b/src/librustc_typeck/check/coercion.rs @@ -831,6 +831,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { self.probe(|_| coerce.coerce(source, target)).is_ok() } + /// Given a type and a target type, this function will calculate and return + /// how many dereference steps needed to achieve `expr_ty <: target`. If + /// it's not possible, return `None`. pub fn deref_steps(&self, expr_ty: Ty<'tcx>, target: Ty<'tcx>) -> Option { let cause = self.cause(rustc_span::DUMMY_SP, ObligationCauseCode::ExprAssignable); // We don't ever need two-phase here since we throw out the result of the coercion From 089d4bbfd79c4ae2e735154cff4448ecd5fc4afc Mon Sep 17 00:00:00 2001 From: Donough Liu Date: Fri, 1 May 2020 21:57:19 +0800 Subject: [PATCH 5/9] Suggestion for immutable reference -> mutable pointer should be emitted as `Applicability::Unspecified` --- src/test/ui/issues/issue-71676-2.rs | 1 - src/test/ui/issues/issue-71676-2.stderr | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/test/ui/issues/issue-71676-2.rs b/src/test/ui/issues/issue-71676-2.rs index d11fce567ce51..f3183899dc523 100644 --- a/src/test/ui/issues/issue-71676-2.rs +++ b/src/test/ui/issues/issue-71676-2.rs @@ -37,7 +37,6 @@ impl DerefMut for Emm { } } fn main() { - // Should not suggest when a is immutable let a = Emm(Foo(Bar(0))); let _: *mut u8 = &a; //~ ERROR mismatched types } diff --git a/src/test/ui/issues/issue-71676-2.stderr b/src/test/ui/issues/issue-71676-2.stderr index 273ae9cb4dba8..ebdd345809af5 100644 --- a/src/test/ui/issues/issue-71676-2.stderr +++ b/src/test/ui/issues/issue-71676-2.stderr @@ -1,5 +1,5 @@ error[E0308]: mismatched types - --> $DIR/issue-71676-2.rs:42:22 + --> $DIR/issue-71676-2.rs:41:22 | LL | let _: *mut u8 = &a; | ------- ^^ From 9a212c1625514fdc5588cdfe6f2d58290e73248d Mon Sep 17 00:00:00 2001 From: Donough Liu Date: Fri, 1 May 2020 21:59:09 +0800 Subject: [PATCH 6/9] Replace convenient function `remove_prefix()` with `replace_prefix()` --- src/librustc_typeck/check/demand.rs | 109 ++++++++++++++----------- src/test/ui/issues/issue-71676-1.fixed | 1 - src/test/ui/issues/issue-71676-1.rs | 1 - 3 files changed, 60 insertions(+), 51 deletions(-) diff --git a/src/librustc_typeck/check/demand.rs b/src/librustc_typeck/check/demand.rs index 2c4ab40ccd443..9e14efb67a94c 100644 --- a/src/librustc_typeck/check/demand.rs +++ b/src/librustc_typeck/check/demand.rs @@ -353,6 +353,17 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { false } + fn replace_prefix(&self, s: A, old: B, new: C) -> Option + where + A: AsRef, + B: AsRef, + C: AsRef, + { + let s = s.as_ref(); + let old = old.as_ref(); + if s.starts_with(old) { Some(new.as_ref().to_owned() + &s[old.len()..]) } else { None } + } + /// This function is used to determine potential "simple" improvements or users' errors and /// provide them useful help. For example: /// @@ -393,20 +404,16 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // `ExprKind::DropTemps` is semantically irrelevant for these suggestions. let expr = expr.peel_drop_temps(); - let remove_prefix = |s: String, prefix: &str| { - if s.starts_with(prefix) { Some(s[prefix.len()..].to_string()) } else { None } - }; - match (&expr.kind, &expected.kind, &checked_ty.kind) { (_, &ty::Ref(_, exp, _), &ty::Ref(_, check, _)) => match (&exp.kind, &check.kind) { (&ty::Str, &ty::Array(arr, _) | &ty::Slice(arr)) if arr == self.tcx.types.u8 => { if let hir::ExprKind::Lit(_) = expr.kind { if let Ok(src) = sm.span_to_snippet(sp) { - if let Some(src) = remove_prefix(src, "b\"") { + if let Some(src) = self.replace_prefix(src, "b\"", "\"") { return Some(( sp, "consider removing the leading `b`", - format!("\"{}", src), + src, Applicability::MachineApplicable, )); } @@ -416,11 +423,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { (&ty::Array(arr, _) | &ty::Slice(arr), &ty::Str) if arr == self.tcx.types.u8 => { if let hir::ExprKind::Lit(_) = expr.kind { if let Ok(src) = sm.span_to_snippet(sp) { - if let Some(src) = remove_prefix(src, "\"") { + if let Some(src) = self.replace_prefix(src, "\"", "b\"") { return Some(( sp, "consider adding a leading `b`", - format!("b\"{}", src), + src, Applicability::MachineApplicable, )); } @@ -539,7 +546,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // we may want to suggest removing a `&`. if sm.is_imported(expr.span) { if let Ok(src) = sm.span_to_snippet(sp) { - if let Some(src) = remove_prefix(src, "&") { + if let Some(src) = self.replace_prefix(src, "&", "") { return Some(( sp, "consider removing the borrow", @@ -569,52 +576,56 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { if steps > 0 { // The pointer type implements `Copy` trait so the suggestion is always valid. if let Ok(src) = sm.span_to_snippet(sp) { - let derefs = "*".repeat(steps); - match mutbl_b { - hir::Mutability::Mut => match mutbl_a { - hir::Mutability::Mut => { - if let Some(src) = remove_prefix(src, "&mut ") { - return Some(( - sp, - "consider dereferencing", - format!("&mut {}{}", derefs, src), - Applicability::MachineApplicable, - )); + let derefs = &"*".repeat(steps); + if let Some((src, applicability)) = match mutbl_b { + hir::Mutability::Mut => { + let new_prefix = "&mut ".to_owned() + derefs; + match mutbl_a { + hir::Mutability::Mut => { + if let Some(s) = + self.replace_prefix(src, "&mut ", new_prefix) + { + Some((s, Applicability::MachineApplicable)) + } else { + None + } } - } - hir::Mutability::Not => { - if let Some(src) = remove_prefix(src, "&") { - return Some(( - sp, - "consider dereferencing", - format!("&mut {}{}", derefs, src), - Applicability::Unspecified, - )); + hir::Mutability::Not => { + if let Some(s) = + self.replace_prefix(src, "&", new_prefix) + { + Some((s, Applicability::Unspecified)) + } else { + None + } } } - }, - hir::Mutability::Not => match mutbl_a { - hir::Mutability::Mut => { - if let Some(src) = remove_prefix(src, "&mut ") { - return Some(( - sp, - "consider dereferencing", - format!("&{}{}", derefs, src), - Applicability::MachineApplicable, - )); + } + hir::Mutability::Not => { + let new_prefix = "&".to_owned() + derefs; + match mutbl_a { + hir::Mutability::Mut => { + if let Some(s) = + self.replace_prefix(src, "&mut ", new_prefix) + { + Some((s, Applicability::MachineApplicable)) + } else { + None + } } - } - hir::Mutability::Not => { - if let Some(src) = remove_prefix(src, "&") { - return Some(( - sp, - "consider dereferencing", - format!("&{}{}", derefs, src), - Applicability::MachineApplicable, - )); + hir::Mutability::Not => { + if let Some(s) = + self.replace_prefix(src, "&", new_prefix) + { + Some((s, Applicability::MachineApplicable)) + } else { + None + } } } - }, + } + } { + return Some((sp, "consider dereferencing", src, applicability)); } } } diff --git a/src/test/ui/issues/issue-71676-1.fixed b/src/test/ui/issues/issue-71676-1.fixed index d2be0db95fb8c..cbc0e8c061b82 100644 --- a/src/test/ui/issues/issue-71676-1.fixed +++ b/src/test/ui/issues/issue-71676-1.fixed @@ -50,5 +50,4 @@ fn main() { let mut a = Emm(Foo(Bar(0))); let _: *mut u8 = &mut ***a; //~ ERROR mismatched types - } diff --git a/src/test/ui/issues/issue-71676-1.rs b/src/test/ui/issues/issue-71676-1.rs index c09ad6dabaa9a..6e87c7174c633 100644 --- a/src/test/ui/issues/issue-71676-1.rs +++ b/src/test/ui/issues/issue-71676-1.rs @@ -50,5 +50,4 @@ fn main() { let mut a = Emm(Foo(Bar(0))); let _: *mut u8 = &mut a; //~ ERROR mismatched types - } From 675b585931d870ad4207d60ef43179390526a2fc Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Sun, 3 May 2020 11:40:45 +0200 Subject: [PATCH 7/9] Remove clippy from some leftover lists of "possibly failing" tools --- src/bootstrap/test.rs | 2 +- src/bootstrap/toolstate.rs | 1 - src/tools/publish_toolstate.py | 5 ----- 3 files changed, 1 insertion(+), 7 deletions(-) diff --git a/src/bootstrap/test.rs b/src/bootstrap/test.rs index 90b8b5eea947f..0cf47d20ead06 100644 --- a/src/bootstrap/test.rs +++ b/src/bootstrap/test.rs @@ -528,7 +528,7 @@ impl Step for Clippy { host, "test", "src/tools/clippy", - SourceType::Submodule, + SourceType::InTree, &[], ); diff --git a/src/bootstrap/toolstate.rs b/src/bootstrap/toolstate.rs index e6560771c0ee7..cd8ce4881b1dd 100644 --- a/src/bootstrap/toolstate.rs +++ b/src/bootstrap/toolstate.rs @@ -78,7 +78,6 @@ static STABLE_TOOLS: &[(&str, &str)] = &[ ("edition-guide", "src/doc/edition-guide"), ("rls", "src/tools/rls"), ("rustfmt", "src/tools/rustfmt"), - ("clippy-driver", "src/tools/clippy"), ]; // These tools are permitted to not build on the beta/stable channels. diff --git a/src/tools/publish_toolstate.py b/src/tools/publish_toolstate.py index 2da62b6bd99f6..988a226706dc0 100755 --- a/src/tools/publish_toolstate.py +++ b/src/tools/publish_toolstate.py @@ -25,10 +25,6 @@ # read privileges on it). CI will fail otherwise. MAINTAINERS = { 'miri': {'oli-obk', 'RalfJung', 'eddyb'}, - 'clippy-driver': { - 'Manishearth', 'llogiq', 'mcarton', 'oli-obk', 'phansch', 'flip1995', - 'yaahc', - }, 'rls': {'Xanewok'}, 'rustfmt': {'topecongiro'}, 'book': {'carols10cents', 'steveklabnik'}, @@ -45,7 +41,6 @@ REPOS = { 'miri': 'https://github.com/rust-lang/miri', - 'clippy-driver': 'https://github.com/rust-lang/rust-clippy', 'rls': 'https://github.com/rust-lang/rls', 'rustfmt': 'https://github.com/rust-lang/rustfmt', 'book': 'https://github.com/rust-lang/book', From 4a79424b748338bd9635f0115c025942af2d5c3d Mon Sep 17 00:00:00 2001 From: ThinkChaos Date: Sun, 3 May 2020 12:52:23 +0200 Subject: [PATCH 8/9] Mention `RefCell::take` can panic in docs --- src/libcore/cell.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/libcore/cell.rs b/src/libcore/cell.rs index 05c9c97a6126e..0f2665eba6f22 100644 --- a/src/libcore/cell.rs +++ b/src/libcore/cell.rs @@ -1026,6 +1026,10 @@ impl RefCell { impl RefCell { /// Takes the wrapped value, leaving `Default::default()` in its place. /// + /// # Panics + /// + /// Panics if the value is currently borrowed. + /// /// # Examples /// /// ``` From 9e19f3a27f61596a45103c815ab4fe3f38e71272 Mon Sep 17 00:00:00 2001 From: Matthew Jasper Date: Sat, 2 May 2020 13:18:31 +0100 Subject: [PATCH 9/9] Correctly check comparison operators in MIR typeck --- .../borrow_check/type_check/mod.rs | 72 ++++++++++++------- .../ui/nll/type-check-pointer-comparisons.rs | 8 +-- .../nll/type-check-pointer-comparisons.stderr | 14 +--- 3 files changed, 50 insertions(+), 44 deletions(-) diff --git a/src/librustc_mir/borrow_check/type_check/mod.rs b/src/librustc_mir/borrow_check/type_check/mod.rs index 7f554742777e2..36ccc0aaa8bb4 100644 --- a/src/librustc_mir/borrow_check/type_check/mod.rs +++ b/src/librustc_mir/borrow_check/type_check/mod.rs @@ -2290,36 +2290,54 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> { right, ) => { let ty_left = left.ty(body, tcx); - if let ty::RawPtr(_) | ty::FnPtr(_) = ty_left.kind { - let ty_right = right.ty(body, tcx); - let common_ty = self.infcx.next_ty_var(TypeVariableOrigin { - kind: TypeVariableOriginKind::MiscVariable, - span: body.source_info(location).span, - }); - self.sub_types( - common_ty, - ty_left, - location.to_locations(), - ConstraintCategory::Boring, - ) - .unwrap_or_else(|err| { - bug!("Could not equate type variable with {:?}: {:?}", ty_left, err) - }); - if let Err(terr) = self.sub_types( - common_ty, - ty_right, - location.to_locations(), - ConstraintCategory::Boring, - ) { - span_mirbug!( - self, - rvalue, - "unexpected comparison types {:?} and {:?} yields {:?}", + match ty_left.kind { + // Types with regions are comparable if they have a common super-type. + ty::RawPtr(_) | ty::FnPtr(_) => { + let ty_right = right.ty(body, tcx); + let common_ty = self.infcx.next_ty_var(TypeVariableOrigin { + kind: TypeVariableOriginKind::MiscVariable, + span: body.source_info(location).span, + }); + self.relate_types( + common_ty, + ty::Variance::Contravariant, ty_left, - ty_right, - terr + location.to_locations(), + ConstraintCategory::Boring, ) + .unwrap_or_else(|err| { + bug!("Could not equate type variable with {:?}: {:?}", ty_left, err) + }); + if let Err(terr) = self.relate_types( + common_ty, + ty::Variance::Contravariant, + ty_right, + location.to_locations(), + ConstraintCategory::Boring, + ) { + span_mirbug!( + self, + rvalue, + "unexpected comparison types {:?} and {:?} yields {:?}", + ty_left, + ty_right, + terr + ) + } } + // For types with no regions we can just check that the + // both operands have the same type. + ty::Int(_) | ty::Uint(_) | ty::Bool | ty::Char | ty::Float(_) + if ty_left == right.ty(body, tcx) => {} + // Other types are compared by trait methods, not by + // `Rvalue::BinaryOp`. + _ => span_mirbug!( + self, + rvalue, + "unexpected comparison types {:?} and {:?}", + ty_left, + right.ty(body, tcx) + ), } } diff --git a/src/test/ui/nll/type-check-pointer-comparisons.rs b/src/test/ui/nll/type-check-pointer-comparisons.rs index 298a6ef7ab3c5..3c900356fab3b 100644 --- a/src/test/ui/nll/type-check-pointer-comparisons.rs +++ b/src/test/ui/nll/type-check-pointer-comparisons.rs @@ -21,13 +21,13 @@ fn compare_fn_ptr<'a, 'b, 'c>(f: fn(&'c mut &'a i32), g: fn(&'c mut &'b i32)) { } fn compare_hr_fn_ptr<'a>(f: fn(&'a i32), g: fn(&i32)) { - f == g; - //~^ ERROR higher-ranked subtype error + // Ideally this should compile with the operands swapped as well, but HIR + // type checking prevents it (and stops compilation) for now. + f == g; // OK } fn compare_const_fn_ptr<'a>(f: *const fn(&'a i32), g: *const fn(&i32)) { - f == g; - //~^ ERROR higher-ranked subtype error + f == g; // OK } fn main() {} diff --git a/src/test/ui/nll/type-check-pointer-comparisons.stderr b/src/test/ui/nll/type-check-pointer-comparisons.stderr index 0fc7480260fdb..f350b861eb6d2 100644 --- a/src/test/ui/nll/type-check-pointer-comparisons.stderr +++ b/src/test/ui/nll/type-check-pointer-comparisons.stderr @@ -76,17 +76,5 @@ LL | f == g; help: `'a` and `'b` must be the same: replace one with the other -error: higher-ranked subtype error - --> $DIR/type-check-pointer-comparisons.rs:24:5 - | -LL | f == g; - | ^^^^^^ - -error: higher-ranked subtype error - --> $DIR/type-check-pointer-comparisons.rs:29:5 - | -LL | f == g; - | ^^^^^^ - -error: aborting due to 8 previous errors +error: aborting due to 6 previous errors