From 6384568fdb3ca50807aa303c7cec7cc920e38561 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Sun, 31 Dec 2017 15:32:41 -0800 Subject: [PATCH] Reword trying to operate in immutable fields The previous message ("cannot assign/mutably borrow immutable field") when trying to modify a field of an immutable binding gave the (incorrect) impression that fields can be mutable independently of their ADT's binding. Slightly reword the message to read "cannot assign/mutably borrow field of immutable binding". --- src/librustc_borrowck/borrowck/mod.rs | 64 ++++++++++++------- src/test/compile-fail/issue-5500-1.rs | 4 +- src/test/ui/did_you_mean/issue-35937.stderr | 12 ++-- src/test/ui/did_you_mean/issue-38147-2.rs | 3 +- src/test/ui/did_you_mean/issue-38147-2.stderr | 4 +- src/test/ui/did_you_mean/issue-38147-3.rs | 3 +- src/test/ui/did_you_mean/issue-38147-3.stderr | 4 +- src/test/ui/did_you_mean/issue-39544.rs | 3 +- src/test/ui/did_you_mean/issue-39544.stderr | 48 +++++++------- .../borrowck-call-is-borrow-issue-12224.rs | 2 +- ...borrowck-call-is-borrow-issue-12224.stderr | 2 +- 11 files changed, 85 insertions(+), 64 deletions(-) diff --git a/src/librustc_borrowck/borrowck/mod.rs b/src/librustc_borrowck/borrowck/mod.rs index b124872ba12..c7a13b42028 100644 --- a/src/librustc_borrowck/borrowck/mod.rs +++ b/src/librustc_borrowck/borrowck/mod.rs @@ -484,54 +484,60 @@ fn common(&self, other: &LoanPath<'tcx>) -> Option> { } } -pub fn opt_loan_path<'tcx>(cmt: &mc::cmt<'tcx>) -> Option>> { - //! Computes the `LoanPath` (if any) for a `cmt`. - //! Note that this logic is somewhat duplicated in - //! the method `compute()` found in `gather_loans::restrictions`, - //! which allows it to share common loan path pieces as it - //! traverses the CMT. - +// Avoid "cannot borrow immutable field `self.x` as mutable" as that implies that a field *can* be +// mutable independently of the struct it belongs to. (#35937) +pub fn opt_loan_path_is_field<'tcx>(cmt: &mc::cmt<'tcx>) -> (Option>>, bool) { let new_lp = |v: LoanPathKind<'tcx>| Rc::new(LoanPath::new(v, cmt.ty)); match cmt.cat { Categorization::Rvalue(..) | Categorization::StaticItem => { - None + (None, false) } Categorization::Local(id) => { - Some(new_lp(LpVar(id))) + (Some(new_lp(LpVar(id))), false) } Categorization::Upvar(mc::Upvar { id, .. }) => { - Some(new_lp(LpUpvar(id))) + (Some(new_lp(LpUpvar(id))), false) } Categorization::Deref(ref cmt_base, pk) => { - opt_loan_path(cmt_base).map(|lp| { + let lp = opt_loan_path_is_field(cmt_base); + (lp.0.map(|lp| { new_lp(LpExtend(lp, cmt.mutbl, LpDeref(pk))) - }) + }), lp.1) } Categorization::Interior(ref cmt_base, ik) => { - opt_loan_path(cmt_base).map(|lp| { + (opt_loan_path(cmt_base).map(|lp| { let opt_variant_id = match cmt_base.cat { Categorization::Downcast(_, did) => Some(did), _ => None }; new_lp(LpExtend(lp, cmt.mutbl, LpInterior(opt_variant_id, ik.cleaned()))) - }) + }), true) } - Categorization::Downcast(ref cmt_base, variant_def_id) => - opt_loan_path(cmt_base) - .map(|lp| { + Categorization::Downcast(ref cmt_base, variant_def_id) => { + let lp = opt_loan_path_is_field(cmt_base); + (lp.0.map(|lp| { new_lp(LpDowncast(lp, variant_def_id)) - }), - + }), lp.1) + } } } +/// Computes the `LoanPath` (if any) for a `cmt`. +/// Note that this logic is somewhat duplicated in +/// the method `compute()` found in `gather_loans::restrictions`, +/// which allows it to share common loan path pieces as it +/// traverses the CMT. +pub fn opt_loan_path<'tcx>(cmt: &mc::cmt<'tcx>) -> Option>> { + opt_loan_path_is_field(cmt).0 +} + /////////////////////////////////////////////////////////////////////////// // Errors @@ -787,14 +793,26 @@ fn report_bckerr(&self, err: &BckError<'tcx>) { mc::NoteClosureEnv(_) | mc::NoteUpvarRef(_) => { self.cmt_to_string(&err.cmt) } - _ => match opt_loan_path(&err.cmt) { - None => { + _ => match opt_loan_path_is_field(&err.cmt) { + (None, true) => { + format!("{} of {} binding", + self.cmt_to_string(&err.cmt), + err.cmt.mutbl.to_user_str()) + + } + (None, false) => { format!("{} {}", err.cmt.mutbl.to_user_str(), self.cmt_to_string(&err.cmt)) } - Some(lp) => { + (Some(lp), true) => { + format!("{} `{}` of {} binding", + self.cmt_to_string(&err.cmt), + self.loan_path_to_string(&lp), + err.cmt.mutbl.to_user_str()) + } + (Some(lp), false) => { format!("{} {} `{}`", err.cmt.mutbl.to_user_str(), self.cmt_to_string(&err.cmt), @@ -1326,7 +1344,7 @@ fn note_and_explain_mutbl_error(&self, db: &mut DiagnosticBuilder, err: &BckErro } else if let Categorization::Interior(ref cmt, _) = err.cmt.cat { if let mc::MutabilityCategory::McImmutable = cmt.mutbl { db.span_label(*error_span, - "cannot mutably borrow immutable field"); + "cannot mutably borrow field of immutable binding"); } } } diff --git a/src/test/compile-fail/issue-5500-1.rs b/src/test/compile-fail/issue-5500-1.rs index 75ff0a12101..4bd147a5e1c 100644 --- a/src/test/compile-fail/issue-5500-1.rs +++ b/src/test/compile-fail/issue-5500-1.rs @@ -18,8 +18,8 @@ struct TrieMapIterator<'a> { fn main() { let a = 5; let _iter = TrieMapIterator{node: &a}; - _iter.node = & //[ast]~ ERROR cannot assign to immutable field `_iter.node` - //[mir]~^ ERROR cannot assign to immutable field `_iter.node` (Ast) + _iter.node = & //[ast]~ ERROR cannot assign to field `_iter.node` of immutable binding + //[mir]~^ ERROR cannot assign to field `_iter.node` of immutable binding (Ast) // MIR doesn't generate an error because the code isn't reachable. This is OK // because the test is here to check that the compiler doesn't ICE (cf. #5500). panic!() diff --git a/src/test/ui/did_you_mean/issue-35937.stderr b/src/test/ui/did_you_mean/issue-35937.stderr index ec44755cb7c..cfaff973170 100644 --- a/src/test/ui/did_you_mean/issue-35937.stderr +++ b/src/test/ui/did_you_mean/issue-35937.stderr @@ -1,26 +1,26 @@ -error[E0596]: cannot borrow immutable field `f.v` as mutable +error[E0596]: cannot borrow field `f.v` of immutable binding as mutable --> $DIR/issue-35937.rs:17:5 | 16 | let f = Foo { v: Vec::new() }; | - consider changing this to `mut f` 17 | f.v.push("cat".to_string()); //~ ERROR cannot borrow - | ^^^ cannot mutably borrow immutable field + | ^^^ cannot mutably borrow field of immutable binding -error[E0594]: cannot assign to immutable field `s.x` +error[E0594]: cannot assign to field `s.x` of immutable binding --> $DIR/issue-35937.rs:26:5 | 25 | let s = S { x: 42 }; | - consider changing this to `mut s` 26 | s.x += 1; //~ ERROR cannot assign - | ^^^^^^^^ cannot mutably borrow immutable field + | ^^^^^^^^ cannot mutably borrow field of immutable binding -error[E0594]: cannot assign to immutable field `s.x` +error[E0594]: cannot assign to field `s.x` of immutable binding --> $DIR/issue-35937.rs:30:5 | 29 | fn bar(s: S) { | - consider changing this to `mut s` 30 | s.x += 1; //~ ERROR cannot assign - | ^^^^^^^^ cannot mutably borrow immutable field + | ^^^^^^^^ cannot mutably borrow field of immutable binding error: aborting due to 3 previous errors diff --git a/src/test/ui/did_you_mean/issue-38147-2.rs b/src/test/ui/did_you_mean/issue-38147-2.rs index cc6be98bcf8..1a24f471f08 100644 --- a/src/test/ui/did_you_mean/issue-38147-2.rs +++ b/src/test/ui/did_you_mean/issue-38147-2.rs @@ -14,7 +14,8 @@ struct Bar<'a> { impl<'a> Bar<'a> { fn f(&mut self) { - self.s.push('x'); //~ ERROR cannot borrow immutable borrowed + self.s.push('x'); + //~^ ERROR cannot borrow borrowed content `*self.s` of immutable binding as mutable } } diff --git a/src/test/ui/did_you_mean/issue-38147-2.stderr b/src/test/ui/did_you_mean/issue-38147-2.stderr index b09ecf9057c..569bfa11803 100644 --- a/src/test/ui/did_you_mean/issue-38147-2.stderr +++ b/src/test/ui/did_you_mean/issue-38147-2.stderr @@ -1,10 +1,10 @@ -error[E0596]: cannot borrow immutable borrowed content `*self.s` as mutable +error[E0596]: cannot borrow borrowed content `*self.s` of immutable binding as mutable --> $DIR/issue-38147-2.rs:17:9 | 12 | s: &'a String | ---------- use `&'a mut String` here to make mutable ... -17 | self.s.push('x'); //~ ERROR cannot borrow immutable borrowed +17 | self.s.push('x'); | ^^^^^^ cannot borrow as mutable error: aborting due to previous error diff --git a/src/test/ui/did_you_mean/issue-38147-3.rs b/src/test/ui/did_you_mean/issue-38147-3.rs index 42b29100517..5e2bf06e5d9 100644 --- a/src/test/ui/did_you_mean/issue-38147-3.rs +++ b/src/test/ui/did_you_mean/issue-38147-3.rs @@ -14,7 +14,8 @@ struct Qux<'a> { impl<'a> Qux<'a> { fn f(&self) { - self.s.push('x'); //~ ERROR cannot borrow immutable borrowed + self.s.push('x'); + //~^ ERROR cannot borrow borrowed content `*self.s` of immutable binding as mutable } } diff --git a/src/test/ui/did_you_mean/issue-38147-3.stderr b/src/test/ui/did_you_mean/issue-38147-3.stderr index ca721f133a4..75d904d394a 100644 --- a/src/test/ui/did_you_mean/issue-38147-3.stderr +++ b/src/test/ui/did_you_mean/issue-38147-3.stderr @@ -1,10 +1,10 @@ -error[E0596]: cannot borrow immutable borrowed content `*self.s` as mutable +error[E0596]: cannot borrow borrowed content `*self.s` of immutable binding as mutable --> $DIR/issue-38147-3.rs:17:9 | 12 | s: &'a String | ---------- use `&'a mut String` here to make mutable ... -17 | self.s.push('x'); //~ ERROR cannot borrow immutable borrowed +17 | self.s.push('x'); | ^^^^^^ cannot borrow as mutable error: aborting due to previous error diff --git a/src/test/ui/did_you_mean/issue-39544.rs b/src/test/ui/did_you_mean/issue-39544.rs index 7cd7768078a..205cbce7094 100644 --- a/src/test/ui/did_you_mean/issue-39544.rs +++ b/src/test/ui/did_you_mean/issue-39544.rs @@ -55,5 +55,6 @@ pub fn with_arg(z: Z, w: &Z) { pub fn with_tuple() { let mut y = 0; let x = (&y,); - *x.0 = 1; //~ ERROR cannot assign to immutable borrowed content + *x.0 = 1; + //~^ ERROR cannot assign to borrowed content `*x.0` of immutable binding } diff --git a/src/test/ui/did_you_mean/issue-39544.stderr b/src/test/ui/did_you_mean/issue-39544.stderr index 1fcb05374f6..d8c089806cd 100644 --- a/src/test/ui/did_you_mean/issue-39544.stderr +++ b/src/test/ui/did_you_mean/issue-39544.stderr @@ -1,99 +1,99 @@ -error[E0596]: cannot borrow immutable field `z.x` as mutable +error[E0596]: cannot borrow field `z.x` of immutable binding as mutable --> $DIR/issue-39544.rs:21:18 | 20 | let z = Z { x: X::Y }; | - consider changing this to `mut z` 21 | let _ = &mut z.x; //~ ERROR cannot borrow - | ^^^ cannot mutably borrow immutable field + | ^^^ cannot mutably borrow field of immutable binding -error[E0596]: cannot borrow immutable field `self.x` as mutable +error[E0596]: cannot borrow field `self.x` of immutable binding as mutable --> $DIR/issue-39544.rs:26:22 | 25 | fn foo<'z>(&'z self) { | -------- use `&'z mut self` here to make mutable 26 | let _ = &mut self.x; //~ ERROR cannot borrow - | ^^^^^^ cannot mutably borrow immutable field + | ^^^^^^ cannot mutably borrow field of immutable binding -error[E0596]: cannot borrow immutable field `self.x` as mutable +error[E0596]: cannot borrow field `self.x` of immutable binding as mutable --> $DIR/issue-39544.rs:30:22 | 29 | fn foo1(&self, other: &Z) { | ----- use `&mut self` here to make mutable 30 | let _ = &mut self.x; //~ ERROR cannot borrow - | ^^^^^^ cannot mutably borrow immutable field + | ^^^^^^ cannot mutably borrow field of immutable binding -error[E0596]: cannot borrow immutable field `other.x` as mutable +error[E0596]: cannot borrow field `other.x` of immutable binding as mutable --> $DIR/issue-39544.rs:31:22 | 29 | fn foo1(&self, other: &Z) { | -- use `&mut Z` here to make mutable 30 | let _ = &mut self.x; //~ ERROR cannot borrow 31 | let _ = &mut other.x; //~ ERROR cannot borrow - | ^^^^^^^ cannot mutably borrow immutable field + | ^^^^^^^ cannot mutably borrow field of immutable binding -error[E0596]: cannot borrow immutable field `self.x` as mutable +error[E0596]: cannot borrow field `self.x` of immutable binding as mutable --> $DIR/issue-39544.rs:35:22 | 34 | fn foo2<'a>(&'a self, other: &Z) { | -------- use `&'a mut self` here to make mutable 35 | let _ = &mut self.x; //~ ERROR cannot borrow - | ^^^^^^ cannot mutably borrow immutable field + | ^^^^^^ cannot mutably borrow field of immutable binding -error[E0596]: cannot borrow immutable field `other.x` as mutable +error[E0596]: cannot borrow field `other.x` of immutable binding as mutable --> $DIR/issue-39544.rs:36:22 | 34 | fn foo2<'a>(&'a self, other: &Z) { | -- use `&mut Z` here to make mutable 35 | let _ = &mut self.x; //~ ERROR cannot borrow 36 | let _ = &mut other.x; //~ ERROR cannot borrow - | ^^^^^^^ cannot mutably borrow immutable field + | ^^^^^^^ cannot mutably borrow field of immutable binding -error[E0596]: cannot borrow immutable field `self.x` as mutable +error[E0596]: cannot borrow field `self.x` of immutable binding as mutable --> $DIR/issue-39544.rs:40:22 | 39 | fn foo3<'a>(self: &'a Self, other: &Z) { | -------- use `&'a mut Self` here to make mutable 40 | let _ = &mut self.x; //~ ERROR cannot borrow - | ^^^^^^ cannot mutably borrow immutable field + | ^^^^^^ cannot mutably borrow field of immutable binding -error[E0596]: cannot borrow immutable field `other.x` as mutable +error[E0596]: cannot borrow field `other.x` of immutable binding as mutable --> $DIR/issue-39544.rs:41:22 | 39 | fn foo3<'a>(self: &'a Self, other: &Z) { | -- use `&mut Z` here to make mutable 40 | let _ = &mut self.x; //~ ERROR cannot borrow 41 | let _ = &mut other.x; //~ ERROR cannot borrow - | ^^^^^^^ cannot mutably borrow immutable field + | ^^^^^^^ cannot mutably borrow field of immutable binding -error[E0596]: cannot borrow immutable field `other.x` as mutable +error[E0596]: cannot borrow field `other.x` of immutable binding as mutable --> $DIR/issue-39544.rs:45:22 | 44 | fn foo4(other: &Z) { | -- use `&mut Z` here to make mutable 45 | let _ = &mut other.x; //~ ERROR cannot borrow - | ^^^^^^^ cannot mutably borrow immutable field + | ^^^^^^^ cannot mutably borrow field of immutable binding -error[E0596]: cannot borrow immutable field `z.x` as mutable +error[E0596]: cannot borrow field `z.x` of immutable binding as mutable --> $DIR/issue-39544.rs:51:18 | 50 | pub fn with_arg(z: Z, w: &Z) { | - consider changing this to `mut z` 51 | let _ = &mut z.x; //~ ERROR cannot borrow - | ^^^ cannot mutably borrow immutable field + | ^^^ cannot mutably borrow field of immutable binding -error[E0596]: cannot borrow immutable field `w.x` as mutable +error[E0596]: cannot borrow field `w.x` of immutable binding as mutable --> $DIR/issue-39544.rs:52:18 | 50 | pub fn with_arg(z: Z, w: &Z) { | -- use `&mut Z` here to make mutable 51 | let _ = &mut z.x; //~ ERROR cannot borrow 52 | let _ = &mut w.x; //~ ERROR cannot borrow - | ^^^ cannot mutably borrow immutable field + | ^^^ cannot mutably borrow field of immutable binding -error[E0594]: cannot assign to immutable borrowed content `*x.0` +error[E0594]: cannot assign to borrowed content `*x.0` of immutable binding --> $DIR/issue-39544.rs:58:5 | -58 | *x.0 = 1; //~ ERROR cannot assign to immutable borrowed content +58 | *x.0 = 1; | ^^^^^^^^ cannot borrow as mutable error: aborting due to 12 previous errors diff --git a/src/test/ui/span/borrowck-call-is-borrow-issue-12224.rs b/src/test/ui/span/borrowck-call-is-borrow-issue-12224.rs index 66673c152d5..8e03c303e54 100644 --- a/src/test/ui/span/borrowck-call-is-borrow-issue-12224.rs +++ b/src/test/ui/span/borrowck-call-is-borrow-issue-12224.rs @@ -42,7 +42,7 @@ fn test3(f: &mut F) where F: FnMut() { fn test4(f: &Test) { f.f.call_mut(()) - //~^ ERROR: cannot borrow immutable `Box` content `*f.f` as mutable + //~^ ERROR: cannot borrow `Box` content `*f.f` of immutable binding as mutable } fn test5(f: &mut Test) { diff --git a/src/test/ui/span/borrowck-call-is-borrow-issue-12224.stderr b/src/test/ui/span/borrowck-call-is-borrow-issue-12224.stderr index 542ee997046..581b366af1c 100644 --- a/src/test/ui/span/borrowck-call-is-borrow-issue-12224.stderr +++ b/src/test/ui/span/borrowck-call-is-borrow-issue-12224.stderr @@ -19,7 +19,7 @@ error[E0596]: cannot borrow immutable borrowed content `*f` as mutable 35 | (*f)(); | ^^^^ cannot borrow as mutable -error[E0596]: cannot borrow immutable `Box` content `*f.f` as mutable +error[E0596]: cannot borrow `Box` content `*f.f` of immutable binding as mutable --> $DIR/borrowck-call-is-borrow-issue-12224.rs:44:5 | 43 | fn test4(f: &Test) {