From 69bded2493c964cb1d5cce4f708d221e242aff41 Mon Sep 17 00:00:00 2001 From: David Wood Date: Mon, 24 Dec 2018 20:22:25 +0100 Subject: [PATCH 1/8] Add union justifications to conflicting borrows. This commit adds justifications to error messages for conflicting borrows of union fields. Where previously an error message would say ``cannot borrow `u.b` as mutable..``, it now says ``cannot borrow `u` (via `u.b`) as mutable..``. --- .../borrow_check/error_reporting.rs | 58 +++++++++++--- ...tderr => borrowck-union-borrow.nll.stderr} | 79 ++++++++----------- src/test/ui/borrowck/borrowck-union-borrow.rs | 38 +++------ ...st.stderr => borrowck-union-borrow.stderr} | 60 +++++++------- src/test/ui/issues/issue-45157.rs | 5 +- src/test/ui/issues/issue-45157.stderr | 10 +-- ...nion-borrow-move-parent-sibling.nll.stderr | 18 ++--- 7 files changed, 141 insertions(+), 127 deletions(-) rename src/test/ui/borrowck/{borrowck-union-borrow.ast.nll.stderr => borrowck-union-borrow.nll.stderr} (50%) rename src/test/ui/borrowck/{borrowck-union-borrow.ast.stderr => borrowck-union-borrow.stderr} (64%) diff --git a/src/librustc_mir/borrow_check/error_reporting.rs b/src/librustc_mir/borrow_check/error_reporting.rs index b072e464a2998..6f12ff994c4b2 100644 --- a/src/librustc_mir/borrow_check/error_reporting.rs +++ b/src/librustc_mir/borrow_check/error_reporting.rs @@ -329,10 +329,16 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { "closure" }; - let desc_place = self.describe_place(place).unwrap_or_else(|| "_".to_owned()); - let tcx = self.infcx.tcx; - - let first_borrow_desc; + let (desc_place, msg_place, msg_borrow) = if issued_borrow.borrowed_place == *place { + let desc_place = self.describe_place(place).unwrap_or_else(|| "_".to_owned()); + (desc_place, "".to_string(), "".to_string()) + } else { + let (desc_place, msg_place) = self.describe_place_for_conflicting_borrow(place); + let (_, msg_borrow) = self.describe_place_for_conflicting_borrow( + &issued_borrow.borrowed_place + ); + (desc_place, msg_place, msg_borrow) + }; let explanation = self.explain_why_borrow_contains_point(context, issued_borrow, None); let second_borrow_desc = if explanation.is_explained() { @@ -342,6 +348,8 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { }; // FIXME: supply non-"" `opt_via` when appropriate + let tcx = self.infcx.tcx; + let first_borrow_desc; let mut err = match ( gen_borrow_kind, "immutable", @@ -355,12 +363,12 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { tcx.cannot_reborrow_already_borrowed( span, &desc_place, - "", + &msg_place, lft, issued_span, "it", rgt, - "", + &msg_borrow, None, Origin::Mir, ) @@ -370,12 +378,12 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { tcx.cannot_reborrow_already_borrowed( span, &desc_place, - "", + &msg_place, lft, issued_span, "it", rgt, - "", + &msg_borrow, None, Origin::Mir, ) @@ -386,9 +394,9 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { tcx.cannot_mutably_borrow_multiply( span, &desc_place, - "", + &msg_place, issued_span, - "", + &msg_borrow, None, Origin::Mir, ) @@ -518,6 +526,36 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { err.buffer(&mut self.errors_buffer); } + /// Returns a description of a place and an associated message for the purposes of conflicting + /// borrow diagnostics. + /// + /// If the borrow is of the field `b` of a union `u`, then the return value will be + /// `("u", " (via \`u.b\`)")`. Otherwise, for some variable `a`, the return value will be + /// `("a", "")`. + pub(super) fn describe_place_for_conflicting_borrow( + &self, + place: &Place<'tcx>, + ) -> (String, String) { + place.base_local() + .filter(|local| { + // Filter out non-unions. + self.mir.local_decls[*local].ty + .ty_adt_def() + .map(|adt| adt.is_union()) + .unwrap_or(false) + }) + .and_then(|local| { + let desc_base = self.describe_place(&Place::Local(local)) + .unwrap_or_else(|| "_".to_owned()); + let desc_original = self.describe_place(place) + .unwrap_or_else(|| "_".to_owned()); + return Some((desc_base, format!(" (via `{}`)", desc_original))); + }) + .unwrap_or_else(|| { + (self.describe_place(place).unwrap_or_else(|| "_".to_owned()), "".to_string()) + }) + } + /// Reports StorageDeadOrDrop of `place` conflicts with `borrow`. /// /// This means that some data referenced by `borrow` needs to live diff --git a/src/test/ui/borrowck/borrowck-union-borrow.ast.nll.stderr b/src/test/ui/borrowck/borrowck-union-borrow.nll.stderr similarity index 50% rename from src/test/ui/borrowck/borrowck-union-borrow.ast.nll.stderr rename to src/test/ui/borrowck/borrowck-union-borrow.nll.stderr index 1a2433c8f6afd..ef5dcef04b074 100644 --- a/src/test/ui/borrowck/borrowck-union-borrow.ast.nll.stderr +++ b/src/test/ui/borrowck/borrowck-union-borrow.nll.stderr @@ -1,132 +1,121 @@ error[E0502]: cannot borrow `u.a` as mutable because it is also borrowed as immutable - --> $DIR/borrowck-union-borrow.rs:27:23 + --> $DIR/borrowck-union-borrow.rs:25:23 | LL | let ra = &u.a; | ---- immutable borrow occurs here -LL | let rma = &mut u.a; //[ast]~ ERROR cannot borrow `u.a` as mutable because it is also borrowed as immutable +LL | let rma = &mut u.a; //~ ERROR cannot borrow `u.a` as mutable because it is also borrowed as immutable | ^^^^^^^^ mutable borrow occurs here -LL | //[mir]~^ ERROR cannot borrow `u.a` as mutable because it is also borrowed as immutable LL | drop(ra); | -- immutable borrow later used here error[E0506]: cannot assign to `u.a` because it is borrowed - --> $DIR/borrowck-union-borrow.rs:33:13 + --> $DIR/borrowck-union-borrow.rs:30:13 | LL | let ra = &u.a; | ---- borrow of `u.a` occurs here -LL | u.a = 1; //[ast]~ ERROR cannot assign to `u.a` because it is borrowed +LL | u.a = 1; //~ ERROR cannot assign to `u.a` because it is borrowed | ^^^^^^^ assignment to borrowed `u.a` occurs here -LL | //[mir]~^ ERROR cannot assign to `u.a` because it is borrowed LL | drop(ra); | -- borrow later used here -error[E0502]: cannot borrow `u.b` as mutable because it is also borrowed as immutable - --> $DIR/borrowck-union-borrow.rs:50:23 +error[E0502]: cannot borrow `u` (via `u.b`) as mutable because it is also borrowed as immutable (via `u.a`) + --> $DIR/borrowck-union-borrow.rs:46:23 | LL | let ra = &u.a; - | ---- immutable borrow occurs here -LL | let rmb = &mut u.b; //[ast]~ ERROR cannot borrow `u` (via `u.b`) as mutable because `u` is also borrowed as immutable (via `u.a`) - | ^^^^^^^^ mutable borrow occurs here -LL | //[mir]~^ ERROR cannot borrow `u.b` as mutable because it is also borrowed as immutable + | ---- immutable borrow occurs here (via `u.a`) +LL | let rmb = &mut u.b; //~ ERROR cannot borrow `u` (via `u.b`) as mutable because `u` is also borrowed as immutable (via `u.a`) + | ^^^^^^^^ mutable borrow occurs here (via `u.b`) LL | drop(ra); | -- immutable borrow later used here error[E0506]: cannot assign to `u.b` because it is borrowed - --> $DIR/borrowck-union-borrow.rs:56:13 + --> $DIR/borrowck-union-borrow.rs:51:13 | LL | let ra = &u.a; | ---- borrow of `u.b` occurs here -LL | u.b = 1; //[ast]~ ERROR cannot assign to `u.b` because it is borrowed +LL | u.b = 1; //~ ERROR cannot assign to `u.b` because it is borrowed | ^^^^^^^ assignment to borrowed `u.b` occurs here -LL | //[mir]~^ ERROR cannot assign to `u.b` because it is borrowed LL | drop(ra); | -- borrow later used here error[E0502]: cannot borrow `u.a` as immutable because it is also borrowed as mutable - --> $DIR/borrowck-union-borrow.rs:63:22 + --> $DIR/borrowck-union-borrow.rs:57:22 | LL | let rma = &mut u.a; | -------- mutable borrow occurs here -LL | let ra = &u.a; //[ast]~ ERROR cannot borrow `u.a` as immutable because it is also borrowed as mutable +LL | let ra = &u.a; //~ ERROR cannot borrow `u.a` as immutable because it is also borrowed as mutable | ^^^^ immutable borrow occurs here -LL | //[mir]~^ ERROR cannot borrow `u.a` as immutable because it is also borrowed as mutable LL | drop(rma); | --- mutable borrow later used here error[E0503]: cannot use `u.a` because it was mutably borrowed - --> $DIR/borrowck-union-borrow.rs:69:21 + --> $DIR/borrowck-union-borrow.rs:62:21 | LL | let ra = &mut u.a; | -------- borrow of `u.a` occurs here -LL | let a = u.a; //[ast]~ ERROR cannot use `u.a` because it was mutably borrowed +LL | let a = u.a; //~ ERROR cannot use `u.a` because it was mutably borrowed | ^^^ use of borrowed `u.a` -LL | //[mir]~^ ERROR cannot use `u.a` because it was mutably borrowed LL | drop(ra); | -- borrow later used here error[E0499]: cannot borrow `u.a` as mutable more than once at a time - --> $DIR/borrowck-union-borrow.rs:75:24 + --> $DIR/borrowck-union-borrow.rs:67:24 | LL | let rma = &mut u.a; | -------- first mutable borrow occurs here -LL | let rma2 = &mut u.a; //[ast]~ ERROR cannot borrow `u.a` as mutable more than once at a time +LL | let rma2 = &mut u.a; //~ ERROR cannot borrow `u.a` as mutable more than once at a time | ^^^^^^^^ second mutable borrow occurs here -LL | //[mir]~^ ERROR cannot borrow `u.a` as mutable more than once at a time LL | drop(rma); | --- first borrow later used here error[E0506]: cannot assign to `u.a` because it is borrowed - --> $DIR/borrowck-union-borrow.rs:81:13 + --> $DIR/borrowck-union-borrow.rs:72:13 | LL | let rma = &mut u.a; | -------- borrow of `u.a` occurs here -LL | u.a = 1; //[ast]~ ERROR cannot assign to `u.a` because it is borrowed +LL | u.a = 1; //~ ERROR cannot assign to `u.a` because it is borrowed | ^^^^^^^ assignment to borrowed `u.a` occurs here -LL | //[mir]~^ ERROR cannot assign to `u.a` because it is borrowed LL | drop(rma); | --- borrow later used here -error[E0502]: cannot borrow `u.b` as immutable because it is also borrowed as mutable - --> $DIR/borrowck-union-borrow.rs:88:22 +error[E0502]: cannot borrow `u` (via `u.b`) as immutable because it is also borrowed as mutable (via `u.a`) + --> $DIR/borrowck-union-borrow.rs:78:22 | LL | let rma = &mut u.a; - | -------- mutable borrow occurs here -LL | let rb = &u.b; //[ast]~ ERROR cannot borrow `u` (via `u.b`) as immutable because `u` is also borrowed as mutable (via `u.a`) - | ^^^^ immutable borrow occurs here -LL | //[mir]~^ ERROR cannot borrow `u.b` as immutable because it is also borrowed as mutable + | -------- mutable borrow occurs here (via `u.a`) +LL | let rb = &u.b; //~ ERROR cannot borrow `u` (via `u.b`) as immutable because `u` is also borrowed as mutable (via `u.a`) + | ^^^^ immutable borrow occurs here (via `u.b`) LL | drop(rma); | --- mutable borrow later used here error[E0503]: cannot use `u.b` because it was mutably borrowed - --> $DIR/borrowck-union-borrow.rs:94:21 + --> $DIR/borrowck-union-borrow.rs:83:21 | LL | let ra = &mut u.a; | -------- borrow of `u.a` occurs here -LL | let b = u.b; //[ast]~ ERROR cannot use `u.b` because it was mutably borrowed +LL | let b = u.b; //~ ERROR cannot use `u.b` because it was mutably borrowed | ^^^ use of borrowed `u.a` -... +LL | LL | drop(ra); | -- borrow later used here -error[E0499]: cannot borrow `u.b` as mutable more than once at a time - --> $DIR/borrowck-union-borrow.rs:101:24 +error[E0499]: cannot borrow `u` (via `u.b`) as mutable more than once at a time + --> $DIR/borrowck-union-borrow.rs:89:24 | LL | let rma = &mut u.a; - | -------- first mutable borrow occurs here -LL | let rmb2 = &mut u.b; //[ast]~ ERROR cannot borrow `u` (via `u.b`) as mutable more than once at a time - | ^^^^^^^^ second mutable borrow occurs here -LL | //[mir]~^ ERROR cannot borrow `u.b` as mutable more than once at a time + | -------- first mutable borrow occurs here (via `u.a`) +LL | let rmb2 = &mut u.b; //~ ERROR cannot borrow `u` (via `u.b`) as mutable more than once at a time + | ^^^^^^^^ second mutable borrow occurs here (via `u.b`) LL | drop(rma); | --- first borrow later used here error[E0506]: cannot assign to `u.b` because it is borrowed - --> $DIR/borrowck-union-borrow.rs:107:13 + --> $DIR/borrowck-union-borrow.rs:94:13 | LL | let rma = &mut u.a; | -------- borrow of `u.b` occurs here -LL | u.b = 1; //[ast]~ ERROR cannot assign to `u.b` because it is borrowed +LL | u.b = 1; //~ ERROR cannot assign to `u.b` because it is borrowed | ^^^^^^^ assignment to borrowed `u.b` occurs here -LL | //[mir]~^ ERROR cannot assign to `u.b` because it is borrowed LL | drop(rma); | --- borrow later used here diff --git a/src/test/ui/borrowck/borrowck-union-borrow.rs b/src/test/ui/borrowck/borrowck-union-borrow.rs index 62647a2f5e85e..8afc0be8b55c5 100644 --- a/src/test/ui/borrowck/borrowck-union-borrow.rs +++ b/src/test/ui/borrowck/borrowck-union-borrow.rs @@ -1,6 +1,4 @@ // ignore-tidy-linelength -// revisions: ast mir -//[mir]compile-flags: -Z borrowck=mir #[derive(Clone, Copy)] union U { @@ -24,14 +22,12 @@ fn main() { } { let ra = &u.a; - let rma = &mut u.a; //[ast]~ ERROR cannot borrow `u.a` as mutable because it is also borrowed as immutable - //[mir]~^ ERROR cannot borrow `u.a` as mutable because it is also borrowed as immutable + let rma = &mut u.a; //~ ERROR cannot borrow `u.a` as mutable because it is also borrowed as immutable drop(ra); } { let ra = &u.a; - u.a = 1; //[ast]~ ERROR cannot assign to `u.a` because it is borrowed - //[mir]~^ ERROR cannot assign to `u.a` because it is borrowed + u.a = 1; //~ ERROR cannot assign to `u.a` because it is borrowed drop(ra); } // Imm borrow, other field @@ -47,65 +43,55 @@ fn main() { } { let ra = &u.a; - let rmb = &mut u.b; //[ast]~ ERROR cannot borrow `u` (via `u.b`) as mutable because `u` is also borrowed as immutable (via `u.a`) - //[mir]~^ ERROR cannot borrow `u.b` as mutable because it is also borrowed as immutable + let rmb = &mut u.b; //~ ERROR cannot borrow `u` (via `u.b`) as mutable because `u` is also borrowed as immutable (via `u.a`) drop(ra); } { let ra = &u.a; - u.b = 1; //[ast]~ ERROR cannot assign to `u.b` because it is borrowed - //[mir]~^ ERROR cannot assign to `u.b` because it is borrowed + u.b = 1; //~ ERROR cannot assign to `u.b` because it is borrowed drop(ra); } // Mut borrow, same field { let rma = &mut u.a; - let ra = &u.a; //[ast]~ ERROR cannot borrow `u.a` as immutable because it is also borrowed as mutable - //[mir]~^ ERROR cannot borrow `u.a` as immutable because it is also borrowed as mutable + let ra = &u.a; //~ ERROR cannot borrow `u.a` as immutable because it is also borrowed as mutable drop(rma); } { let ra = &mut u.a; - let a = u.a; //[ast]~ ERROR cannot use `u.a` because it was mutably borrowed - //[mir]~^ ERROR cannot use `u.a` because it was mutably borrowed + let a = u.a; //~ ERROR cannot use `u.a` because it was mutably borrowed drop(ra); } { let rma = &mut u.a; - let rma2 = &mut u.a; //[ast]~ ERROR cannot borrow `u.a` as mutable more than once at a time - //[mir]~^ ERROR cannot borrow `u.a` as mutable more than once at a time + let rma2 = &mut u.a; //~ ERROR cannot borrow `u.a` as mutable more than once at a time drop(rma); } { let rma = &mut u.a; - u.a = 1; //[ast]~ ERROR cannot assign to `u.a` because it is borrowed - //[mir]~^ ERROR cannot assign to `u.a` because it is borrowed + u.a = 1; //~ ERROR cannot assign to `u.a` because it is borrowed drop(rma); } // Mut borrow, other field { let rma = &mut u.a; - let rb = &u.b; //[ast]~ ERROR cannot borrow `u` (via `u.b`) as immutable because `u` is also borrowed as mutable (via `u.a`) - //[mir]~^ ERROR cannot borrow `u.b` as immutable because it is also borrowed as mutable + let rb = &u.b; //~ ERROR cannot borrow `u` (via `u.b`) as immutable because `u` is also borrowed as mutable (via `u.a`) drop(rma); } { let ra = &mut u.a; - let b = u.b; //[ast]~ ERROR cannot use `u.b` because it was mutably borrowed - //[mir]~^ ERROR cannot use `u.b` because it was mutably borrowed + let b = u.b; //~ ERROR cannot use `u.b` because it was mutably borrowed drop(ra); } { let rma = &mut u.a; - let rmb2 = &mut u.b; //[ast]~ ERROR cannot borrow `u` (via `u.b`) as mutable more than once at a time - //[mir]~^ ERROR cannot borrow `u.b` as mutable more than once at a time + let rmb2 = &mut u.b; //~ ERROR cannot borrow `u` (via `u.b`) as mutable more than once at a time drop(rma); } { let rma = &mut u.a; - u.b = 1; //[ast]~ ERROR cannot assign to `u.b` because it is borrowed - //[mir]~^ ERROR cannot assign to `u.b` because it is borrowed + u.b = 1; //~ ERROR cannot assign to `u.b` because it is borrowed drop(rma); } } diff --git a/src/test/ui/borrowck/borrowck-union-borrow.ast.stderr b/src/test/ui/borrowck/borrowck-union-borrow.stderr similarity index 64% rename from src/test/ui/borrowck/borrowck-union-borrow.ast.stderr rename to src/test/ui/borrowck/borrowck-union-borrow.stderr index 0f786b81d3a25..f9010d3bf08c4 100644 --- a/src/test/ui/borrowck/borrowck-union-borrow.ast.stderr +++ b/src/test/ui/borrowck/borrowck-union-borrow.stderr @@ -1,115 +1,115 @@ error[E0502]: cannot borrow `u.a` as mutable because it is also borrowed as immutable - --> $DIR/borrowck-union-borrow.rs:27:28 + --> $DIR/borrowck-union-borrow.rs:25:28 | LL | let ra = &u.a; | --- immutable borrow occurs here -LL | let rma = &mut u.a; //[ast]~ ERROR cannot borrow `u.a` as mutable because it is also borrowed as immutable +LL | let rma = &mut u.a; //~ ERROR cannot borrow `u.a` as mutable because it is also borrowed as immutable | ^^^ mutable borrow occurs here -... +LL | drop(ra); LL | } | - immutable borrow ends here error[E0506]: cannot assign to `u.a` because it is borrowed - --> $DIR/borrowck-union-borrow.rs:33:13 + --> $DIR/borrowck-union-borrow.rs:30:13 | LL | let ra = &u.a; | --- borrow of `u.a` occurs here -LL | u.a = 1; //[ast]~ ERROR cannot assign to `u.a` because it is borrowed +LL | u.a = 1; //~ ERROR cannot assign to `u.a` because it is borrowed | ^^^^^^^ assignment to borrowed `u.a` occurs here error[E0502]: cannot borrow `u` (via `u.b`) as mutable because `u` is also borrowed as immutable (via `u.a`) - --> $DIR/borrowck-union-borrow.rs:50:28 + --> $DIR/borrowck-union-borrow.rs:46:28 | LL | let ra = &u.a; | --- immutable borrow occurs here (via `u.a`) -LL | let rmb = &mut u.b; //[ast]~ ERROR cannot borrow `u` (via `u.b`) as mutable because `u` is also borrowed as immutable (via `u.a`) +LL | let rmb = &mut u.b; //~ ERROR cannot borrow `u` (via `u.b`) as mutable because `u` is also borrowed as immutable (via `u.a`) | ^^^ mutable borrow occurs here (via `u.b`) -... +LL | drop(ra); LL | } | - immutable borrow ends here error[E0506]: cannot assign to `u.b` because it is borrowed - --> $DIR/borrowck-union-borrow.rs:56:13 + --> $DIR/borrowck-union-borrow.rs:51:13 | LL | let ra = &u.a; | --- borrow of `u.b` occurs here -LL | u.b = 1; //[ast]~ ERROR cannot assign to `u.b` because it is borrowed +LL | u.b = 1; //~ ERROR cannot assign to `u.b` because it is borrowed | ^^^^^^^ assignment to borrowed `u.b` occurs here error[E0502]: cannot borrow `u.a` as immutable because it is also borrowed as mutable - --> $DIR/borrowck-union-borrow.rs:63:23 + --> $DIR/borrowck-union-borrow.rs:57:23 | LL | let rma = &mut u.a; | --- mutable borrow occurs here -LL | let ra = &u.a; //[ast]~ ERROR cannot borrow `u.a` as immutable because it is also borrowed as mutable +LL | let ra = &u.a; //~ ERROR cannot borrow `u.a` as immutable because it is also borrowed as mutable | ^^^ immutable borrow occurs here -... +LL | drop(rma); LL | } | - mutable borrow ends here error[E0503]: cannot use `u.a` because it was mutably borrowed - --> $DIR/borrowck-union-borrow.rs:69:17 + --> $DIR/borrowck-union-borrow.rs:62:17 | LL | let ra = &mut u.a; | --- borrow of `u.a` occurs here -LL | let a = u.a; //[ast]~ ERROR cannot use `u.a` because it was mutably borrowed +LL | let a = u.a; //~ ERROR cannot use `u.a` because it was mutably borrowed | ^ use of borrowed `u.a` error[E0499]: cannot borrow `u.a` as mutable more than once at a time - --> $DIR/borrowck-union-borrow.rs:75:29 + --> $DIR/borrowck-union-borrow.rs:67:29 | LL | let rma = &mut u.a; | --- first mutable borrow occurs here -LL | let rma2 = &mut u.a; //[ast]~ ERROR cannot borrow `u.a` as mutable more than once at a time +LL | let rma2 = &mut u.a; //~ ERROR cannot borrow `u.a` as mutable more than once at a time | ^^^ second mutable borrow occurs here -... +LL | drop(rma); LL | } | - first borrow ends here error[E0506]: cannot assign to `u.a` because it is borrowed - --> $DIR/borrowck-union-borrow.rs:81:13 + --> $DIR/borrowck-union-borrow.rs:72:13 | LL | let rma = &mut u.a; | --- borrow of `u.a` occurs here -LL | u.a = 1; //[ast]~ ERROR cannot assign to `u.a` because it is borrowed +LL | u.a = 1; //~ ERROR cannot assign to `u.a` because it is borrowed | ^^^^^^^ assignment to borrowed `u.a` occurs here error[E0502]: cannot borrow `u` (via `u.b`) as immutable because `u` is also borrowed as mutable (via `u.a`) - --> $DIR/borrowck-union-borrow.rs:88:23 + --> $DIR/borrowck-union-borrow.rs:78:23 | LL | let rma = &mut u.a; | --- mutable borrow occurs here (via `u.a`) -LL | let rb = &u.b; //[ast]~ ERROR cannot borrow `u` (via `u.b`) as immutable because `u` is also borrowed as mutable (via `u.a`) +LL | let rb = &u.b; //~ ERROR cannot borrow `u` (via `u.b`) as immutable because `u` is also borrowed as mutable (via `u.a`) | ^^^ immutable borrow occurs here (via `u.b`) -... +LL | drop(rma); LL | } | - mutable borrow ends here error[E0503]: cannot use `u.b` because it was mutably borrowed - --> $DIR/borrowck-union-borrow.rs:94:17 + --> $DIR/borrowck-union-borrow.rs:83:17 | LL | let ra = &mut u.a; | --- borrow of `u.a` occurs here -LL | let b = u.b; //[ast]~ ERROR cannot use `u.b` because it was mutably borrowed +LL | let b = u.b; //~ ERROR cannot use `u.b` because it was mutably borrowed | ^ use of borrowed `u.a` error[E0499]: cannot borrow `u` (via `u.b`) as mutable more than once at a time - --> $DIR/borrowck-union-borrow.rs:101:29 + --> $DIR/borrowck-union-borrow.rs:89:29 | LL | let rma = &mut u.a; | --- first mutable borrow occurs here (via `u.a`) -LL | let rmb2 = &mut u.b; //[ast]~ ERROR cannot borrow `u` (via `u.b`) as mutable more than once at a time +LL | let rmb2 = &mut u.b; //~ ERROR cannot borrow `u` (via `u.b`) as mutable more than once at a time | ^^^ second mutable borrow occurs here (via `u.b`) -... +LL | drop(rma); LL | } | - first borrow ends here error[E0506]: cannot assign to `u.b` because it is borrowed - --> $DIR/borrowck-union-borrow.rs:107:13 + --> $DIR/borrowck-union-borrow.rs:94:13 | LL | let rma = &mut u.a; | --- borrow of `u.b` occurs here -LL | u.b = 1; //[ast]~ ERROR cannot assign to `u.b` because it is borrowed +LL | u.b = 1; //~ ERROR cannot assign to `u.b` because it is borrowed | ^^^^^^^ assignment to borrowed `u.b` occurs here error: aborting due to 12 previous errors diff --git a/src/test/ui/issues/issue-45157.rs b/src/test/ui/issues/issue-45157.rs index c2c9ed62d4edc..22ea254a769e8 100644 --- a/src/test/ui/issues/issue-45157.rs +++ b/src/test/ui/issues/issue-45157.rs @@ -1,6 +1,8 @@ #![allow(unused)] #![feature(nll)] +// ignore-tidy-linelength + #[derive(Clone, Copy, Default)] struct S { a: u8, @@ -25,8 +27,7 @@ fn main() { *mref = 22; let nref = &u.z.c; - //~^ ERROR cannot borrow `u.z.c` as immutable because it is also borrowed as mutable [E0502] + //~^ ERROR cannot borrow `u` (via `u.z.c`) as immutable because it is also borrowed as mutable (via `u.s.a`) [E0502] println!("{} {}", mref, nref) } } - diff --git a/src/test/ui/issues/issue-45157.stderr b/src/test/ui/issues/issue-45157.stderr index 038d6ecf48d29..eadbd608699be 100644 --- a/src/test/ui/issues/issue-45157.stderr +++ b/src/test/ui/issues/issue-45157.stderr @@ -1,12 +1,12 @@ -error[E0502]: cannot borrow `u.z.c` as immutable because it is also borrowed as mutable - --> $DIR/issue-45157.rs:27:20 +error[E0502]: cannot borrow `u` (via `u.z.c`) as immutable because it is also borrowed as mutable (via `u.s.a`) + --> $DIR/issue-45157.rs:29:20 | LL | let mref = &mut u.s.a; - | ---------- mutable borrow occurs here + | ---------- mutable borrow occurs here (via `u.s.a`) ... LL | let nref = &u.z.c; - | ^^^^^^ immutable borrow occurs here -LL | //~^ ERROR cannot borrow `u.z.c` as immutable because it is also borrowed as mutable [E0502] + | ^^^^^^ immutable borrow occurs here (via `u.z.c`) +LL | //~^ ERROR cannot borrow `u` (via `u.z.c`) as immutable because it is also borrowed as mutable (via `u.s.a`) [E0502] LL | println!("{} {}", mref, nref) | ---- mutable borrow later used here diff --git a/src/test/ui/union/union-borrow-move-parent-sibling.nll.stderr b/src/test/ui/union/union-borrow-move-parent-sibling.nll.stderr index 953160db14e38..4bf3b6286aa72 100644 --- a/src/test/ui/union/union-borrow-move-parent-sibling.nll.stderr +++ b/src/test/ui/union/union-borrow-move-parent-sibling.nll.stderr @@ -1,10 +1,10 @@ -error[E0502]: cannot borrow `u.y` as immutable because it is also borrowed as mutable +error[E0502]: cannot borrow `u` (via `u.y`) as immutable because it is also borrowed as mutable (via `u.x.0`) --> $DIR/union-borrow-move-parent-sibling.rs:15:13 | LL | let a = &mut u.x.0; - | ---------- mutable borrow occurs here + | ---------- mutable borrow occurs here (via `u.x.0`) LL | let b = &u.y; //~ ERROR cannot borrow `u.y` - | ^^^^ immutable borrow occurs here + | ^^^^ immutable borrow occurs here (via `u.y`) LL | use_borrow(a); | - mutable borrow later used here @@ -18,13 +18,13 @@ LL | let b = u.y; //~ ERROR use of moved value: `u.y` | = note: move occurs because `u` has type `U`, which does not implement the `Copy` trait -error[E0502]: cannot borrow `u.y` as immutable because it is also borrowed as mutable +error[E0502]: cannot borrow `u` (via `u.y`) as immutable because it is also borrowed as mutable (via `u.x.0.0`) --> $DIR/union-borrow-move-parent-sibling.rs:28:13 | LL | let a = &mut (u.x.0).0; - | -------------- mutable borrow occurs here + | -------------- mutable borrow occurs here (via `u.x.0.0`) LL | let b = &u.y; //~ ERROR cannot borrow `u.y` - | ^^^^ immutable borrow occurs here + | ^^^^ immutable borrow occurs here (via `u.y`) LL | use_borrow(a); | - mutable borrow later used here @@ -38,13 +38,13 @@ LL | let b = u.y; //~ ERROR use of moved value: `u.y` | = note: move occurs because `u` has type `U`, which does not implement the `Copy` trait -error[E0502]: cannot borrow `u.x` as immutable because it is also borrowed as mutable +error[E0502]: cannot borrow `u` (via `u.x`) as immutable because it is also borrowed as mutable (via `*u.y`) --> $DIR/union-borrow-move-parent-sibling.rs:41:13 | LL | let a = &mut *u.y; - | --------- mutable borrow occurs here + | --------- mutable borrow occurs here (via `*u.y`) LL | let b = &u.x; //~ ERROR cannot borrow `u` (via `u.x`) - | ^^^^ immutable borrow occurs here + | ^^^^ immutable borrow occurs here (via `u.x`) LL | use_borrow(a); | - mutable borrow later used here From ae1a50d5d84f2ff42131344f69f6ad32c0f7dea3 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Fri, 4 Jan 2019 07:54:33 -0800 Subject: [PATCH 2/8] rustc: Place wasm linker args first instead of last This ensures that arguments passed via `-C link-arg` can override the first ones on the command line, for example allowing configuring of the stack size. --- src/librustc_codegen_ssa/back/linker.rs | 122 ++++++++++++------------ 1 file changed, 62 insertions(+), 60 deletions(-) diff --git a/src/librustc_codegen_ssa/back/linker.rs b/src/librustc_codegen_ssa/back/linker.rs index 270387572f137..06d4f940436da 100644 --- a/src/librustc_codegen_ssa/back/linker.rs +++ b/src/librustc_codegen_ssa/back/linker.rs @@ -81,11 +81,7 @@ impl LinkerInfo { } LinkerFlavor::Lld(LldFlavor::Wasm) => { - Box::new(WasmLd { - cmd, - sess, - info: self - }) as Box + Box::new(WasmLd::new(cmd, sess, self)) as Box } } } @@ -876,6 +872,67 @@ pub struct WasmLd<'a> { info: &'a LinkerInfo, } +impl<'a> WasmLd<'a> { + fn new(mut cmd: Command, sess: &'a Session, info: &'a LinkerInfo) -> WasmLd<'a> { + // There have been reports in the wild (rustwasm/wasm-bindgen#119) of + // using threads causing weird hangs and bugs. Disable it entirely as + // this isn't yet the bottleneck of compilation at all anyway. + cmd.arg("--no-threads"); + + // By default LLD only gives us one page of stack (64k) which is a + // little small. Default to a larger stack closer to other PC platforms + // (1MB) and users can always inject their own link-args to override this. + cmd.arg("-z").arg("stack-size=1048576"); + + // By default LLD's memory layout is: + // + // 1. First, a blank page + // 2. Next, all static data + // 3. Finally, the main stack (which grows down) + // + // This has the unfortunate consequence that on stack overflows you + // corrupt static data and can cause some exceedingly weird bugs. To + // help detect this a little sooner we instead request that the stack is + // placed before static data. + // + // This means that we'll generate slightly larger binaries as references + // to static data will take more bytes in the ULEB128 encoding, but + // stack overflow will be guaranteed to trap as it underflows instead of + // corrupting static data. + cmd.arg("--stack-first"); + + // FIXME we probably shouldn't pass this but instead pass an explicit + // whitelist of symbols we'll allow to be undefined. Unfortunately + // though we can't handle symbols like `log10` that LLVM injects at a + // super late date without actually parsing object files. For now let's + // stick to this and hopefully fix it before stabilization happens. + cmd.arg("--allow-undefined"); + + // For now we just never have an entry symbol + cmd.arg("--no-entry"); + + // Make the default table accessible + cmd.arg("--export-table"); + + // Rust code should never have warnings, and warnings are often + // indicative of bugs, let's prevent them. + cmd.arg("--fatal-warnings"); + + // The symbol visibility story is a bit in flux right now with LLD. + // It's... not entirely clear to me what's going on, but this looks to + // make everything work when `export_symbols` isn't otherwise called for + // things like executables. + cmd.arg("--export-dynamic"); + + // LLD only implements C++-like demangling, which doesn't match our own + // mangling scheme. Tell LLD to not demangle anything and leave it up to + // us to demangle these symbols later. + cmd.arg("--no-demangle"); + + WasmLd { cmd, sess, info } + } +} + impl<'a> Linker for WasmLd<'a> { fn link_dylib(&mut self, lib: &str) { self.cmd.arg("-l").arg(lib); @@ -982,61 +1039,6 @@ impl<'a> Linker for WasmLd<'a> { } fn finalize(&mut self) -> Command { - // There have been reports in the wild (rustwasm/wasm-bindgen#119) of - // using threads causing weird hangs and bugs. Disable it entirely as - // this isn't yet the bottleneck of compilation at all anyway. - self.cmd.arg("--no-threads"); - - // By default LLD only gives us one page of stack (64k) which is a - // little small. Default to a larger stack closer to other PC platforms - // (1MB) and users can always inject their own link-args to override this. - self.cmd.arg("-z").arg("stack-size=1048576"); - - // By default LLD's memory layout is: - // - // 1. First, a blank page - // 2. Next, all static data - // 3. Finally, the main stack (which grows down) - // - // This has the unfortunate consequence that on stack overflows you - // corrupt static data and can cause some exceedingly weird bugs. To - // help detect this a little sooner we instead request that the stack is - // placed before static data. - // - // This means that we'll generate slightly larger binaries as references - // to static data will take more bytes in the ULEB128 encoding, but - // stack overflow will be guaranteed to trap as it underflows instead of - // corrupting static data. - self.cmd.arg("--stack-first"); - - // FIXME we probably shouldn't pass this but instead pass an explicit - // whitelist of symbols we'll allow to be undefined. Unfortunately - // though we can't handle symbols like `log10` that LLVM injects at a - // super late date without actually parsing object files. For now let's - // stick to this and hopefully fix it before stabilization happens. - self.cmd.arg("--allow-undefined"); - - // For now we just never have an entry symbol - self.cmd.arg("--no-entry"); - - // Make the default table accessible - self.cmd.arg("--export-table"); - - // Rust code should never have warnings, and warnings are often - // indicative of bugs, let's prevent them. - self.cmd.arg("--fatal-warnings"); - - // The symbol visibility story is a bit in flux right now with LLD. - // It's... not entirely clear to me what's going on, but this looks to - // make everything work when `export_symbols` isn't otherwise called for - // things like executables. - self.cmd.arg("--export-dynamic"); - - // LLD only implements C++-like demangling, which doesn't match our own - // mangling scheme. Tell LLD to not demangle anything and leave it up to - // us to demangle these symbols later. - self.cmd.arg("--no-demangle"); - ::std::mem::replace(&mut self.cmd, Command::new("")) } From 388dffe347f86f2c95ffc1fa2f5fa7898b7f8d66 Mon Sep 17 00:00:00 2001 From: David Wood Date: Fri, 4 Jan 2019 20:56:41 +0100 Subject: [PATCH 3/8] Make conflicting borrow description more robust. This commit improves the logic for place descriptions in conflicting borrow errors so that borrows of union fields have better messages even when the unions are embedded in other unions or structs. --- .../borrow_check/error_reporting.rs | 122 +++++++++++++----- src/test/ui/nll/issue-57100.rs | 69 ++++++++++ src/test/ui/nll/issue-57100.stderr | 27 ++++ 3 files changed, 187 insertions(+), 31 deletions(-) create mode 100644 src/test/ui/nll/issue-57100.rs create mode 100644 src/test/ui/nll/issue-57100.stderr diff --git a/src/librustc_mir/borrow_check/error_reporting.rs b/src/librustc_mir/borrow_check/error_reporting.rs index 6f12ff994c4b2..aeba56cb7e260 100644 --- a/src/librustc_mir/borrow_check/error_reporting.rs +++ b/src/librustc_mir/borrow_check/error_reporting.rs @@ -329,16 +329,12 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { "closure" }; - let (desc_place, msg_place, msg_borrow) = if issued_borrow.borrowed_place == *place { - let desc_place = self.describe_place(place).unwrap_or_else(|| "_".to_owned()); - (desc_place, "".to_string(), "".to_string()) - } else { - let (desc_place, msg_place) = self.describe_place_for_conflicting_borrow(place); - let (_, msg_borrow) = self.describe_place_for_conflicting_borrow( - &issued_borrow.borrowed_place - ); - (desc_place, msg_place, msg_borrow) - }; + let (desc_place, msg_place, msg_borrow) = self.describe_place_for_conflicting_borrow( + place, &issued_borrow.borrowed_place, + ); + let via = |msg: String| if msg.is_empty() { msg } else { format!(" (via `{}`)", msg) }; + let msg_place = via(msg_place); + let msg_borrow = via(msg_borrow); let explanation = self.explain_why_borrow_contains_point(context, issued_borrow, None); let second_borrow_desc = if explanation.is_explained() { @@ -526,33 +522,97 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { err.buffer(&mut self.errors_buffer); } - /// Returns a description of a place and an associated message for the purposes of conflicting - /// borrow diagnostics. + /// Returns the description of the root place for a conflicting borrow and the full + /// descriptions of the places that caused the conflict. + /// + /// In the simplest case, where there are no unions involved, if a mutable borrow of `x` is + /// attempted while a shared borrow is live, then this function will return: + /// + /// ("x", "", "") /// - /// If the borrow is of the field `b` of a union `u`, then the return value will be - /// `("u", " (via \`u.b\`)")`. Otherwise, for some variable `a`, the return value will be - /// `("a", "")`. + /// In the simple union case, if a mutable borrow of a union field `x.z` is attempted while + /// a shared borrow of another field `x.y`, then this function will return: + /// + /// ("x", "x.z", "x.y") + /// + /// In the more complex union case, where the union is a field of a struct, then if a mutable + /// borrow of a union field in a struct `x.u.z` is attempted while a shared borrow of + /// another field `x.u.y`, then this function will return: + /// + /// ("x.u", "x.u.z", "x.u.y") + /// + /// This is used when creating error messages like below: + /// + /// > cannot borrow `a.u` (via `a.u.z.c`) as immutable because it is also borrowed as + /// > mutable (via `a.u.s.b`) [E0502] pub(super) fn describe_place_for_conflicting_borrow( &self, - place: &Place<'tcx>, - ) -> (String, String) { - place.base_local() - .filter(|local| { - // Filter out non-unions. - self.mir.local_decls[*local].ty - .ty_adt_def() - .map(|adt| adt.is_union()) - .unwrap_or(false) + first_borrowed_place: &Place<'tcx>, + second_borrowed_place: &Place<'tcx>, + ) -> (String, String, String) { + // Define a small closure that we can use to check if the type of a place + // is a union. + let is_union = |place: &Place<'tcx>| -> bool { + place.ty(self.mir, self.infcx.tcx) + .to_ty(self.infcx.tcx) + .ty_adt_def() + .map(|adt| adt.is_union()) + .unwrap_or(false) + }; + + // Start with an empty tuple, so we can use the functions on `Option` to reduce some + // code duplication (particularly around returning an empty description in the failure + // case). + Some(()) + .filter(|_| { + // If we have a conflicting borrow of the same place, then we don't want to add + // an extraneous "via x.y" to our diagnostics, so filter out this case. + first_borrowed_place != second_borrowed_place }) - .and_then(|local| { - let desc_base = self.describe_place(&Place::Local(local)) - .unwrap_or_else(|| "_".to_owned()); - let desc_original = self.describe_place(place) - .unwrap_or_else(|| "_".to_owned()); - return Some((desc_base, format!(" (via `{}`)", desc_original))); + .and_then(|_| { + // We're going to want to traverse the first borrowed place to see if we can find + // field access to a union. If we find that, then we will keep the place of the + // union being accessed and the field that was being accessed so we can check the + // second borrowed place for the same union and a access to a different field. + let mut current = first_borrowed_place; + while let Place::Projection(box PlaceProjection { base, elem }) = current { + match elem { + ProjectionElem::Field(field, _) if is_union(base) => { + return Some((base, field)); + }, + _ => current = base, + } + } + None + }) + .and_then(|(target_base, target_field)| { + // With the place of a union and a field access into it, we traverse the second + // borrowed place and look for a access to a different field of the same union. + let mut current = second_borrowed_place; + while let Place::Projection(box PlaceProjection { base, elem }) = current { + match elem { + ProjectionElem::Field(field, _) if { + is_union(base) && field != target_field && base == target_base + } => { + let desc_base = self.describe_place(base) + .unwrap_or_else(|| "_".to_owned()); + let desc_first = self.describe_place(first_borrowed_place) + .unwrap_or_else(|| "_".to_owned()); + let desc_second = self.describe_place(second_borrowed_place) + .unwrap_or_else(|| "_".to_owned()); + return Some((desc_base, desc_first, desc_second)); + }, + _ => current = base, + } + } + None }) .unwrap_or_else(|| { - (self.describe_place(place).unwrap_or_else(|| "_".to_owned()), "".to_string()) + // If we didn't find a field access into a union, or both places match, then + // only return the description of the first place. + let desc_place = self.describe_place(first_borrowed_place) + .unwrap_or_else(|| "_".to_owned()); + (desc_place, "".to_string(), "".to_string()) }) } diff --git a/src/test/ui/nll/issue-57100.rs b/src/test/ui/nll/issue-57100.rs new file mode 100644 index 0000000000000..f669fe00956ef --- /dev/null +++ b/src/test/ui/nll/issue-57100.rs @@ -0,0 +1,69 @@ +#![allow(unused)] +#![feature(nll)] + +// ignore-tidy-linelength + +// This tests the error messages for borrows of union fields when the unions are embedded in other +// structs or unions. + +#[derive(Clone, Copy, Default)] +struct Leaf { + l1_u8: u8, + l2_u8: u8, +} + +#[derive(Clone, Copy)] +union First { + f1_leaf: Leaf, + f2_leaf: Leaf, + f3_union: Second, +} + +#[derive(Clone, Copy)] +union Second { + s1_leaf: Leaf, + s2_leaf: Leaf, +} + +struct Root { + r1_u8: u8, + r2_union: First, +} + +// Borrow a different field of the nested union. +fn nested_union() { + unsafe { + let mut r = Root { + r1_u8: 3, + r2_union: First { f3_union: Second { s2_leaf: Leaf { l1_u8: 8, l2_u8: 4 } } } + }; + + let mref = &mut r.r2_union.f3_union.s1_leaf.l1_u8; + // ^^^^^^^ + *mref = 22; + let nref = &r.r2_union.f3_union.s2_leaf.l1_u8; + // ^^^^^^^ + //~^^ ERROR cannot borrow `r.r2_union.f3_union` (via `r.r2_union.f3_union.s2_leaf.l1_u8`) as immutable because it is also borrowed as mutable (via `r.r2_union.f3_union.s1_leaf.l1_u8`) [E0502] + println!("{} {}", mref, nref) + } +} + +// Borrow a different field of the first union. +fn first_union() { + unsafe { + let mut r = Root { + r1_u8: 3, + r2_union: First { f3_union: Second { s2_leaf: Leaf { l1_u8: 8, l2_u8: 4 } } } + }; + + let mref = &mut r.r2_union.f2_leaf.l1_u8; + // ^^^^^^^ + *mref = 22; + let nref = &r.r2_union.f1_leaf.l1_u8; + // ^^^^^^^ + //~^^ ERROR cannot borrow `r.r2_union` (via `r.r2_union.f1_leaf.l1_u8`) as immutable because it is also borrowed as mutable (via `r.r2_union.f2_leaf.l1_u8`) [E0502] + println!("{} {}", mref, nref) + } +} + +fn main() {} diff --git a/src/test/ui/nll/issue-57100.stderr b/src/test/ui/nll/issue-57100.stderr new file mode 100644 index 0000000000000..34dcdfc49478b --- /dev/null +++ b/src/test/ui/nll/issue-57100.stderr @@ -0,0 +1,27 @@ +error[E0502]: cannot borrow `r.r2_union.f3_union` (via `r.r2_union.f3_union.s2_leaf.l1_u8`) as immutable because it is also borrowed as mutable (via `r.r2_union.f3_union.s1_leaf.l1_u8`) + --> $DIR/issue-57100.rs:44:20 + | +LL | let mref = &mut r.r2_union.f3_union.s1_leaf.l1_u8; + | -------------------------------------- mutable borrow occurs here (via `r.r2_union.f3_union.s1_leaf.l1_u8`) +... +LL | let nref = &r.r2_union.f3_union.s2_leaf.l1_u8; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ immutable borrow occurs here (via `r.r2_union.f3_union.s2_leaf.l1_u8`) +... +LL | println!("{} {}", mref, nref) + | ---- mutable borrow later used here + +error[E0502]: cannot borrow `r.r2_union` (via `r.r2_union.f1_leaf.l1_u8`) as immutable because it is also borrowed as mutable (via `r.r2_union.f2_leaf.l1_u8`) + --> $DIR/issue-57100.rs:62:20 + | +LL | let mref = &mut r.r2_union.f2_leaf.l1_u8; + | ----------------------------- mutable borrow occurs here (via `r.r2_union.f2_leaf.l1_u8`) +... +LL | let nref = &r.r2_union.f1_leaf.l1_u8; + | ^^^^^^^^^^^^^^^^^^^^^^^^^ immutable borrow occurs here (via `r.r2_union.f1_leaf.l1_u8`) +... +LL | println!("{} {}", mref, nref) + | ---- mutable borrow later used here + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0502`. From c2b477c19af9c5b8241e3d37a9d8a9cf1616750f Mon Sep 17 00:00:00 2001 From: David Wood Date: Fri, 4 Jan 2019 22:43:51 +0100 Subject: [PATCH 4/8] Improve diagnostic labels and add note. This commit improves diagnostic labels to mention which field a borrow overlaps with and adds a note explaining that the fields overlap. --- src/librustc_borrowck/borrowck/check_loans.rs | 8 +--- .../borrow_check/error_reporting.rs | 26 +++++++---- src/librustc_mir/util/borrowck_errors.rs | 43 ++++++++++++++----- .../borrowck-borrow-from-owned-ptr.stderr | 2 +- .../borrowck-box-insensitivity.ast.stderr | 4 +- .../ui/borrowck/borrowck-box-insensitivity.rs | 4 +- .../borrowck/borrowck-union-borrow.nll.stderr | 10 ++++- .../ui/borrowck/borrowck-union-borrow.stderr | 4 +- src/test/ui/issues/issue-17263.ast.stderr | 2 +- src/test/ui/issues/issue-45157.stderr | 4 +- src/test/ui/nll/issue-57100.stderr | 8 +++- ...nion-borrow-move-parent-sibling.nll.stderr | 12 ++++-- .../union-borrow-move-parent-sibling.stderr | 2 +- 13 files changed, 86 insertions(+), 43 deletions(-) diff --git a/src/librustc_borrowck/borrowck/check_loans.rs b/src/librustc_borrowck/borrowck/check_loans.rs index 04085387c37f0..cafb29ed99a41 100644 --- a/src/librustc_borrowck/borrowck/check_loans.rs +++ b/src/librustc_borrowck/borrowck/check_loans.rs @@ -557,12 +557,8 @@ impl<'a, 'tcx> CheckLoanCtxt<'a, 'tcx> { if new_loan.loan_path.has_fork(&old_loan.loan_path) && common.is_some() { let nl = self.bccx.loan_path_to_string(&common.unwrap()); let ol = nl.clone(); - let new_loan_msg = format!(" (via `{}`)", - self.bccx.loan_path_to_string( - &new_loan.loan_path)); - let old_loan_msg = format!(" (via `{}`)", - self.bccx.loan_path_to_string( - &old_loan.loan_path)); + let new_loan_msg = self.bccx.loan_path_to_string(&new_loan.loan_path); + let old_loan_msg = self.bccx.loan_path_to_string(&old_loan.loan_path); (nl, ol, new_loan_msg, old_loan_msg) } else { (self.bccx.loan_path_to_string(&new_loan.loan_path), diff --git a/src/librustc_mir/borrow_check/error_reporting.rs b/src/librustc_mir/borrow_check/error_reporting.rs index aeba56cb7e260..f6a80d7fae37c 100644 --- a/src/librustc_mir/borrow_check/error_reporting.rs +++ b/src/librustc_mir/borrow_check/error_reporting.rs @@ -329,12 +329,8 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { "closure" }; - let (desc_place, msg_place, msg_borrow) = self.describe_place_for_conflicting_borrow( - place, &issued_borrow.borrowed_place, - ); - let via = |msg: String| if msg.is_empty() { msg } else { format!(" (via `{}`)", msg) }; - let msg_place = via(msg_place); - let msg_borrow = via(msg_borrow); + let (desc_place, msg_place, msg_borrow, union_type_name) = + self.describe_place_for_conflicting_borrow(place, &issued_borrow.borrowed_place); let explanation = self.explain_why_borrow_contains_point(context, issued_borrow, None); let second_borrow_desc = if explanation.is_explained() { @@ -516,6 +512,13 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { ); } + if union_type_name != "" { + err.note(&format!( + "`{}` is a field of the union `{}`, so it overlaps the field `{}`", + msg_place, union_type_name, msg_borrow, + )); + } + explanation .add_explanation_to_diagnostic(self.infcx.tcx, self.mir, &mut err, first_borrow_desc); @@ -549,7 +552,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { &self, first_borrowed_place: &Place<'tcx>, second_borrowed_place: &Place<'tcx>, - ) -> (String, String, String) { + ) -> (String, String, String, String) { // Define a small closure that we can use to check if the type of a place // is a union. let is_union = |place: &Place<'tcx>| -> bool { @@ -600,7 +603,12 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { .unwrap_or_else(|| "_".to_owned()); let desc_second = self.describe_place(second_borrowed_place) .unwrap_or_else(|| "_".to_owned()); - return Some((desc_base, desc_first, desc_second)); + + // Also compute the name of the union type, eg. `Foo` so we + // can add a helpful note with it. + let ty = base.ty(self.mir, self.infcx.tcx).to_ty(self.infcx.tcx); + + return Some((desc_base, desc_first, desc_second, ty.to_string())); }, _ => current = base, } @@ -612,7 +620,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { // only return the description of the first place. let desc_place = self.describe_place(first_borrowed_place) .unwrap_or_else(|| "_".to_owned()); - (desc_place, "".to_string(), "".to_string()) + (desc_place, "".to_string(), "".to_string(), "".to_string()) }) } diff --git a/src/librustc_mir/util/borrowck_errors.rs b/src/librustc_mir/util/borrowck_errors.rs index 5e55bb4a9b6cf..7ad73aaa3f9a9 100644 --- a/src/librustc_mir/util/borrowck_errors.rs +++ b/src/librustc_mir/util/borrowck_errors.rs @@ -138,13 +138,15 @@ pub trait BorrowckErrors<'cx>: Sized + Copy { old_load_end_span: Option, o: Origin, ) -> DiagnosticBuilder<'cx> { + let via = |msg: &str| + if msg.is_empty() { msg.to_string() } else { format!(" (via `{}`)", msg) }; let mut err = struct_span_err!( self, new_loan_span, E0499, "cannot borrow `{}`{} as mutable more than once at a time{OGN}", desc, - opt_via, + via(opt_via), OGN = o ); if old_loan_span == new_loan_span { @@ -164,11 +166,11 @@ pub trait BorrowckErrors<'cx>: Sized + Copy { } else { err.span_label( old_loan_span, - format!("first mutable borrow occurs here{}", old_opt_via), + format!("first mutable borrow occurs here{}", via(old_opt_via)), ); err.span_label( new_loan_span, - format!("second mutable borrow occurs here{}", opt_via), + format!("second mutable borrow occurs here{}", via(opt_via)), ); if let Some(old_load_end_span) = old_load_end_span { err.span_label(old_load_end_span, "first borrow ends here"); @@ -292,27 +294,46 @@ pub trait BorrowckErrors<'cx>: Sized + Copy { old_load_end_span: Option, o: Origin, ) -> DiagnosticBuilder<'cx> { + let via = |msg: &str| + if msg.is_empty() { msg.to_string() } else { format!(" (via `{}`)", msg) }; let mut err = struct_span_err!( self, span, E0502, - "cannot borrow `{}`{} as {} because {} is also borrowed as {}{}{OGN}", + "cannot borrow `{}`{} as {} because {} is also borrowed \ + as {}{}{OGN}", desc_new, - msg_new, + via(msg_new), kind_new, noun_old, kind_old, - msg_old, + via(msg_old), OGN = o ); - err.span_label(span, format!("{} borrow occurs here{}", kind_new, msg_new)); - err.span_label( - old_span, - format!("{} borrow occurs here{}", kind_old, msg_old), - ); + + if msg_new == "" { + // If `msg_new` is empty, then this isn't a borrow of a union field. + err.span_label(span, format!("{} borrow occurs here", kind_new)); + err.span_label(old_span, format!("{} borrow occurs here", kind_old)); + } else { + // If `msg_new` isn't empty, then this a borrow of a union field. + err.span_label( + span, + format!( + "{} borrow of `{}` -- which overlaps with `{}` -- occurs here", + kind_new, msg_new, msg_old, + ) + ); + err.span_label( + old_span, + format!("{} borrow occurs here{}", kind_old, via(msg_old)), + ); + } + if let Some(old_load_end_span) = old_load_end_span { err.span_label(old_load_end_span, format!("{} borrow ends here", kind_old)); } + self.cancel_if_wrong_origin(err, o) } diff --git a/src/test/ui/borrowck/borrowck-borrow-from-owned-ptr.stderr b/src/test/ui/borrowck/borrowck-borrow-from-owned-ptr.stderr index 79dc55bbbaeda..e116cb70c1c31 100644 --- a/src/test/ui/borrowck/borrowck-borrow-from-owned-ptr.stderr +++ b/src/test/ui/borrowck/borrowck-borrow-from-owned-ptr.stderr @@ -138,7 +138,7 @@ error[E0502]: cannot borrow `foo` (via `foo.bar2`) as immutable because `foo` is LL | let bar1 = &mut foo.bar1; | -------- mutable borrow occurs here (via `foo.bar1`) LL | let _foo1 = &foo.bar2; //~ ERROR cannot borrow - | ^^^^^^^^ immutable borrow occurs here (via `foo.bar2`) + | ^^^^^^^^ immutable borrow of `foo.bar2` -- which overlaps with `foo.bar1` -- occurs here LL | *bar1; LL | } | - mutable borrow ends here diff --git a/src/test/ui/borrowck/borrowck-box-insensitivity.ast.stderr b/src/test/ui/borrowck/borrowck-box-insensitivity.ast.stderr index 95b26a5724a41..236064da3e8b6 100644 --- a/src/test/ui/borrowck/borrowck-box-insensitivity.ast.stderr +++ b/src/test/ui/borrowck/borrowck-box-insensitivity.ast.stderr @@ -61,7 +61,7 @@ error[E0502]: cannot borrow `a` (via `a.y`) as immutable because `a` is also bor LL | let _x = &mut a.x; | --- mutable borrow occurs here (via `a.x`) LL | let _y = &a.y; //[ast]~ ERROR cannot borrow - | ^^^ immutable borrow occurs here (via `a.y`) + | ^^^ immutable borrow of `a.y` -- which overlaps with `a.x` -- occurs here ... LL | } | - mutable borrow ends here @@ -72,7 +72,7 @@ error[E0502]: cannot borrow `a` (via `a.y`) as mutable because `a` is also borro LL | let _x = &a.x; | --- immutable borrow occurs here (via `a.x`) LL | let _y = &mut a.y; //[ast]~ ERROR cannot borrow - | ^^^ mutable borrow occurs here (via `a.y`) + | ^^^ mutable borrow of `a.y` -- which overlaps with `a.x` -- occurs here ... LL | } | - immutable borrow ends here diff --git a/src/test/ui/borrowck/borrowck-box-insensitivity.rs b/src/test/ui/borrowck/borrowck-box-insensitivity.rs index 2af97a9fc1d58..e72048d0ea4bc 100644 --- a/src/test/ui/borrowck/borrowck-box-insensitivity.rs +++ b/src/test/ui/borrowck/borrowck-box-insensitivity.rs @@ -83,14 +83,14 @@ fn borrow_after_mut_borrow() { let mut a: Box<_> = box A { x: box 0, y: 1 }; let _x = &mut a.x; let _y = &a.y; //[ast]~ ERROR cannot borrow - //[ast]~^ immutable borrow occurs here (via `a.y`) + //[ast]~^ immutable borrow of `a.y` -- which overlaps with `a.x` -- occurs here use_mut(_x); } fn mut_borrow_after_borrow() { let mut a: Box<_> = box A { x: box 0, y: 1 }; let _x = &a.x; let _y = &mut a.y; //[ast]~ ERROR cannot borrow - //[ast]~^ mutable borrow occurs here (via `a.y`) + //[ast]~^ mutable borrow of `a.y` -- which overlaps with `a.x` -- occurs here use_imm(_x); } fn copy_after_move_nested() { diff --git a/src/test/ui/borrowck/borrowck-union-borrow.nll.stderr b/src/test/ui/borrowck/borrowck-union-borrow.nll.stderr index ef5dcef04b074..5cba30b43b8a0 100644 --- a/src/test/ui/borrowck/borrowck-union-borrow.nll.stderr +++ b/src/test/ui/borrowck/borrowck-union-borrow.nll.stderr @@ -24,9 +24,11 @@ error[E0502]: cannot borrow `u` (via `u.b`) as mutable because it is also borrow LL | let ra = &u.a; | ---- immutable borrow occurs here (via `u.a`) LL | let rmb = &mut u.b; //~ ERROR cannot borrow `u` (via `u.b`) as mutable because `u` is also borrowed as immutable (via `u.a`) - | ^^^^^^^^ mutable borrow occurs here (via `u.b`) + | ^^^^^^^^ mutable borrow of `u.b` -- which overlaps with `u.a` -- occurs here LL | drop(ra); | -- immutable borrow later used here + | + = note: `u.b` is a field of the union `U`, so it overlaps the field `u.a` error[E0506]: cannot assign to `u.b` because it is borrowed --> $DIR/borrowck-union-borrow.rs:51:13 @@ -84,9 +86,11 @@ error[E0502]: cannot borrow `u` (via `u.b`) as immutable because it is also borr LL | let rma = &mut u.a; | -------- mutable borrow occurs here (via `u.a`) LL | let rb = &u.b; //~ ERROR cannot borrow `u` (via `u.b`) as immutable because `u` is also borrowed as mutable (via `u.a`) - | ^^^^ immutable borrow occurs here (via `u.b`) + | ^^^^ immutable borrow of `u.b` -- which overlaps with `u.a` -- occurs here LL | drop(rma); | --- mutable borrow later used here + | + = note: `u.b` is a field of the union `U`, so it overlaps the field `u.a` error[E0503]: cannot use `u.b` because it was mutably borrowed --> $DIR/borrowck-union-borrow.rs:83:21 @@ -108,6 +112,8 @@ LL | let rmb2 = &mut u.b; //~ ERROR cannot borrow `u` (via `u.b`) as | ^^^^^^^^ second mutable borrow occurs here (via `u.b`) LL | drop(rma); | --- first borrow later used here + | + = note: `u.b` is a field of the union `U`, so it overlaps the field `u.a` error[E0506]: cannot assign to `u.b` because it is borrowed --> $DIR/borrowck-union-borrow.rs:94:13 diff --git a/src/test/ui/borrowck/borrowck-union-borrow.stderr b/src/test/ui/borrowck/borrowck-union-borrow.stderr index f9010d3bf08c4..ef6a331eda04c 100644 --- a/src/test/ui/borrowck/borrowck-union-borrow.stderr +++ b/src/test/ui/borrowck/borrowck-union-borrow.stderr @@ -23,7 +23,7 @@ error[E0502]: cannot borrow `u` (via `u.b`) as mutable because `u` is also borro LL | let ra = &u.a; | --- immutable borrow occurs here (via `u.a`) LL | let rmb = &mut u.b; //~ ERROR cannot borrow `u` (via `u.b`) as mutable because `u` is also borrowed as immutable (via `u.a`) - | ^^^ mutable borrow occurs here (via `u.b`) + | ^^^ mutable borrow of `u.b` -- which overlaps with `u.a` -- occurs here LL | drop(ra); LL | } | - immutable borrow ends here @@ -80,7 +80,7 @@ error[E0502]: cannot borrow `u` (via `u.b`) as immutable because `u` is also bor LL | let rma = &mut u.a; | --- mutable borrow occurs here (via `u.a`) LL | let rb = &u.b; //~ ERROR cannot borrow `u` (via `u.b`) as immutable because `u` is also borrowed as mutable (via `u.a`) - | ^^^ immutable borrow occurs here (via `u.b`) + | ^^^ immutable borrow of `u.b` -- which overlaps with `u.a` -- occurs here LL | drop(rma); LL | } | - mutable borrow ends here diff --git a/src/test/ui/issues/issue-17263.ast.stderr b/src/test/ui/issues/issue-17263.ast.stderr index 3d42dcb52f5db..823f2c747d686 100644 --- a/src/test/ui/issues/issue-17263.ast.stderr +++ b/src/test/ui/issues/issue-17263.ast.stderr @@ -13,7 +13,7 @@ error[E0502]: cannot borrow `foo` (via `foo.b`) as immutable because `foo` is al --> $DIR/issue-17263.rs:21:32 | LL | let (c, d) = (&mut foo.a, &foo.b); - | ----- ^^^^^ immutable borrow occurs here (via `foo.b`) + | ----- ^^^^^ immutable borrow of `foo.b` -- which overlaps with `foo.a` -- occurs here | | | mutable borrow occurs here (via `foo.a`) ... diff --git a/src/test/ui/issues/issue-45157.stderr b/src/test/ui/issues/issue-45157.stderr index eadbd608699be..3b15a8dbd9ef8 100644 --- a/src/test/ui/issues/issue-45157.stderr +++ b/src/test/ui/issues/issue-45157.stderr @@ -5,10 +5,12 @@ LL | let mref = &mut u.s.a; | ---------- mutable borrow occurs here (via `u.s.a`) ... LL | let nref = &u.z.c; - | ^^^^^^ immutable borrow occurs here (via `u.z.c`) + | ^^^^^^ immutable borrow of `u.z.c` -- which overlaps with `u.s.a` -- occurs here LL | //~^ ERROR cannot borrow `u` (via `u.z.c`) as immutable because it is also borrowed as mutable (via `u.s.a`) [E0502] LL | println!("{} {}", mref, nref) | ---- mutable borrow later used here + | + = note: `u.z.c` is a field of the union `U`, so it overlaps the field `u.s.a` error: aborting due to previous error diff --git a/src/test/ui/nll/issue-57100.stderr b/src/test/ui/nll/issue-57100.stderr index 34dcdfc49478b..5d5c86c34875c 100644 --- a/src/test/ui/nll/issue-57100.stderr +++ b/src/test/ui/nll/issue-57100.stderr @@ -5,10 +5,12 @@ LL | let mref = &mut r.r2_union.f3_union.s1_leaf.l1_u8; | -------------------------------------- mutable borrow occurs here (via `r.r2_union.f3_union.s1_leaf.l1_u8`) ... LL | let nref = &r.r2_union.f3_union.s2_leaf.l1_u8; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ immutable borrow occurs here (via `r.r2_union.f3_union.s2_leaf.l1_u8`) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ immutable borrow of `r.r2_union.f3_union.s2_leaf.l1_u8` -- which overlaps with `r.r2_union.f3_union.s1_leaf.l1_u8` -- occurs here ... LL | println!("{} {}", mref, nref) | ---- mutable borrow later used here + | + = note: `r.r2_union.f3_union.s2_leaf.l1_u8` is a field of the union `Second`, so it overlaps the field `r.r2_union.f3_union.s1_leaf.l1_u8` error[E0502]: cannot borrow `r.r2_union` (via `r.r2_union.f1_leaf.l1_u8`) as immutable because it is also borrowed as mutable (via `r.r2_union.f2_leaf.l1_u8`) --> $DIR/issue-57100.rs:62:20 @@ -17,10 +19,12 @@ LL | let mref = &mut r.r2_union.f2_leaf.l1_u8; | ----------------------------- mutable borrow occurs here (via `r.r2_union.f2_leaf.l1_u8`) ... LL | let nref = &r.r2_union.f1_leaf.l1_u8; - | ^^^^^^^^^^^^^^^^^^^^^^^^^ immutable borrow occurs here (via `r.r2_union.f1_leaf.l1_u8`) + | ^^^^^^^^^^^^^^^^^^^^^^^^^ immutable borrow of `r.r2_union.f1_leaf.l1_u8` -- which overlaps with `r.r2_union.f2_leaf.l1_u8` -- occurs here ... LL | println!("{} {}", mref, nref) | ---- mutable borrow later used here + | + = note: `r.r2_union.f1_leaf.l1_u8` is a field of the union `First`, so it overlaps the field `r.r2_union.f2_leaf.l1_u8` error: aborting due to 2 previous errors diff --git a/src/test/ui/union/union-borrow-move-parent-sibling.nll.stderr b/src/test/ui/union/union-borrow-move-parent-sibling.nll.stderr index 4bf3b6286aa72..848c3d9bdb017 100644 --- a/src/test/ui/union/union-borrow-move-parent-sibling.nll.stderr +++ b/src/test/ui/union/union-borrow-move-parent-sibling.nll.stderr @@ -4,9 +4,11 @@ error[E0502]: cannot borrow `u` (via `u.y`) as immutable because it is also borr LL | let a = &mut u.x.0; | ---------- mutable borrow occurs here (via `u.x.0`) LL | let b = &u.y; //~ ERROR cannot borrow `u.y` - | ^^^^ immutable borrow occurs here (via `u.y`) + | ^^^^ immutable borrow of `u.y` -- which overlaps with `u.x.0` -- occurs here LL | use_borrow(a); | - mutable borrow later used here + | + = note: `u.y` is a field of the union `U`, so it overlaps the field `u.x.0` error[E0382]: use of moved value: `u` --> $DIR/union-borrow-move-parent-sibling.rs:22:13 @@ -24,9 +26,11 @@ error[E0502]: cannot borrow `u` (via `u.y`) as immutable because it is also borr LL | let a = &mut (u.x.0).0; | -------------- mutable borrow occurs here (via `u.x.0.0`) LL | let b = &u.y; //~ ERROR cannot borrow `u.y` - | ^^^^ immutable borrow occurs here (via `u.y`) + | ^^^^ immutable borrow of `u.y` -- which overlaps with `u.x.0.0` -- occurs here LL | use_borrow(a); | - mutable borrow later used here + | + = note: `u.y` is a field of the union `U`, so it overlaps the field `u.x.0.0` error[E0382]: use of moved value: `u` --> $DIR/union-borrow-move-parent-sibling.rs:35:13 @@ -44,9 +48,11 @@ error[E0502]: cannot borrow `u` (via `u.x`) as immutable because it is also borr LL | let a = &mut *u.y; | --------- mutable borrow occurs here (via `*u.y`) LL | let b = &u.x; //~ ERROR cannot borrow `u` (via `u.x`) - | ^^^^ immutable borrow occurs here (via `u.x`) + | ^^^^ immutable borrow of `u.x` -- which overlaps with `*u.y` -- occurs here LL | use_borrow(a); | - mutable borrow later used here + | + = note: `u.x` is a field of the union `U`, so it overlaps the field `*u.y` error[E0382]: use of moved value: `u` --> $DIR/union-borrow-move-parent-sibling.rs:48:13 diff --git a/src/test/ui/union/union-borrow-move-parent-sibling.stderr b/src/test/ui/union/union-borrow-move-parent-sibling.stderr index c5baf82c74584..9058707e50516 100644 --- a/src/test/ui/union/union-borrow-move-parent-sibling.stderr +++ b/src/test/ui/union/union-borrow-move-parent-sibling.stderr @@ -46,7 +46,7 @@ error[E0502]: cannot borrow `u` (via `u.x`) as immutable because `u` is also bor LL | let a = &mut *u.y; | ---- mutable borrow occurs here (via `*u.y`) LL | let b = &u.x; //~ ERROR cannot borrow `u` (via `u.x`) - | ^^^ immutable borrow occurs here (via `u.x`) + | ^^^ immutable borrow of `u.x` -- which overlaps with `*u.y` -- occurs here LL | use_borrow(a); LL | } | - mutable borrow ends here From e80a93040ffbbb7eb8013f1dcd3b594ce8a631cd Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 19 Dec 2018 14:53:52 +1100 Subject: [PATCH 5/8] Make `TokenStream` less recursive. `TokenStream` is currently recursive in *two* ways: - the `TokenTree` variant contains a `ThinTokenStream`, which can contain a `TokenStream`; - the `TokenStream` variant contains a `Vec`. The latter is not necessary and causes significant complexity. This commit replaces it with the simpler `Vec<(TokenTree, IsJoint)>`. This reduces complexity significantly. In particular, `StreamCursor` is eliminated, and `Cursor` becomes much simpler, consisting now of just a `TokenStream` and an index. The commit also removes the `Extend` impl for `TokenStream`, because it is only used in tests. (The commit also removes those tests.) Overall, the commit reduces the number of lines of code by almost 200. --- src/libsyntax/attr/mod.rs | 8 +- src/libsyntax/ext/quote.rs | 2 +- src/libsyntax/ext/tt/transcribe.rs | 6 +- src/libsyntax/parse/lexer/tokentrees.rs | 8 +- src/libsyntax/parse/parser.rs | 2 +- src/libsyntax/tokenstream.rs | 450 +++++++----------------- src/libsyntax_ext/proc_macro_server.rs | 13 +- 7 files changed, 148 insertions(+), 341 deletions(-) diff --git a/src/libsyntax/attr/mod.rs b/src/libsyntax/attr/mod.rs index a309775a1a40b..d03563f8891aa 100644 --- a/src/libsyntax/attr/mod.rs +++ b/src/libsyntax/attr/mod.rs @@ -472,7 +472,7 @@ impl MetaItem { Token::from_ast_ident(segment.ident)).into()); last_pos = segment.ident.span.hi(); } - idents.push(self.node.tokens(self.span)); + self.node.tokens(self.span).append_to_tree_and_joint_vec(&mut idents); TokenStream::new(idents) } @@ -529,7 +529,9 @@ impl MetaItemKind { match *self { MetaItemKind::Word => TokenStream::empty(), MetaItemKind::NameValue(ref lit) => { - TokenStream::new(vec![TokenTree::Token(span, Token::Eq).into(), lit.tokens()]) + let mut vec = vec![TokenTree::Token(span, Token::Eq).into()]; + lit.tokens().append_to_tree_and_joint_vec(&mut vec); + TokenStream::new(vec) } MetaItemKind::List(ref list) => { let mut tokens = Vec::new(); @@ -537,7 +539,7 @@ impl MetaItemKind { if i > 0 { tokens.push(TokenTree::Token(span, Token::Comma).into()); } - tokens.push(item.node.tokens()); + item.node.tokens().append_to_tree_and_joint_vec(&mut tokens); } TokenTree::Delimited( DelimSpan::from_single(span), diff --git a/src/libsyntax/ext/quote.rs b/src/libsyntax/ext/quote.rs index ad8668aca70f9..c3124144009ab 100644 --- a/src/libsyntax/ext/quote.rs +++ b/src/libsyntax/ext/quote.rs @@ -233,7 +233,7 @@ pub mod rt { self.span, token::Token::from_ast_ident(segment.ident) ).into()); } - inner.push(self.tokens.clone()); + self.tokens.clone().append_to_tree_and_joint_vec(&mut inner); let delim_span = DelimSpan::from_single(self.span); r.push(TokenTree::Delimited( diff --git a/src/libsyntax/ext/tt/transcribe.rs b/src/libsyntax/ext/tt/transcribe.rs index 31d87508c6bca..0ef2d3b749d81 100644 --- a/src/libsyntax/ext/tt/transcribe.rs +++ b/src/libsyntax/ext/tt/transcribe.rs @@ -7,7 +7,7 @@ use fold::noop_fold_tt; use parse::token::{self, Token, NtTT}; use smallvec::SmallVec; use syntax_pos::DUMMY_SP; -use tokenstream::{TokenStream, TokenTree, DelimSpan}; +use tokenstream::{DelimSpan, TokenStream, TokenTree, TreeAndJoint}; use rustc_data_structures::fx::FxHashMap; use rustc_data_structures::sync::Lrc; @@ -63,7 +63,7 @@ pub fn transcribe(cx: &ExtCtxt, let mut stack: SmallVec<[Frame; 1]> = smallvec![Frame::new(src)]; let interpolations = interp.unwrap_or_else(FxHashMap::default); /* just a convenience */ let mut repeats = Vec::new(); - let mut result: Vec = Vec::new(); + let mut result: Vec = Vec::new(); let mut result_stack = Vec::new(); loop { @@ -78,7 +78,7 @@ pub fn transcribe(cx: &ExtCtxt, if let Some(sep) = sep.clone() { // repeat same span, I guess let prev_span = match result.last() { - Some(stream) => stream.trees().next().unwrap().span(), + Some((tt, _)) => tt.span(), None => DUMMY_SP, }; result.push(TokenTree::Token(prev_span, sep).into()); diff --git a/src/libsyntax/parse/lexer/tokentrees.rs b/src/libsyntax/parse/lexer/tokentrees.rs index 6c4e9e1c940c5..d219f29f06c20 100644 --- a/src/libsyntax/parse/lexer/tokentrees.rs +++ b/src/libsyntax/parse/lexer/tokentrees.rs @@ -1,7 +1,7 @@ use print::pprust::token_to_string; use parse::lexer::StringReader; use parse::{token, PResult}; -use tokenstream::{DelimSpan, IsJoint::*, TokenStream, TokenTree}; +use tokenstream::{DelimSpan, IsJoint::*, TokenStream, TokenTree, TreeAndJoint}; impl<'a> StringReader<'a> { // Parse a stream of tokens into a list of `TokenTree`s, up to an `Eof`. @@ -33,7 +33,7 @@ impl<'a> StringReader<'a> { } } - fn parse_token_tree(&mut self) -> PResult<'a, TokenStream> { + fn parse_token_tree(&mut self) -> PResult<'a, TreeAndJoint> { let sm = self.sess.source_map(); match self.token { token::Eof => { @@ -156,7 +156,7 @@ impl<'a> StringReader<'a> { Ok(TokenTree::Delimited( delim_span, delim, - tts.into(), + tts.into() ).into()) }, token::CloseDelim(_) => { @@ -176,7 +176,7 @@ impl<'a> StringReader<'a> { let raw = self.span_src_raw; self.real_token(); let is_joint = raw.hi() == self.span_src_raw.lo() && token::is_op(&self.token); - Ok(TokenStream::Tree(tt, if is_joint { Joint } else { NonJoint })) + Ok((tt, if is_joint { Joint } else { NonJoint })) } } } diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index 1e4a26b353759..eababf58dfa4c 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -2914,7 +2914,7 @@ impl<'a> Parser<'a> { TokenTree::Delimited( frame.span, frame.delim, - frame.tree_cursor.original_stream().into(), + frame.tree_cursor.stream.into(), ) }, token::CloseDelim(_) | token::Eof => unreachable!(), diff --git a/src/libsyntax/tokenstream.rs b/src/libsyntax/tokenstream.rs index 4b33a715c5c89..fb72ef9c956ce 100644 --- a/src/libsyntax/tokenstream.rs +++ b/src/libsyntax/tokenstream.rs @@ -147,9 +147,11 @@ impl TokenTree { pub enum TokenStream { Empty, Tree(TokenTree, IsJoint), - Stream(Lrc>), + Stream(Lrc>), } +pub type TreeAndJoint = (TokenTree, IsJoint); + // `TokenStream` is used a lot. Make sure it doesn't unintentionally get bigger. #[cfg(target_arch = "x86_64")] static_assert!(MEM_SIZE_OF_TOKEN_STREAM: mem::size_of::() == 32); @@ -173,16 +175,14 @@ impl TokenStream { while let Some((pos, ts)) = iter.next() { if let Some((_, next)) = iter.peek() { let sp = match (&ts, &next) { - (TokenStream::Tree(TokenTree::Token(_, token::Token::Comma), NonJoint), _) | - (_, TokenStream::Tree(TokenTree::Token(_, token::Token::Comma), NonJoint)) - => continue, - (TokenStream::Tree(TokenTree::Token(sp, _), NonJoint), _) => *sp, - (TokenStream::Tree(TokenTree::Delimited(sp, ..), NonJoint), _) => - sp.entire(), + ((TokenTree::Token(_, token::Token::Comma), NonJoint), _) | + (_, (TokenTree::Token(_, token::Token::Comma), NonJoint)) => continue, + ((TokenTree::Token(sp, _), NonJoint), _) => *sp, + ((TokenTree::Delimited(sp, ..), NonJoint), _) => sp.entire(), _ => continue, }; let sp = sp.shrink_to_hi(); - let comma = TokenStream::Tree(TokenTree::Token(sp, token::Comma), NonJoint); + let comma = (TokenTree::Token(sp, token::Comma), NonJoint); suggestion = Some((pos, comma, sp)); } } @@ -200,8 +200,14 @@ impl TokenStream { } impl From for TokenStream { - fn from(tt: TokenTree) -> TokenStream { - TokenStream::Tree(tt, NonJoint) + fn from(tree: TokenTree) -> TokenStream { + TokenStream::Tree(tree, NonJoint) + } +} + +impl From for TreeAndJoint { + fn from(tree: TokenTree) -> TreeAndJoint { + (tree, NonJoint) } } @@ -213,56 +219,7 @@ impl From for TokenStream { impl> iter::FromIterator for TokenStream { fn from_iter>(iter: I) -> Self { - TokenStream::new(iter.into_iter().map(Into::into).collect::>()) - } -} - -impl Extend for TokenStream { - fn extend>(&mut self, iter: I) { - let iter = iter.into_iter(); - let this = mem::replace(self, TokenStream::Empty); - - // Vector of token streams originally in self. - let tts: Vec = match this { - TokenStream::Empty => { - let mut vec = Vec::new(); - vec.reserve(iter.size_hint().0); - vec - } - TokenStream::Tree(..) => { - let mut vec = Vec::new(); - vec.reserve(1 + iter.size_hint().0); - vec.push(this); - vec - } - TokenStream::Stream(rc_vec) => match Lrc::try_unwrap(rc_vec) { - Ok(mut vec) => { - // Extend in place using the existing capacity if possible. - // This is the fast path for libraries like `quote` that - // build a token stream. - vec.reserve(iter.size_hint().0); - vec - } - Err(rc_vec) => { - // Self is shared so we need to copy and extend that. - let mut vec = Vec::new(); - vec.reserve(rc_vec.len() + iter.size_hint().0); - vec.extend_from_slice(&rc_vec); - vec - } - } - }; - - // Perform the extend, joining tokens as needed along the way. - let mut builder = TokenStreamBuilder(tts); - for stream in iter { - builder.push(stream); - } - - // Build the resulting token stream. If it contains more than one token, - // preserve capacity in the vector in anticipation of the caller - // performing additional calls to extend. - *self = TokenStream::new(builder.0); + TokenStream::from_streams(iter.into_iter().map(Into::into).collect::>()) } } @@ -294,14 +251,43 @@ impl TokenStream { } } - pub fn new(mut streams: Vec) -> TokenStream { + fn from_streams(mut streams: Vec) -> TokenStream { match streams.len() { 0 => TokenStream::empty(), 1 => streams.pop().unwrap(), + _ => { + let mut vec = vec![]; + for stream in streams { + match stream { + TokenStream::Empty => {}, + TokenStream::Tree(tree, is_joint) => vec.push((tree, is_joint)), + TokenStream::Stream(stream2) => vec.extend(stream2.iter().cloned()), + } + } + TokenStream::new(vec) + } + } + } + + pub fn new(mut streams: Vec) -> TokenStream { + match streams.len() { + 0 => TokenStream::empty(), + 1 => { + let (tree, is_joint) = streams.pop().unwrap(); + TokenStream::Tree(tree, is_joint) + } _ => TokenStream::Stream(Lrc::new(streams)), } } + pub fn append_to_tree_and_joint_vec(self, vec: &mut Vec) { + match self { + TokenStream::Empty => {} + TokenStream::Tree(tree, is_joint) => vec.push((tree, is_joint)), + TokenStream::Stream(stream) => vec.extend(stream.iter().cloned()), + } + } + pub fn trees(&self) -> Cursor { self.clone().into_trees() } @@ -362,54 +348,58 @@ impl TokenStream { t1.next().is_none() && t2.next().is_none() } - /// Precondition: `self` consists of a single token tree. - /// Returns true if the token tree is a joint operation w.r.t. `proc_macro::TokenNode`. - pub fn as_tree(self) -> (TokenTree, bool /* joint? */) { - match self { - TokenStream::Tree(tree, is_joint) => (tree, is_joint == Joint), - _ => unreachable!(), - } - } - pub fn map_enumerated TokenTree>(self, mut f: F) -> TokenStream { - let mut trees = self.into_trees(); - let mut result = Vec::new(); - let mut i = 0; - while let Some(stream) = trees.next_as_stream() { - result.push(match stream { - TokenStream::Tree(tree, is_joint) => TokenStream::Tree(f(i, tree), is_joint), - _ => unreachable!() - }); - i += 1; + match self { + TokenStream::Empty => TokenStream::Empty, + TokenStream::Tree(tree, is_joint) => TokenStream::Tree(f(0, tree), is_joint), + TokenStream::Stream(stream) => TokenStream::Stream(Lrc::new( + stream + .iter() + .enumerate() + .map(|(i, (tree, is_joint))| (f(i, tree.clone()), *is_joint)) + .collect() + )), } - TokenStream::new(result) } pub fn map TokenTree>(self, mut f: F) -> TokenStream { - let mut trees = self.into_trees(); - let mut result = Vec::new(); - while let Some(stream) = trees.next_as_stream() { - result.push(match stream { - TokenStream::Tree(tree, is_joint) => TokenStream::Tree(f(tree), is_joint), - _ => unreachable!() - }); + match self { + TokenStream::Empty => TokenStream::Empty, + TokenStream::Tree(tree, is_joint) => TokenStream::Tree(f(tree), is_joint), + TokenStream::Stream(stream) => TokenStream::Stream(Lrc::new( + stream + .iter() + .map(|(tree, is_joint)| (f(tree.clone()), *is_joint)) + .collect() + )), } - TokenStream::new(result) } fn first_tree_and_joint(&self) -> Option<(TokenTree, IsJoint)> { match self { TokenStream::Empty => None, TokenStream::Tree(ref tree, is_joint) => Some((tree.clone(), *is_joint)), - TokenStream::Stream(ref stream) => stream.first().unwrap().first_tree_and_joint(), + TokenStream::Stream(ref stream) => Some(stream.first().unwrap().clone()) } } fn last_tree_if_joint(&self) -> Option { match self { - TokenStream::Empty | TokenStream::Tree(_, NonJoint) => None, - TokenStream::Tree(ref tree, Joint) => Some(tree.clone()), - TokenStream::Stream(ref stream) => stream.last().unwrap().last_tree_if_joint(), + TokenStream::Empty => None, + TokenStream::Tree(ref tree, is_joint) => { + if *is_joint == Joint { + Some(tree.clone()) + } else { + None + } + } + TokenStream::Stream(ref stream) => { + if let (tree, Joint) = stream.last().unwrap() { + Some(tree.clone()) + } else { + None + } + } } } } @@ -442,13 +432,8 @@ impl TokenStreamBuilder { self.0.push(stream); } - pub fn add>(mut self, stream: T) -> Self { - self.push(stream); - self - } - pub fn build(self) -> TokenStream { - TokenStream::new(self.0) + TokenStream::from_streams(self.0) } fn push_all_but_last_tree(&mut self, stream: &TokenStream) { @@ -456,10 +441,9 @@ impl TokenStreamBuilder { let len = streams.len(); match len { 1 => {} - 2 => self.0.push(streams[0].clone().into()), - _ => self.0.push(TokenStream::new(streams[0 .. len - 1].to_vec())), + 2 => self.0.push(TokenStream::Tree(streams[0].0.clone(), streams[0].1)), + _ => self.0.push(TokenStream::Stream(Lrc::new(streams[0 .. len - 1].to_vec()))), } - self.push_all_but_last_tree(&streams[len - 1]) } } @@ -468,154 +452,77 @@ impl TokenStreamBuilder { let len = streams.len(); match len { 1 => {} - 2 => self.0.push(streams[1].clone().into()), - _ => self.0.push(TokenStream::new(streams[1 .. len].to_vec())), + 2 => self.0.push(TokenStream::Tree(streams[1].0.clone(), streams[1].1)), + _ => self.0.push(TokenStream::Stream(Lrc::new(streams[1 .. len].to_vec()))), } - self.push_all_but_first_tree(&streams[0]) } } } #[derive(Clone)] -pub struct Cursor(CursorKind); - -#[derive(Clone)] -enum CursorKind { - Empty, - Tree(TokenTree, IsJoint, bool /* consumed? */), - Stream(StreamCursor), -} - -#[derive(Clone)] -struct StreamCursor { - stream: Lrc>, +pub struct Cursor { + pub stream: TokenStream, index: usize, - stack: Vec<(Lrc>, usize)>, -} - -impl StreamCursor { - fn new(stream: Lrc>) -> Self { - StreamCursor { stream: stream, index: 0, stack: Vec::new() } - } - - fn next_as_stream(&mut self) -> Option { - loop { - if self.index < self.stream.len() { - self.index += 1; - let next = self.stream[self.index - 1].clone(); - match next { - TokenStream::Empty => {} - TokenStream::Tree(..) => return Some(next), - TokenStream::Stream(stream) => self.insert(stream), - } - } else if let Some((stream, index)) = self.stack.pop() { - self.stream = stream; - self.index = index; - } else { - return None; - } - } - } - - fn insert(&mut self, stream: Lrc>) { - self.stack.push((mem::replace(&mut self.stream, stream), - mem::replace(&mut self.index, 0))); - } } impl Iterator for Cursor { type Item = TokenTree; fn next(&mut self) -> Option { - self.next_as_stream().map(|stream| match stream { - TokenStream::Tree(tree, _) => tree, - _ => unreachable!() - }) + self.next_with_joint().map(|(tree, _)| tree) } } impl Cursor { fn new(stream: TokenStream) -> Self { - Cursor(match stream { - TokenStream::Empty => CursorKind::Empty, - TokenStream::Tree(tree, is_joint) => CursorKind::Tree(tree, is_joint, false), - TokenStream::Stream(stream) => CursorKind::Stream(StreamCursor::new(stream)), - }) - } - - pub fn next_as_stream(&mut self) -> Option { - let (stream, consumed) = match self.0 { - CursorKind::Tree(ref tree, ref is_joint, ref mut consumed @ false) => - (TokenStream::Tree(tree.clone(), *is_joint), consumed), - CursorKind::Stream(ref mut cursor) => return cursor.next_as_stream(), - _ => return None, - }; - - *consumed = true; - Some(stream) + Cursor { stream, index: 0 } } - pub fn insert(&mut self, stream: TokenStream) { - match self.0 { - _ if stream.is_empty() => return, - CursorKind::Empty => *self = stream.trees(), - CursorKind::Tree(_, _, consumed) => { - *self = TokenStream::new(vec![self.original_stream(), stream]).trees(); - if consumed { - self.next(); + pub fn next_with_joint(&mut self) -> Option { + match self.stream { + TokenStream::Empty => None, + TokenStream::Tree(ref tree, ref is_joint) => { + if self.index == 0 { + self.index = 1; + Some((tree.clone(), *is_joint)) + } else { + None } } - CursorKind::Stream(ref mut cursor) => { - cursor.insert(ThinTokenStream::from(stream).0.unwrap()); + TokenStream::Stream(ref stream) => { + if self.index < stream.len() { + self.index += 1; + Some(stream[self.index - 1].clone()) + } else { + None + } } } } - pub fn original_stream(&self) -> TokenStream { - match self.0 { - CursorKind::Empty => TokenStream::empty(), - CursorKind::Tree(ref tree, ref is_joint, _) => - TokenStream::Tree(tree.clone(), *is_joint), - CursorKind::Stream(ref cursor) => TokenStream::Stream( - cursor.stack.get(0).cloned().map(|(stream, _)| stream) - .unwrap_or_else(|| cursor.stream.clone()) - ), + pub fn append(&mut self, new_stream: TokenStream) { + if new_stream.is_empty() { + return; } + let index = self.index; + let stream = mem::replace(&mut self.stream, TokenStream::Empty); + *self = TokenStream::from_streams(vec![stream, new_stream]).into_trees(); + self.index = index; } pub fn look_ahead(&self, n: usize) -> Option { - fn look_ahead(streams: &[TokenStream], mut n: usize) -> Result { - for stream in streams { - n = match stream { - TokenStream::Tree(ref tree, _) if n == 0 => return Ok(tree.clone()), - TokenStream::Tree(..) => n - 1, - TokenStream::Stream(ref stream) => match look_ahead(stream, n) { - Ok(tree) => return Ok(tree), - Err(n) => n, - }, - _ => n, - }; + match self.stream { + TokenStream::Empty => None, + TokenStream::Tree(ref tree, _) => { + if n == 0 && self.index == 0 { + Some(tree.clone()) + } else { + None + } } - Err(n) + TokenStream::Stream(ref stream) => + stream[self.index ..].get(n).map(|(tree, _)| tree.clone()), } - - match self.0 { - CursorKind::Empty | - CursorKind::Tree(_, _, true) => Err(n), - CursorKind::Tree(ref tree, _, false) => look_ahead(&[tree.clone().into()], n), - CursorKind::Stream(ref cursor) => { - look_ahead(&cursor.stream[cursor.index ..], n).or_else(|mut n| { - for &(ref stream, index) in cursor.stack.iter().rev() { - n = match look_ahead(&stream[index..], n) { - Ok(tree) => return Ok(tree), - Err(n) => n, - } - } - - Err(n) - }) - } - }.ok() } } @@ -623,7 +530,7 @@ impl Cursor { /// `ThinTokenStream` is smaller, but needs to allocate to represent a single `TokenTree`. /// We must use `ThinTokenStream` in `TokenTree::Delimited` to avoid infinite size due to recursion. #[derive(Debug, Clone)] -pub struct ThinTokenStream(Option>>); +pub struct ThinTokenStream(Option>>); impl ThinTokenStream { pub fn stream(&self) -> TokenStream { @@ -635,7 +542,7 @@ impl From for ThinTokenStream { fn from(stream: TokenStream) -> ThinTokenStream { ThinTokenStream(match stream { TokenStream::Empty => None, - TokenStream::Tree(..) => Some(Lrc::new(vec![stream])), + TokenStream::Tree(tree, is_joint) => Some(Lrc::new(vec![(tree, is_joint)])), TokenStream::Stream(stream) => Some(stream), }) } @@ -742,7 +649,7 @@ mod tests { let test_res = string_to_ts("foo::bar::baz"); let test_fst = string_to_ts("foo::bar"); let test_snd = string_to_ts("::baz"); - let eq_res = TokenStream::new(vec![test_fst, test_snd]); + let eq_res = TokenStream::from_streams(vec![test_fst, test_snd]); assert_eq!(test_res.trees().count(), 5); assert_eq!(eq_res.trees().count(), 5); assert_eq!(test_res.eq_unspanned(&eq_res), true); @@ -827,107 +734,4 @@ mod tests { assert!(stream.eq_unspanned(&string_to_ts("..."))); assert_eq!(stream.trees().count(), 1); } - - #[test] - fn test_extend_empty() { - with_globals(|| { - // Append a token onto an empty token stream. - let mut stream = TokenStream::empty(); - stream.extend(vec![string_to_ts("t")]); - - let expected = string_to_ts("t"); - assert!(stream.eq_unspanned(&expected)); - }); - } - - #[test] - fn test_extend_nothing() { - with_globals(|| { - // Append nothing onto a token stream containing one token. - let mut stream = string_to_ts("t"); - stream.extend(vec![]); - - let expected = string_to_ts("t"); - assert!(stream.eq_unspanned(&expected)); - }); - } - - #[test] - fn test_extend_single() { - with_globals(|| { - // Append a token onto token stream containing a single token. - let mut stream = string_to_ts("t1"); - stream.extend(vec![string_to_ts("t2")]); - - let expected = string_to_ts("t1 t2"); - assert!(stream.eq_unspanned(&expected)); - }); - } - - #[test] - fn test_extend_in_place() { - with_globals(|| { - // Append a token onto token stream containing a reference counted - // vec of tokens. The token stream has a reference count of 1 so - // this can happen in place. - let mut stream = string_to_ts("t1 t2"); - stream.extend(vec![string_to_ts("t3")]); - - let expected = string_to_ts("t1 t2 t3"); - assert!(stream.eq_unspanned(&expected)); - }); - } - - #[test] - fn test_extend_copy() { - with_globals(|| { - // Append a token onto token stream containing a reference counted - // vec of tokens. The token stream is shared so the extend takes - // place on a copy. - let mut stream = string_to_ts("t1 t2"); - let _incref = stream.clone(); - stream.extend(vec![string_to_ts("t3")]); - - let expected = string_to_ts("t1 t2 t3"); - assert!(stream.eq_unspanned(&expected)); - }); - } - - #[test] - fn test_extend_no_join() { - with_globals(|| { - let first = TokenTree::Token(DUMMY_SP, Token::Dot); - let second = TokenTree::Token(DUMMY_SP, Token::Dot); - - // Append a dot onto a token stream containing a dot, but do not - // join them. - let mut stream = TokenStream::from(first); - stream.extend(vec![TokenStream::from(second)]); - - let expected = string_to_ts(". ."); - assert!(stream.eq_unspanned(&expected)); - - let unexpected = string_to_ts(".."); - assert!(!stream.eq_unspanned(&unexpected)); - }); - } - - #[test] - fn test_extend_join() { - with_globals(|| { - let first = TokenTree::Token(DUMMY_SP, Token::Dot).joint(); - let second = TokenTree::Token(DUMMY_SP, Token::Dot); - - // Append a dot onto a token stream containing a dot, forming a - // dotdot. - let mut stream = first; - stream.extend(vec![TokenStream::from(second)]); - - let expected = string_to_ts(".."); - assert!(stream.eq_unspanned(&expected)); - - let unexpected = string_to_ts(". ."); - assert!(!stream.eq_unspanned(&unexpected)); - }); - } } diff --git a/src/libsyntax_ext/proc_macro_server.rs b/src/libsyntax_ext/proc_macro_server.rs index afd86a4f7465c..158cbc791ef50 100644 --- a/src/libsyntax_ext/proc_macro_server.rs +++ b/src/libsyntax_ext/proc_macro_server.rs @@ -11,7 +11,7 @@ use syntax::ast; use syntax::ext::base::ExtCtxt; use syntax::parse::lexer::comments; use syntax::parse::{self, token, ParseSess}; -use syntax::tokenstream::{self, DelimSpan, IsJoint::*, TokenStream}; +use syntax::tokenstream::{self, DelimSpan, IsJoint::*, TokenStream, TreeAndJoint}; use syntax_pos::hygiene::{SyntaxContext, Transparency}; use syntax_pos::symbol::{keywords, Symbol}; use syntax_pos::{BytePos, FileName, MultiSpan, Pos, SourceFile, Span}; @@ -46,13 +46,14 @@ impl ToInternal for Delimiter { } } -impl FromInternal<(TokenStream, &'_ ParseSess, &'_ mut Vec)> +impl FromInternal<(TreeAndJoint, &'_ ParseSess, &'_ mut Vec)> for TokenTree { - fn from_internal((stream, sess, stack): (TokenStream, &ParseSess, &mut Vec)) -> Self { + fn from_internal(((tree, is_joint), sess, stack): (TreeAndJoint, &ParseSess, &mut Vec)) + -> Self { use syntax::parse::token::*; - let (tree, joint) = stream.as_tree(); + let joint = is_joint == Joint; let (span, token) = match tree { tokenstream::TokenTree::Delimited(span, delim, tts) => { let delimiter = Delimiter::from_internal(delim); @@ -450,7 +451,7 @@ impl server::TokenStreamIter for Rustc<'_> { ) -> Option> { loop { let tree = iter.stack.pop().or_else(|| { - let next = iter.cursor.next_as_stream()?; + let next = iter.cursor.next_with_joint()?; Some(TokenTree::from_internal((next, self.sess, &mut iter.stack))) })?; // HACK: The condition "dummy span + group with empty delimiter" represents an AST @@ -461,7 +462,7 @@ impl server::TokenStreamIter for Rustc<'_> { // and not doing the roundtrip through AST. if let TokenTree::Group(ref group) = tree { if group.delimiter == Delimiter::None && group.span.entire().is_dummy() { - iter.cursor.insert(group.stream.clone()); + iter.cursor.append(group.stream.clone()); continue; } } From 6d442938e04a851dc8b01ad810e150fb04129986 Mon Sep 17 00:00:00 2001 From: Taiki Endo Date: Sun, 13 Jan 2019 02:57:27 +0900 Subject: [PATCH 6/8] Add #[must_use] message to Iterator and Future --- src/libcore/future/future.rs | 2 +- src/libcore/iter/iterator.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libcore/future/future.rs b/src/libcore/future/future.rs index 0bc8a0fd26a04..539b07fc21eea 100644 --- a/src/libcore/future/future.rs +++ b/src/libcore/future/future.rs @@ -23,7 +23,7 @@ use task::{Poll, LocalWaker}; /// /// When using a future, you generally won't call `poll` directly, but instead /// `await!` the value. -#[must_use] +#[must_use = "futures do nothing unless polled"] pub trait Future { /// The result of the `Future`. type Output; diff --git a/src/libcore/iter/iterator.rs b/src/libcore/iter/iterator.rs index 1ea500858ed16..6cac152aab5e8 100644 --- a/src/libcore/iter/iterator.rs +++ b/src/libcore/iter/iterator.rs @@ -88,7 +88,7 @@ fn _assert_is_object_safe(_: &dyn Iterator) {} message="`{Self}` is not an iterator" )] #[doc(spotlight)] -#[must_use] +#[must_use = "iterator adaptors are lazy and do nothing unless consumed"] pub trait Iterator { /// The type of the elements being iterated over. #[stable(feature = "rust1", since = "1.0.0")] From a6535d78dc5237574e323682fb56cff2575840bd Mon Sep 17 00:00:00 2001 From: Taiki Endo Date: Sun, 13 Jan 2019 14:46:42 +0900 Subject: [PATCH 7/8] Change #[must_use] message of Iterator --- src/libcore/iter/iterator.rs | 2 +- src/libcore/iter/mod.rs | 42 +++++++++---------- .../streaming_iterator.rs | 2 +- 3 files changed, 23 insertions(+), 23 deletions(-) diff --git a/src/libcore/iter/iterator.rs b/src/libcore/iter/iterator.rs index 6cac152aab5e8..640af74817282 100644 --- a/src/libcore/iter/iterator.rs +++ b/src/libcore/iter/iterator.rs @@ -88,7 +88,7 @@ fn _assert_is_object_safe(_: &dyn Iterator) {} message="`{Self}` is not an iterator" )] #[doc(spotlight)] -#[must_use = "iterator adaptors are lazy and do nothing unless consumed"] +#[must_use = "iterators are lazy and do nothing unless consumed"] pub trait Iterator { /// The type of the elements being iterated over. #[stable(feature = "rust1", since = "1.0.0")] diff --git a/src/libcore/iter/mod.rs b/src/libcore/iter/mod.rs index 03369d6c8f3fd..2c3fe1526bcb3 100644 --- a/src/libcore/iter/mod.rs +++ b/src/libcore/iter/mod.rs @@ -404,7 +404,7 @@ impl LoopState { /// [`rev`]: trait.Iterator.html#method.rev /// [`Iterator`]: trait.Iterator.html #[derive(Clone, Debug)] -#[must_use = "iterator adaptors are lazy and do nothing unless consumed"] +#[must_use = "iterators are lazy and do nothing unless consumed"] #[stable(feature = "rust1", since = "1.0.0")] pub struct Rev { iter: T @@ -505,7 +505,7 @@ unsafe impl TrustedLen for Rev /// [`copied`]: trait.Iterator.html#method.copied /// [`Iterator`]: trait.Iterator.html #[unstable(feature = "iter_copied", issue = "57127")] -#[must_use = "iterator adaptors are lazy and do nothing unless consumed"] +#[must_use = "iterators are lazy and do nothing unless consumed"] #[derive(Clone, Debug)] pub struct Copied { it: I, @@ -605,7 +605,7 @@ unsafe impl<'a, I, T: 'a> TrustedLen for Copied /// [`cloned`]: trait.Iterator.html#method.cloned /// [`Iterator`]: trait.Iterator.html #[stable(feature = "iter_cloned", since = "1.1.0")] -#[must_use = "iterator adaptors are lazy and do nothing unless consumed"] +#[must_use = "iterators are lazy and do nothing unless consumed"] #[derive(Clone, Debug)] pub struct Cloned { it: I, @@ -717,7 +717,7 @@ unsafe impl<'a, I, T: 'a> TrustedLen for Cloned /// [`cycle`]: trait.Iterator.html#method.cycle /// [`Iterator`]: trait.Iterator.html #[derive(Clone, Debug)] -#[must_use = "iterator adaptors are lazy and do nothing unless consumed"] +#[must_use = "iterators are lazy and do nothing unless consumed"] #[stable(feature = "rust1", since = "1.0.0")] pub struct Cycle { orig: I, @@ -757,7 +757,7 @@ impl FusedIterator for Cycle where I: Clone + Iterator {} /// /// [`step_by`]: trait.Iterator.html#method.step_by /// [`Iterator`]: trait.Iterator.html -#[must_use = "iterator adaptors are lazy and do nothing unless consumed"] +#[must_use = "iterators are lazy and do nothing unless consumed"] #[stable(feature = "iterator_step_by", since = "1.28.0")] #[derive(Clone, Debug)] pub struct StepBy { @@ -849,7 +849,7 @@ impl ExactSizeIterator for StepBy where I: ExactSizeIterator {} /// [`chain`]: trait.Iterator.html#method.chain /// [`Iterator`]: trait.Iterator.html #[derive(Clone, Debug)] -#[must_use = "iterator adaptors are lazy and do nothing unless consumed"] +#[must_use = "iterators are lazy and do nothing unless consumed"] #[stable(feature = "rust1", since = "1.0.0")] pub struct Chain { a: A, @@ -1100,7 +1100,7 @@ unsafe impl TrustedLen for Chain /// [`zip`]: trait.Iterator.html#method.zip /// [`Iterator`]: trait.Iterator.html #[derive(Clone, Debug)] -#[must_use = "iterator adaptors are lazy and do nothing unless consumed"] +#[must_use = "iterators are lazy and do nothing unless consumed"] #[stable(feature = "rust1", since = "1.0.0")] pub struct Zip { a: A, @@ -1400,7 +1400,7 @@ unsafe impl TrustedLen for Zip /// println!("{:?}", pair); /// } /// ``` -#[must_use = "iterator adaptors are lazy and do nothing unless consumed"] +#[must_use = "iterators are lazy and do nothing unless consumed"] #[stable(feature = "rust1", since = "1.0.0")] #[derive(Clone)] pub struct Map { @@ -1511,7 +1511,7 @@ unsafe impl TrustedRandomAccess for Map /// /// [`filter`]: trait.Iterator.html#method.filter /// [`Iterator`]: trait.Iterator.html -#[must_use = "iterator adaptors are lazy and do nothing unless consumed"] +#[must_use = "iterators are lazy and do nothing unless consumed"] #[stable(feature = "rust1", since = "1.0.0")] #[derive(Clone)] pub struct Filter { @@ -1643,7 +1643,7 @@ impl FusedIterator for Filter /// /// [`filter_map`]: trait.Iterator.html#method.filter_map /// [`Iterator`]: trait.Iterator.html -#[must_use = "iterator adaptors are lazy and do nothing unless consumed"] +#[must_use = "iterators are lazy and do nothing unless consumed"] #[stable(feature = "rust1", since = "1.0.0")] #[derive(Clone)] pub struct FilterMap { @@ -1754,7 +1754,7 @@ impl FusedIterator for FilterMap /// [`enumerate`]: trait.Iterator.html#method.enumerate /// [`Iterator`]: trait.Iterator.html #[derive(Clone, Debug)] -#[must_use = "iterator adaptors are lazy and do nothing unless consumed"] +#[must_use = "iterators are lazy and do nothing unless consumed"] #[stable(feature = "rust1", since = "1.0.0")] pub struct Enumerate { iter: I, @@ -1915,7 +1915,7 @@ unsafe impl TrustedLen for Enumerate /// [`peekable`]: trait.Iterator.html#method.peekable /// [`Iterator`]: trait.Iterator.html #[derive(Clone, Debug)] -#[must_use = "iterator adaptors are lazy and do nothing unless consumed"] +#[must_use = "iterators are lazy and do nothing unless consumed"] #[stable(feature = "rust1", since = "1.0.0")] pub struct Peekable { iter: I, @@ -2066,7 +2066,7 @@ impl Peekable { /// /// [`skip_while`]: trait.Iterator.html#method.skip_while /// [`Iterator`]: trait.Iterator.html -#[must_use = "iterator adaptors are lazy and do nothing unless consumed"] +#[must_use = "iterators are lazy and do nothing unless consumed"] #[stable(feature = "rust1", since = "1.0.0")] #[derive(Clone)] pub struct SkipWhile { @@ -2149,7 +2149,7 @@ impl FusedIterator for SkipWhile /// /// [`take_while`]: trait.Iterator.html#method.take_while /// [`Iterator`]: trait.Iterator.html -#[must_use = "iterator adaptors are lazy and do nothing unless consumed"] +#[must_use = "iterators are lazy and do nothing unless consumed"] #[stable(feature = "rust1", since = "1.0.0")] #[derive(Clone)] pub struct TakeWhile { @@ -2233,7 +2233,7 @@ impl FusedIterator for TakeWhile /// [`skip`]: trait.Iterator.html#method.skip /// [`Iterator`]: trait.Iterator.html #[derive(Clone, Debug)] -#[must_use = "iterator adaptors are lazy and do nothing unless consumed"] +#[must_use = "iterators are lazy and do nothing unless consumed"] #[stable(feature = "rust1", since = "1.0.0")] pub struct Skip { iter: I, @@ -2371,7 +2371,7 @@ impl FusedIterator for Skip where I: FusedIterator {} /// [`take`]: trait.Iterator.html#method.take /// [`Iterator`]: trait.Iterator.html #[derive(Clone, Debug)] -#[must_use = "iterator adaptors are lazy and do nothing unless consumed"] +#[must_use = "iterators are lazy and do nothing unless consumed"] #[stable(feature = "rust1", since = "1.0.0")] pub struct Take { iter: I, @@ -2458,7 +2458,7 @@ unsafe impl TrustedLen for Take {} /// /// [`scan`]: trait.Iterator.html#method.scan /// [`Iterator`]: trait.Iterator.html -#[must_use = "iterator adaptors are lazy and do nothing unless consumed"] +#[must_use = "iterators are lazy and do nothing unless consumed"] #[stable(feature = "rust1", since = "1.0.0")] #[derive(Clone)] pub struct Scan { @@ -2518,7 +2518,7 @@ impl Iterator for Scan where /// /// [`flat_map`]: trait.Iterator.html#method.flat_map /// [`Iterator`]: trait.Iterator.html -#[must_use = "iterator adaptors are lazy and do nothing unless consumed"] +#[must_use = "iterators are lazy and do nothing unless consumed"] #[stable(feature = "rust1", since = "1.0.0")] pub struct FlatMap { inner: FlattenCompat, ::IntoIter> @@ -2603,7 +2603,7 @@ impl FusedIterator for FlatMap /// /// [`flatten`]: trait.Iterator.html#method.flatten /// [`Iterator`]: trait.Iterator.html -#[must_use = "iterator adaptors are lazy and do nothing unless consumed"] +#[must_use = "iterators are lazy and do nothing unless consumed"] #[stable(feature = "iterator_flatten", since = "1.29.0")] pub struct Flatten where I::Item: IntoIterator { @@ -2832,7 +2832,7 @@ impl DoubleEndedIterator for FlattenCompat /// [`fuse`]: trait.Iterator.html#method.fuse /// [`Iterator`]: trait.Iterator.html #[derive(Clone, Debug)] -#[must_use = "iterator adaptors are lazy and do nothing unless consumed"] +#[must_use = "iterators are lazy and do nothing unless consumed"] #[stable(feature = "rust1", since = "1.0.0")] pub struct Fuse { iter: I, @@ -3056,7 +3056,7 @@ impl ExactSizeIterator for Fuse where I: ExactSizeIterator { /// /// [`inspect`]: trait.Iterator.html#method.inspect /// [`Iterator`]: trait.Iterator.html -#[must_use = "iterator adaptors are lazy and do nothing unless consumed"] +#[must_use = "iterators are lazy and do nothing unless consumed"] #[stable(feature = "rust1", since = "1.0.0")] #[derive(Clone)] pub struct Inspect { diff --git a/src/test/ui/rfc1598-generic-associated-types/streaming_iterator.rs b/src/test/ui/rfc1598-generic-associated-types/streaming_iterator.rs index 1ef154447903b..e0184164b3ac2 100644 --- a/src/test/ui/rfc1598-generic-associated-types/streaming_iterator.rs +++ b/src/test/ui/rfc1598-generic-associated-types/streaming_iterator.rs @@ -28,7 +28,7 @@ fn foo(iter: T) where T: StreamingIterator, for<'a> T::Item<'a>: Display { /* // Full example of enumerate iterator -#[must_use = "iterator adaptors are lazy and do nothing unless consumed"] +#[must_use = "iterators are lazy and do nothing unless consumed"] struct StreamEnumerate { iter: I, count: usize, From da933cca1a2db66c904efeb8b800c0d309f8c18d Mon Sep 17 00:00:00 2001 From: Taiki Endo Date: Sun, 13 Jan 2019 15:17:57 +0900 Subject: [PATCH 8/8] Change #[must_use] message of Iterator in documentation --- src/libcore/iter/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libcore/iter/mod.rs b/src/libcore/iter/mod.rs index 2c3fe1526bcb3..1ef5428a789cf 100644 --- a/src/libcore/iter/mod.rs +++ b/src/libcore/iter/mod.rs @@ -243,7 +243,7 @@ //! using it. The compiler will warn us about this kind of behavior: //! //! ```text -//! warning: unused result that must be used: iterator adaptors are lazy and +//! warning: unused result that must be used: iterators are lazy and //! do nothing unless consumed //! ``` //!