From 28f84850fdccd42d2de774948690fc09800ccfe7 Mon Sep 17 00:00:00 2001 From: George <33588728+George-lewis@users.noreply.github.com> Date: Thu, 28 Apr 2022 02:00:51 -0400 Subject: [PATCH 1/4] Simplify code, change applicability --- .../src/diagnostics/move_errors.rs | 21 +++++++++++++++---- src/test/ui/borrowck/issue-96438.rs | 13 ++++++++++++ src/test/ui/borrowck/issue-96438.stderr | 16 ++++++++++++++ .../ui/suggestions/option-content-move.fixed | 4 ++-- .../ui/suggestions/option-content-move.stderr | 4 ++++ 5 files changed, 52 insertions(+), 6 deletions(-) create mode 100644 src/test/ui/borrowck/issue-96438.rs create mode 100644 src/test/ui/borrowck/issue-96438.stderr diff --git a/compiler/rustc_borrowck/src/diagnostics/move_errors.rs b/compiler/rustc_borrowck/src/diagnostics/move_errors.rs index e7fd89c140fc7..d23b6be2e1320 100644 --- a/compiler/rustc_borrowck/src/diagnostics/move_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/move_errors.rs @@ -4,7 +4,7 @@ use rustc_middle::ty; use rustc_mir_dataflow::move_paths::{ IllegalMoveOrigin, IllegalMoveOriginKind, LookupResult, MoveError, MovePathIndex, }; -use rustc_span::{sym, Span}; +use rustc_span::{sym, Span, BytePos}; use crate::diagnostics::UseSpans; use crate::prefixes::PrefixSet; @@ -409,16 +409,29 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { | ty::Opaque(def_id, _) => def_id, _ => return err, }; + // self.infcx.tcx. let diag_name = self.infcx.tcx.get_diagnostic_name(def_id); if matches!(diag_name, Some(sym::Option | sym::Result)) && use_spans.map_or(true, |v| !v.for_closure()) && !has_complex_bindings { - err.span_suggestion_verbose( + let span = match use_spans { + Some(UseSpans::FnSelfUse { var_span, fn_call_span, .. }) => { + // `fn_call_span` is the span of `unwrap()` + // This gets the 'variable' span excluding the unwrap + let span = var_span.until(fn_call_span); + + // Unfortunately the new span has a "." on the end which makes + // the suggestion format strangely, so we adjust the hi bound manually + span.with_hi(span.hi() - BytePos(1)) + }, + _ => span + }; + err.span_suggestions( span.shrink_to_hi(), &format!("consider borrowing the `{}`'s content", diag_name.unwrap()), - ".as_ref()".to_string(), - Applicability::MaybeIncorrect, + vec![".as_ref()".to_string(), ".as_mut()".to_string()].into_iter(), + Applicability::MachineApplicable ); } else if let Some(use_spans) = use_spans { self.explain_captures( diff --git a/src/test/ui/borrowck/issue-96438.rs b/src/test/ui/borrowck/issue-96438.rs new file mode 100644 index 0000000000000..3ff6a025df241 --- /dev/null +++ b/src/test/ui/borrowck/issue-96438.rs @@ -0,0 +1,13 @@ +use std::cell::RefCell; + +fn foo(cell: &RefCell>>) { + cell.borrow_mut().unwrap().pop().unwrap(); + //~^ ERROR cannot move out of dereference of `RefMut<'_, Option>>` [E0507] + //~| NOTE move occurs because value has type `Option>`, which does not implement the `Copy` trait + //~| HELP consider borrowing the `Option`'s content +} + +fn main() { + let cell = RefCell::new(None); + foo(&cell); +} \ No newline at end of file diff --git a/src/test/ui/borrowck/issue-96438.stderr b/src/test/ui/borrowck/issue-96438.stderr new file mode 100644 index 0000000000000..5294b5767ca1c --- /dev/null +++ b/src/test/ui/borrowck/issue-96438.stderr @@ -0,0 +1,16 @@ +error[E0507]: cannot move out of dereference of `RefMut<'_, Option>>` + --> $DIR/issue-96438.rs:4:5 + | +LL | cell.borrow_mut().unwrap().pop().unwrap(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ move occurs because value has type `Option>`, which does not implement the `Copy` trait + | +help: consider borrowing the `Option`'s content + | +LL | cell.borrow_mut().as_mut().unwrap().pop().unwrap(); + | +++++++++ +LL | cell.borrow_mut().as_ref().unwrap().pop().unwrap(); + | +++++++++ + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0507`. diff --git a/src/test/ui/suggestions/option-content-move.fixed b/src/test/ui/suggestions/option-content-move.fixed index ba16bcc8a336d..710a6172b0e87 100644 --- a/src/test/ui/suggestions/option-content-move.fixed +++ b/src/test/ui/suggestions/option-content-move.fixed @@ -8,7 +8,7 @@ impl LipogramCorpora { pub fn validate_all(&mut self) -> Result<(), char> { for selection in &self.selections { if selection.1.is_some() { - if selection.1.as_ref().unwrap().contains(selection.0) { + if selection.1.as_ref().as_mut().unwrap().contains(selection.0) { //~^ ERROR cannot move out of `selection.1` return Err(selection.0); } @@ -26,7 +26,7 @@ impl LipogramCorpora2 { pub fn validate_all(&mut self) -> Result<(), char> { for selection in &self.selections { if selection.1.is_ok() { - if selection.1.as_ref().unwrap().contains(selection.0) { + if selection.1.as_ref().as_mut().unwrap().contains(selection.0) { //~^ ERROR cannot move out of `selection.1` return Err(selection.0); } diff --git a/src/test/ui/suggestions/option-content-move.stderr b/src/test/ui/suggestions/option-content-move.stderr index a9e540084e590..950af8c4f2779 100644 --- a/src/test/ui/suggestions/option-content-move.stderr +++ b/src/test/ui/suggestions/option-content-move.stderr @@ -6,6 +6,8 @@ LL | if selection.1.unwrap().contains(selection.0) { | help: consider borrowing the `Option`'s content | +LL | if selection.1.as_mut().unwrap().contains(selection.0) { + | +++++++++ LL | if selection.1.as_ref().unwrap().contains(selection.0) { | +++++++++ @@ -17,6 +19,8 @@ LL | if selection.1.unwrap().contains(selection.0) { | help: consider borrowing the `Result`'s content | +LL | if selection.1.as_mut().unwrap().contains(selection.0) { + | +++++++++ LL | if selection.1.as_ref().unwrap().contains(selection.0) { | +++++++++ From 355dafa85a4840055dc63ea580ee6f0cbcf4af54 Mon Sep 17 00:00:00 2001 From: George <33588728+George-lewis@users.noreply.github.com> Date: Thu, 28 Apr 2022 02:02:51 -0400 Subject: [PATCH 2/4] Tidy --- src/test/ui/borrowck/issue-96438.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/ui/borrowck/issue-96438.rs b/src/test/ui/borrowck/issue-96438.rs index 3ff6a025df241..a180f86ceeb63 100644 --- a/src/test/ui/borrowck/issue-96438.rs +++ b/src/test/ui/borrowck/issue-96438.rs @@ -10,4 +10,4 @@ fn foo(cell: &RefCell>>) { fn main() { let cell = RefCell::new(None); foo(&cell); -} \ No newline at end of file +} From aee411f0dfd067991be70914a6f589fb6508799b Mon Sep 17 00:00:00 2001 From: George <33588728+George-lewis@users.noreply.github.com> Date: Thu, 28 Apr 2022 02:05:49 -0400 Subject: [PATCH 3/4] Lint --- .../src/diagnostics/move_errors.rs | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/compiler/rustc_borrowck/src/diagnostics/move_errors.rs b/compiler/rustc_borrowck/src/diagnostics/move_errors.rs index d23b6be2e1320..6381dd332e4b3 100644 --- a/compiler/rustc_borrowck/src/diagnostics/move_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/move_errors.rs @@ -4,7 +4,7 @@ use rustc_middle::ty; use rustc_mir_dataflow::move_paths::{ IllegalMoveOrigin, IllegalMoveOriginKind, LookupResult, MoveError, MovePathIndex, }; -use rustc_span::{sym, Span, BytePos}; +use rustc_span::{sym, BytePos, Span}; use crate::diagnostics::UseSpans; use crate::prefixes::PrefixSet; @@ -416,22 +416,22 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { && !has_complex_bindings { let span = match use_spans { - Some(UseSpans::FnSelfUse { var_span, fn_call_span, .. }) => { - // `fn_call_span` is the span of `unwrap()` - // This gets the 'variable' span excluding the unwrap - let span = var_span.until(fn_call_span); - - // Unfortunately the new span has a "." on the end which makes - // the suggestion format strangely, so we adjust the hi bound manually - span.with_hi(span.hi() - BytePos(1)) - }, - _ => span + Some(UseSpans::FnSelfUse { var_span, fn_call_span, .. }) => { + // `fn_call_span` is the span of `unwrap()` + // This gets the 'variable' span excluding the unwrap + let span = var_span.until(fn_call_span); + + // Unfortunately the new span has a "." on the end which makes + // the suggestion format strangely, so we adjust the hi bound manually + span.with_hi(span.hi() - BytePos(1)) + } + _ => span, }; err.span_suggestions( span.shrink_to_hi(), &format!("consider borrowing the `{}`'s content", diag_name.unwrap()), vec![".as_ref()".to_string(), ".as_mut()".to_string()].into_iter(), - Applicability::MachineApplicable + Applicability::MachineApplicable, ); } else if let Some(use_spans) = use_spans { self.explain_captures( From 58443e7f0af2c616dbd5bc71ced636313fb3c4b6 Mon Sep 17 00:00:00 2001 From: George <33588728+George-lewis@users.noreply.github.com> Date: Thu, 28 Apr 2022 02:21:01 -0400 Subject: [PATCH 4/4] Delete dead code --- compiler/rustc_borrowck/src/diagnostics/move_errors.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/compiler/rustc_borrowck/src/diagnostics/move_errors.rs b/compiler/rustc_borrowck/src/diagnostics/move_errors.rs index 6381dd332e4b3..bc288b161f3b8 100644 --- a/compiler/rustc_borrowck/src/diagnostics/move_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/move_errors.rs @@ -409,7 +409,6 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> { | ty::Opaque(def_id, _) => def_id, _ => return err, }; - // self.infcx.tcx. let diag_name = self.infcx.tcx.get_diagnostic_name(def_id); if matches!(diag_name, Some(sym::Option | sym::Result)) && use_spans.map_or(true, |v| !v.for_closure())