From 52d6ae854d0f4c33aa65ad1c151bf7e1700de476 Mon Sep 17 00:00:00 2001 From: ashtneoi Date: Tue, 10 Jul 2018 22:58:43 -0700 Subject: [PATCH] Reimplement some "add `mut`" suggestions under NLL Specifically, `&self` -> `&mut self` and explicit `ref` -> `ref mut`. Implicit `ref` isn't handled yet and causes an ICE. --- src/librustc_mir/borrow_check/mod.rs | 99 +++++++++++++------ src/librustc_mir/lib.rs | 2 + .../ui/did_you_mean/issue-38147-1.nll.stderr | 2 +- .../ui/did_you_mean/issue-39544.nll.stderr | 6 +- .../ui/suggestions/suggest-ref-mut.stderr | 14 ++- 5 files changed, 88 insertions(+), 35 deletions(-) diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index 1a66a2d2cb9..243f378377f 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -29,6 +29,8 @@ use rustc_data_structures::indexed_set::IdxSetBuf; use rustc_data_structures::indexed_vec::Idx; use rustc_data_structures::small_vec::SmallVec; +use core::unicode::property::Pattern_White_Space; + use std::rc::Rc; use syntax_pos::Span; @@ -1837,17 +1839,45 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { Place::Projection(box Projection { base: Place::Local(local), elem: ProjectionElem::Deref, - }) if self.mir.local_decls[*local].is_nonref_binding() => - { - let (err_help_span, suggested_code) = - find_place_to_suggest_ampmut(self.tcx, self.mir, *local); + }) if self.mir.local_decls[*local].is_user_variable.is_some() => { + let local_decl = &self.mir.local_decls[*local]; + let (err_help_span, suggested_code) = match local_decl.is_user_variable { + Some(ClearCrossCrate::Set(mir::BindingForm::ImplicitSelf)) => { + suggest_ampmut_self(local_decl) + }, + + Some(ClearCrossCrate::Set(mir::BindingForm::Var(mir::VarBindingForm { + binding_mode: ty::BindingMode::BindByValue(_), + opt_ty_info, + .. + }))) => { + if let Some(x) = try_suggest_ampmut_rhs( + self.tcx, self.mir, *local, + ) { + x + } else { + suggest_ampmut_type(local_decl, opt_ty_info) + } + }, + + Some(ClearCrossCrate::Set(mir::BindingForm::Var(mir::VarBindingForm { + binding_mode: ty::BindingMode::BindByReference(_), + .. + }))) => { + suggest_ref_mut(self.tcx, local_decl) + }, + + Some(ClearCrossCrate::Clear) => bug!("saw cleared local state"), + + None => bug!(), + }; + err.span_suggestion( err_help_span, "consider changing this to be a mutable reference", suggested_code, ); - let local_decl = &self.mir.local_decls[*local]; if let Some(name) = local_decl.name { err.span_label( span, @@ -1874,13 +1904,16 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { err.emit(); return true; - // Returns the span to highlight and the associated text to - // present when suggesting that the user use an `&mut`. - // + fn suggest_ampmut_self<'cx, 'gcx, 'tcx>( + local_decl: &mir::LocalDecl<'tcx>, + ) -> (Span, String) { + (local_decl.source_info.span, "&mut self".to_string()) + } + // When we want to suggest a user change a local variable to be a `&mut`, there // are three potential "obvious" things to highlight: // - // let ident [: Type] [= RightHandSideExresssion]; + // let ident [: Type] [= RightHandSideExpression]; // ^^^^^ ^^^^ ^^^^^^^^^^^^^^^^^^^^^^^ // (1.) (2.) (3.) // @@ -1889,48 +1922,58 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { // for example, if the RHS is present and the Type is not, then the type is going to // be inferred *from* the RHS, which means we should highlight that (and suggest // that they borrow the RHS mutably). - fn find_place_to_suggest_ampmut<'cx, 'gcx, 'tcx>( + // + // This implementation attempts to emulate AST-borrowck prioritization + // by trying (3.), then (2.) and finally falling back on (1.). + + fn try_suggest_ampmut_rhs<'cx, 'gcx, 'tcx>( tcx: TyCtxt<'cx, 'gcx, 'tcx>, mir: &Mir<'tcx>, local: Local, - ) -> (Span, String) { - // This implementation attempts to emulate AST-borrowck prioritization - // by trying (3.), then (2.) and finally falling back on (1.). + ) -> Option<(Span, String)> { let locations = mir.find_assignments(local); if locations.len() > 0 { let assignment_rhs_span = mir.source_info(locations[0]).span; let snippet = tcx.sess.codemap().span_to_snippet(assignment_rhs_span); if let Ok(src) = snippet { - // pnkfelix inherited code; believes intention is - // highlighted text will always be `&` and - // thus can transform to `&mut` by slicing off - // first ASCII character and prepending "&mut ". if src.starts_with('&') { let borrowed_expr = src[1..].to_string(); - return (assignment_rhs_span, format!("&mut {}", borrowed_expr)); + return Some((assignment_rhs_span, format!("&mut {}", borrowed_expr))); } } } + None + } - let local_decl = &mir.local_decls[local]; - let highlight_span = match local_decl.is_user_variable { + fn suggest_ampmut_type<'tcx>( + local_decl: &mir::LocalDecl<'tcx>, + opt_ty_info: Option, + ) -> (Span, String) { + let highlight_span = match opt_ty_info { // if this is a variable binding with an explicit type, // try to highlight that for the suggestion. - Some(ClearCrossCrate::Set(mir::BindingForm::Var(mir::VarBindingForm { - opt_ty_info: Some(ty_span), - .. - }))) => ty_span, - - Some(ClearCrossCrate::Clear) => bug!("saw cleared local state"), + Some(ty_span) => ty_span, // otherwise, just highlight the span associated with // the (MIR) LocalDecl. - _ => local_decl.source_info.span, + None => local_decl.source_info.span, }; let ty_mut = local_decl.ty.builtin_deref(true).unwrap(); assert_eq!(ty_mut.mutbl, hir::MutImmutable); - return (highlight_span, format!("&mut {}", ty_mut.ty)); + (highlight_span, format!("&mut {}", ty_mut.ty)) + } + + fn suggest_ref_mut<'cx, 'gcx, 'tcx>( + tcx: TyCtxt<'cx, 'gcx, 'tcx>, + local_decl: &mir::LocalDecl<'tcx>, + ) -> (Span, String) { + let hi_span = local_decl.source_info.span; + let hi_src = tcx.sess.codemap().span_to_snippet(hi_span).unwrap(); + assert!(hi_src.starts_with("ref")); + assert!(hi_src["ref".len()..].starts_with(Pattern_White_Space)); + let suggestion = format!("ref mut{}", &hi_src["ref".len()..]); + (hi_span, suggestion) } } diff --git a/src/librustc_mir/lib.rs b/src/librustc_mir/lib.rs index dc0d0b24463..92c0a2b475c 100644 --- a/src/librustc_mir/lib.rs +++ b/src/librustc_mir/lib.rs @@ -33,6 +33,7 @@ Rust MIR: a lowered representation of Rust. Also: an experiment! #![feature(never_type)] #![feature(specialization)] #![feature(try_trait)] +#![feature(unicode_internals)] #![recursion_limit="256"] @@ -56,6 +57,7 @@ extern crate rustc_target; extern crate log_settings; extern crate rustc_apfloat; extern crate byteorder; +extern crate core; mod diagnostics; diff --git a/src/test/ui/did_you_mean/issue-38147-1.nll.stderr b/src/test/ui/did_you_mean/issue-38147-1.nll.stderr index 76b8c8ebf60..d156d64b9d6 100644 --- a/src/test/ui/did_you_mean/issue-38147-1.nll.stderr +++ b/src/test/ui/did_you_mean/issue-38147-1.nll.stderr @@ -2,7 +2,7 @@ error[E0596]: cannot borrow immutable item `*self.s` as mutable --> $DIR/issue-38147-1.rs:27:9 | LL | fn f(&self) { - | ----- help: consider changing this to be a mutable reference: `&mut Foo<'_>` + | ----- help: consider changing this to be a mutable reference: `&mut self` LL | self.s.push('x'); //~ ERROR cannot borrow data mutably | ^^^^^^ `self` is a `&` reference, so the data it refers to cannot be borrowed as mutable diff --git a/src/test/ui/did_you_mean/issue-39544.nll.stderr b/src/test/ui/did_you_mean/issue-39544.nll.stderr index 02c1debca69..e2d2fcd63db 100644 --- a/src/test/ui/did_you_mean/issue-39544.nll.stderr +++ b/src/test/ui/did_you_mean/issue-39544.nll.stderr @@ -10,7 +10,7 @@ error[E0596]: cannot borrow immutable item `self.x` as mutable --> $DIR/issue-39544.rs:26:17 | LL | fn foo<'z>(&'z self) { - | -------- help: consider changing this to be a mutable reference: `&mut Z` + | -------- help: consider changing this to be a mutable reference: `&mut self` LL | let _ = &mut self.x; //~ ERROR cannot borrow | ^^^^^^^^^^^ `self` is a `&` reference, so the data it refers to cannot be borrowed as mutable @@ -18,7 +18,7 @@ error[E0596]: cannot borrow immutable item `self.x` as mutable --> $DIR/issue-39544.rs:30:17 | LL | fn foo1(&self, other: &Z) { - | ----- help: consider changing this to be a mutable reference: `&mut Z` + | ----- help: consider changing this to be a mutable reference: `&mut self` LL | let _ = &mut self.x; //~ ERROR cannot borrow | ^^^^^^^^^^^ `self` is a `&` reference, so the data it refers to cannot be borrowed as mutable @@ -35,7 +35,7 @@ error[E0596]: cannot borrow immutable item `self.x` as mutable --> $DIR/issue-39544.rs:35:17 | LL | fn foo2<'a>(&'a self, other: &Z) { - | -------- help: consider changing this to be a mutable reference: `&mut Z` + | -------- help: consider changing this to be a mutable reference: `&mut self` LL | let _ = &mut self.x; //~ ERROR cannot borrow | ^^^^^^^^^^^ `self` is a `&` reference, so the data it refers to cannot be borrowed as mutable diff --git a/src/test/ui/suggestions/suggest-ref-mut.stderr b/src/test/ui/suggestions/suggest-ref-mut.stderr index 6e151a995b8..d1590b3934e 100644 --- a/src/test/ui/suggestions/suggest-ref-mut.stderr +++ b/src/test/ui/suggestions/suggest-ref-mut.stderr @@ -1,20 +1,28 @@ error[E0594]: cannot assign to `*foo` which is behind a `&` reference --> $DIR/suggest-ref-mut.rs:17:5 | +LL | let ref foo = 16; + | ------- help: consider changing this to be a mutable reference: `ref mut foo` +... LL | *foo = 32; - | ^^^^^^^^^ cannot assign + | ^^^^^^^^^ `foo` is a `&` reference, so the data it refers to cannot be written error[E0594]: cannot assign to `*bar` which is behind a `&` reference --> $DIR/suggest-ref-mut.rs:22:9 | +LL | if let Some(ref bar) = Some(16) { + | ------- help: consider changing this to be a mutable reference: `ref mut bar` +... LL | *bar = 32; - | ^^^^^^^^^ cannot assign + | ^^^^^^^^^ `bar` is a `&` reference, so the data it refers to cannot be written error[E0594]: cannot assign to `*quo` which is behind a `&` reference --> $DIR/suggest-ref-mut.rs:26:22 | LL | ref quo => { *quo = 32; }, - | ^^^^^^^^^ cannot assign + | ------- ^^^^^^^^^ `quo` is a `&` reference, so the data it refers to cannot be written + | | + | help: consider changing this to be a mutable reference: `ref mut quo` error: aborting due to 3 previous errors