From 6d00c9686becb0a84f7f4885fcaf9a764c201890 Mon Sep 17 00:00:00 2001 From: David Wood Date: Sat, 13 Jan 2018 23:28:00 +0000 Subject: [PATCH 1/9] Updated tests with fixed span location. --- src/test/ui/issue-46471-1.stderr | 2 +- src/test/ui/issue-46471.stderr | 2 +- src/test/ui/issue-46472.stderr | 2 +- src/test/ui/nll/capture-ref-in-struct.stderr | 2 +- src/test/ui/nll/closure-requirements/escape-argument.stderr | 2 +- .../ui/nll/closure-requirements/escape-upvar-nested.stderr | 2 +- src/test/ui/nll/closure-requirements/escape-upvar-ref.stderr | 2 +- src/test/ui/nll/drop-no-may-dangle.stderr | 4 ++-- .../nll/maybe-initialized-drop-implicit-fragment-drop.stderr | 2 +- src/test/ui/nll/maybe-initialized-drop-with-fragment.stderr | 2 +- ...maybe-initialized-drop-with-uninitialized-fragments.stderr | 2 +- src/test/ui/nll/maybe-initialized-drop.stderr | 2 +- 12 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/test/ui/issue-46471-1.stderr b/src/test/ui/issue-46471-1.stderr index 9f12092f99cc0..97dfb458d2dfb 100644 --- a/src/test/ui/issue-46471-1.stderr +++ b/src/test/ui/issue-46471-1.stderr @@ -15,7 +15,7 @@ error[E0597]: `z` does not live long enough (Mir) 16 | &mut z | ^^^^^^ borrowed value does not live long enough 17 | }; - | - `z` dropped here while still borrowed + | - `z` dropped here while still borrowed ... 21 | } | - borrowed value needs to live until here diff --git a/src/test/ui/issue-46471.stderr b/src/test/ui/issue-46471.stderr index 19fc579d198ff..4c196bba5a1f1 100644 --- a/src/test/ui/issue-46471.stderr +++ b/src/test/ui/issue-46471.stderr @@ -16,7 +16,7 @@ error[E0597]: `x` does not live long enough (Mir) | ^^ borrowed value does not live long enough ... 18 | } - | - borrowed value only lives until here + | - borrowed value only lives until here | = note: borrowed value must be valid for the static lifetime... diff --git a/src/test/ui/issue-46472.stderr b/src/test/ui/issue-46472.stderr index 50df72fc2a020..2f332a7a55850 100644 --- a/src/test/ui/issue-46472.stderr +++ b/src/test/ui/issue-46472.stderr @@ -24,7 +24,7 @@ error[E0597]: borrowed value does not live long enough (Mir) | ^ temporary value does not live long enough ... 17 | } - | - temporary value only lives until here + | - temporary value only lives until here | note: borrowed value must be valid for the lifetime 'a as defined on the function body at 13:1... --> $DIR/issue-46472.rs:13:1 diff --git a/src/test/ui/nll/capture-ref-in-struct.stderr b/src/test/ui/nll/capture-ref-in-struct.stderr index 7e7487daa67a3..d05ec91be3026 100644 --- a/src/test/ui/nll/capture-ref-in-struct.stderr +++ b/src/test/ui/nll/capture-ref-in-struct.stderr @@ -5,7 +5,7 @@ error[E0597]: `y` does not live long enough | ^^ borrowed value does not live long enough ... 38 | } - | - borrowed value only lives until here + | - borrowed value only lives until here 39 | 40 | deref(p); | - borrow later used here diff --git a/src/test/ui/nll/closure-requirements/escape-argument.stderr b/src/test/ui/nll/closure-requirements/escape-argument.stderr index 09d5617b08ef5..ee29f2f9c5c59 100644 --- a/src/test/ui/nll/closure-requirements/escape-argument.stderr +++ b/src/test/ui/nll/closure-requirements/escape-argument.stderr @@ -30,7 +30,7 @@ error[E0597]: `y` does not live long enough | ^^ borrowed value does not live long enough 38 | //~^ ERROR `y` does not live long enough [E0597] 39 | } - | - borrowed value only lives until here + | - borrowed value only lives until here 40 | 41 | deref(p); | - borrow later used here diff --git a/src/test/ui/nll/closure-requirements/escape-upvar-nested.stderr b/src/test/ui/nll/closure-requirements/escape-upvar-nested.stderr index 430fb711c635d..501d299154737 100644 --- a/src/test/ui/nll/closure-requirements/escape-upvar-nested.stderr +++ b/src/test/ui/nll/closure-requirements/escape-upvar-nested.stderr @@ -57,7 +57,7 @@ error[E0597]: `y` does not live long enough | |_________^ borrowed value does not live long enough ... 36 | } - | - borrowed value only lives until here + | - borrowed value only lives until here 37 | 38 | deref(p); | - borrow later used here diff --git a/src/test/ui/nll/closure-requirements/escape-upvar-ref.stderr b/src/test/ui/nll/closure-requirements/escape-upvar-ref.stderr index 090bacbc17d07..556cd020f7ff5 100644 --- a/src/test/ui/nll/closure-requirements/escape-upvar-ref.stderr +++ b/src/test/ui/nll/closure-requirements/escape-upvar-ref.stderr @@ -34,7 +34,7 @@ error[E0597]: `y` does not live long enough | ^^^^^^^^^ borrowed value does not live long enough ... 36 | } - | - borrowed value only lives until here + | - borrowed value only lives until here 37 | 38 | deref(p); | - borrow later used here diff --git a/src/test/ui/nll/drop-no-may-dangle.stderr b/src/test/ui/nll/drop-no-may-dangle.stderr index ef850f3a568c0..dee5873ba3be1 100644 --- a/src/test/ui/nll/drop-no-may-dangle.stderr +++ b/src/test/ui/nll/drop-no-may-dangle.stderr @@ -8,7 +8,7 @@ error[E0506]: cannot assign to `v[..]` because it is borrowed | ^^^^^^^^^ assignment to borrowed `v[..]` occurs here ... 35 | } - | - borrow later used here, when `p` is dropped + | - borrow later used here, when `p` is dropped error[E0506]: cannot assign to `v[..]` because it is borrowed --> $DIR/drop-no-may-dangle.rs:34:5 @@ -19,7 +19,7 @@ error[E0506]: cannot assign to `v[..]` because it is borrowed 34 | v[0] += 1; //~ ERROR cannot assign to `v[..]` because it is borrowed | ^^^^^^^^^ assignment to borrowed `v[..]` occurs here 35 | } - | - borrow later used here, when `p` is dropped + | - borrow later used here, when `p` is dropped error: aborting due to 2 previous errors diff --git a/src/test/ui/nll/maybe-initialized-drop-implicit-fragment-drop.stderr b/src/test/ui/nll/maybe-initialized-drop-implicit-fragment-drop.stderr index 3c685ce111a97..fbaaf5511ccd5 100644 --- a/src/test/ui/nll/maybe-initialized-drop-implicit-fragment-drop.stderr +++ b/src/test/ui/nll/maybe-initialized-drop-implicit-fragment-drop.stderr @@ -8,7 +8,7 @@ error[E0506]: cannot assign to `x` because it is borrowed | ^^^^^ assignment to borrowed `x` occurs here 33 | // FIXME ^ Should not error in the future with implicit dtors, only manually implemented ones 34 | } - | - borrow later used here, when `foo` is dropped + | - borrow later used here, when `foo` is dropped error: aborting due to previous error diff --git a/src/test/ui/nll/maybe-initialized-drop-with-fragment.stderr b/src/test/ui/nll/maybe-initialized-drop-with-fragment.stderr index 072818c7ce17b..5d526cda042e9 100644 --- a/src/test/ui/nll/maybe-initialized-drop-with-fragment.stderr +++ b/src/test/ui/nll/maybe-initialized-drop-with-fragment.stderr @@ -7,7 +7,7 @@ error[E0506]: cannot assign to `x` because it is borrowed 31 | x = 1; //~ ERROR cannot assign to `x` because it is borrowed [E0506] | ^^^^^ assignment to borrowed `x` occurs here 32 | } - | - borrow later used here, when `foo` is dropped + | - borrow later used here, when `foo` is dropped error: aborting due to previous error diff --git a/src/test/ui/nll/maybe-initialized-drop-with-uninitialized-fragments.stderr b/src/test/ui/nll/maybe-initialized-drop-with-uninitialized-fragments.stderr index 89117c2bfeafe..ecd60821194f8 100644 --- a/src/test/ui/nll/maybe-initialized-drop-with-uninitialized-fragments.stderr +++ b/src/test/ui/nll/maybe-initialized-drop-with-uninitialized-fragments.stderr @@ -8,7 +8,7 @@ error[E0506]: cannot assign to `x` because it is borrowed | ^^^^^ assignment to borrowed `x` occurs here 33 | // FIXME ^ This currently errors and it should not. 34 | } - | - borrow later used here, when `foo` is dropped + | - borrow later used here, when `foo` is dropped error: aborting due to previous error diff --git a/src/test/ui/nll/maybe-initialized-drop.stderr b/src/test/ui/nll/maybe-initialized-drop.stderr index 626307a80ed57..874d63a0441b6 100644 --- a/src/test/ui/nll/maybe-initialized-drop.stderr +++ b/src/test/ui/nll/maybe-initialized-drop.stderr @@ -6,7 +6,7 @@ error[E0506]: cannot assign to `x` because it is borrowed 26 | x = 1; //~ ERROR cannot assign to `x` because it is borrowed [E0506] | ^^^^^ assignment to borrowed `x` occurs here 27 | } - | - borrow later used here, when `wrap` is dropped + | - borrow later used here, when `wrap` is dropped error: aborting due to previous error From f6fee2a479070526495b65b6b3e7959088a1dd62 Mon Sep 17 00:00:00 2001 From: David Wood Date: Sat, 13 Jan 2018 23:28:42 +0000 Subject: [PATCH 2/9] Fixed off-by-one spans in MIR borrowck errors. --- src/librustc_mir/build/scope.rs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/librustc_mir/build/scope.rs b/src/librustc_mir/build/scope.rs index ddb4bf0e36ba7..389e06e933490 100644 --- a/src/librustc_mir/build/scope.rs +++ b/src/librustc_mir/build/scope.rs @@ -695,10 +695,17 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { if let DropKind::Value { .. } = drop_kind { scope.needs_cleanup = true; } + let region_scope_span = region_scope.span(self.hir.tcx(), &self.hir.region_scope_tree); - // Attribute scope exit drops to scope's closing brace - let scope_end = region_scope_span.with_lo(region_scope_span.hi()); + // Attribute scope exit drops to scope's closing brace. + // Without this check when finding the endpoint, we'll run into an ICE. + let scope_end = if region_scope_span.hi().0 == 0 { + region_scope_span + } else { + region_scope_span.end_point() + }; + scope.drops.push(DropData { span: scope_end, location: place.clone(), From c6e6428d1a13f61f5ffbe43697a21f3cd82cd01d Mon Sep 17 00:00:00 2001 From: David Wood Date: Sun, 14 Jan 2018 00:23:35 +0000 Subject: [PATCH 3/9] Moved overflow check into end_point function. --- src/librustc_mir/build/scope.rs | 7 +------ src/libsyntax_pos/lib.rs | 4 +++- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/src/librustc_mir/build/scope.rs b/src/librustc_mir/build/scope.rs index 389e06e933490..50e50b95f7750 100644 --- a/src/librustc_mir/build/scope.rs +++ b/src/librustc_mir/build/scope.rs @@ -699,12 +699,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { let region_scope_span = region_scope.span(self.hir.tcx(), &self.hir.region_scope_tree); // Attribute scope exit drops to scope's closing brace. - // Without this check when finding the endpoint, we'll run into an ICE. - let scope_end = if region_scope_span.hi().0 == 0 { - region_scope_span - } else { - region_scope_span.end_point() - }; + let scope_end = region_scope_span.end_point(); scope.drops.push(DropData { span: scope_end, diff --git a/src/libsyntax_pos/lib.rs b/src/libsyntax_pos/lib.rs index 85f0925b98210..5866d8e4aa932 100644 --- a/src/libsyntax_pos/lib.rs +++ b/src/libsyntax_pos/lib.rs @@ -219,7 +219,9 @@ impl Span { /// Returns a new span representing just the end-point of this span pub fn end_point(self) -> Span { let span = self.data(); - let lo = cmp::max(span.hi.0 - 1, span.lo.0); + // We can avoid an ICE by checking if subtraction would cause an overflow. + let hi = if span.hi.0 == u32::min_value() { span.hi.0 } else { span.hi.0 - 1 }; + let lo = cmp::max(hi, span.lo.0); span.with_lo(BytePos(lo)) } From c71cec8834bf30032a8e49d2949f6d8d4080b639 Mon Sep 17 00:00:00 2001 From: David Wood Date: Sun, 14 Jan 2018 17:29:07 +0000 Subject: [PATCH 4/9] end_point handling multibyte characters correctly. --- src/librustc/infer/error_reporting/mod.rs | 3 +- src/librustc_borrowck/borrowck/check_loans.rs | 4 +- src/librustc_borrowck/borrowck/mod.rs | 3 +- src/librustc_mir/borrow_check/mod.rs | 3 +- src/librustc_mir/build/scope.rs | 2 +- src/librustc_mir/dataflow/impls/borrows.rs | 4 +- src/librustc_resolve/lib.rs | 4 +- src/librustc_typeck/check/mod.rs | 12 +++--- src/libsyntax/codemap.rs | 37 +++++++++++++++++++ src/libsyntax/parse/parser.rs | 8 ++-- src/libsyntax_pos/lib.rs | 16 -------- 11 files changed, 61 insertions(+), 35 deletions(-) diff --git a/src/librustc/infer/error_reporting/mod.rs b/src/librustc/infer/error_reporting/mod.rs index 541c1356dd4ab..b10e742595720 100644 --- a/src/librustc/infer/error_reporting/mod.rs +++ b/src/librustc/infer/error_reporting/mod.rs @@ -958,7 +958,8 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { // `sp` only covers `T`, change it so that it covers // `T:` when appropriate let sp = if has_lifetimes { - sp.to(sp.next_point().next_point()) + sp.to(self.tcx.sess.codemap().next_point( + self.tcx.sess.codemap().next_point(sp))) } else { sp }; diff --git a/src/librustc_borrowck/borrowck/check_loans.rs b/src/librustc_borrowck/borrowck/check_loans.rs index 862ea0c240ab9..9888b2fffc779 100644 --- a/src/librustc_borrowck/borrowck/check_loans.rs +++ b/src/librustc_borrowck/borrowck/check_loans.rs @@ -591,8 +591,8 @@ impl<'a, 'tcx> CheckLoanCtxt<'a, 'tcx> { // 3. Where does old loan expire. let previous_end_span = - Some(old_loan.kill_scope.span(self.tcx(), &self.bccx.region_scope_tree) - .end_point()); + Some(self.tcx().sess.codemap().end_point( + old_loan.kill_scope.span(self.tcx(), &self.bccx.region_scope_tree))); let mut err = match (new_loan.kind, old_loan.kind) { (ty::MutBorrow, ty::MutBorrow) => diff --git a/src/librustc_borrowck/borrowck/mod.rs b/src/librustc_borrowck/borrowck/mod.rs index f755efc89a58e..b35e8c6b41953 100644 --- a/src/librustc_borrowck/borrowck/mod.rs +++ b/src/librustc_borrowck/borrowck/mod.rs @@ -1276,7 +1276,8 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> { fn region_end_span(&self, region: ty::Region<'tcx>) -> Option { match *region { ty::ReScope(scope) => { - Some(scope.span(self.tcx, &self.region_scope_tree).end_point()) + Some(self.tcx.sess.codemap().end_point( + scope.span(self.tcx, &self.region_scope_tree))) } _ => None } diff --git a/src/librustc_mir/borrow_check/mod.rs b/src/librustc_mir/borrow_check/mod.rs index d6937c405f961..f69236516630d 100644 --- a/src/librustc_mir/borrow_check/mod.rs +++ b/src/librustc_mir/borrow_check/mod.rs @@ -1112,10 +1112,11 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { debug!("check_for_invalidation_at_exit({:?}): INVALID", place); // FIXME: should be talking about the region lifetime instead // of just a span here. + let span = self.tcx.sess.codemap().end_point(span); self.report_borrowed_value_does_not_live_long_enough( context, borrow, - span.end_point(), + span, flow_state.borrows.operator(), ) } diff --git a/src/librustc_mir/build/scope.rs b/src/librustc_mir/build/scope.rs index 50e50b95f7750..a631ab27d1c87 100644 --- a/src/librustc_mir/build/scope.rs +++ b/src/librustc_mir/build/scope.rs @@ -699,7 +699,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { let region_scope_span = region_scope.span(self.hir.tcx(), &self.hir.region_scope_tree); // Attribute scope exit drops to scope's closing brace. - let scope_end = region_scope_span.end_point(); + let scope_end = self.hir.tcx().sess.codemap().end_point(region_scope_span); scope.drops.push(DropData { span: scope_end, diff --git a/src/librustc_mir/dataflow/impls/borrows.rs b/src/librustc_mir/dataflow/impls/borrows.rs index f543a33b130b6..d7cd8830adb00 100644 --- a/src/librustc_mir/dataflow/impls/borrows.rs +++ b/src/librustc_mir/dataflow/impls/borrows.rs @@ -537,8 +537,8 @@ impl<'a, 'gcx, 'tcx> ActiveBorrows<'a, 'gcx, 'tcx> { Some(_) => None, None => { match self.0.region_span_map.get(region) { - Some(span) => Some(span.end_point()), - None => Some(self.0.mir.span.end_point()) + Some(span) => Some(self.0.tcx.sess.codemap().end_point(*span)), + None => Some(self.0.tcx.sess.codemap().end_point(self.0.mir.span)) } } } diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 557ff887a3ef2..f7228fc3314c3 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -2871,8 +2871,8 @@ impl<'a> Resolver<'a> { if let Some(sp) = self.current_type_ascription.last() { let mut sp = *sp; loop { // try to find the `:`, bail on first non-':'/non-whitespace - sp = sp.next_point(); - if let Ok(snippet) = cm.span_to_snippet(sp.to(sp.next_point())) { + sp = cm.next_point(sp); + if let Ok(snippet) = cm.span_to_snippet(sp.to(cm.next_point(sp))) { debug!("snippet {:?}", snippet); let line_sp = cm.lookup_char_pos(sp.hi()).line; let line_base_sp = cm.lookup_char_pos(base_span.lo()).line; diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index 6147743437b8e..9b24c09036bce 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -2457,7 +2457,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { err.span_label(def_s, "defined here"); } if sugg_unit { - let sugg_span = expr_sp.end_point(); + let sugg_span = sess.codemap().end_point(expr_sp); // remove closing `)` from the span let sugg_span = sugg_span.with_hi(sugg_span.lo()); err.span_suggestion( @@ -4446,10 +4446,10 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { /// statement and the return type has been left as default or has been specified as `()`. If so, /// it suggests adding a semicolon. fn suggest_missing_semicolon(&self, - err: &mut DiagnosticBuilder<'tcx>, - expression: &'gcx hir::Expr, - expected: Ty<'tcx>, - cause_span: Span) { + err: &mut DiagnosticBuilder<'tcx>, + expression: &'gcx hir::Expr, + expected: Ty<'tcx>, + cause_span: Span) { if expected.is_nil() { // `BlockTailExpression` only relevant if the tail expr would be // useful on its own. @@ -4461,7 +4461,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { hir::ExprLoop(..) | hir::ExprMatch(..) | hir::ExprBlock(..) => { - let sp = cause_span.next_point(); + let sp = self.tcx.sess.codemap().next_point(cause_span); err.span_suggestion(sp, "try adding a semicolon", ";".to_string()); diff --git a/src/libsyntax/codemap.rs b/src/libsyntax/codemap.rs index a58a61c36361b..e74066da0ac83 100644 --- a/src/libsyntax/codemap.rs +++ b/src/libsyntax/codemap.rs @@ -25,6 +25,7 @@ pub use self::ExpnFormat::*; use rustc_data_structures::fx::FxHashMap; use rustc_data_structures::stable_hasher::StableHasher; use std::cell::{RefCell, Ref}; +use std::cmp; use std::hash::Hash; use std::path::{Path, PathBuf}; use std::rc::Rc; @@ -607,6 +608,42 @@ impl CodeMap { self.span_until_char(sp, '{') } + /// Returns a new span representing just the end-point of this span + pub fn end_point(&self, sp: Span) -> Span { + let hi = sp.hi().0.checked_sub(1).unwrap_or(sp.hi().0); + let hi = self.get_start_of_char_bytepos(BytePos(hi)); + let lo = cmp::max(hi.0, sp.lo().0); + sp.with_lo(BytePos(lo)) + } + + /// Returns a new span representing the next character after the end-point of this span + pub fn next_point(&self, sp: Span) -> Span { + let hi = sp.lo().0.checked_add(1).unwrap_or(sp.lo().0); + let hi = self.get_start_of_char_bytepos(BytePos(hi)); + let lo = cmp::max(sp.hi().0, hi.0); + Span::new(BytePos(lo), BytePos(lo), sp.ctxt()) + } + + fn get_start_of_char_bytepos(&self, bpos: BytePos) -> BytePos { + let idx = self.lookup_filemap_idx(bpos); + let files = self.files.borrow(); + let map = &(*files)[idx]; + + for mbc in map.multibyte_chars.borrow().iter() { + if mbc.pos < bpos { + if bpos.to_usize() >= mbc.pos.to_usize() + mbc.bytes { + // If we do, then return the start of the character. + return mbc.pos; + } + } else { + break; + } + } + + // If this isn't a multibyte character, return the original position. + return bpos; + } + pub fn get_filemap(&self, filename: &FileName) -> Option> { for fm in self.files.borrow().iter() { if *filename == fm.name { diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index d393cab471850..e8e87e2854b77 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -704,13 +704,15 @@ impl<'a> Parser<'a> { expect.clone() }; (format!("expected one of {}, found `{}`", expect, actual), - (self.prev_span.next_point(), format!("expected one of {} here", short_expect))) + (self.sess.codemap().next_point(self.prev_span), + format!("expected one of {} here", short_expect))) } else if expected.is_empty() { (format!("unexpected token: `{}`", actual), (self.prev_span, "unexpected token after this".to_string())) } else { (format!("expected {}, found `{}`", expect, actual), - (self.prev_span.next_point(), format!("expected {} here", expect))) + (self.sess.codemap().next_point(self.prev_span), + format!("expected {} here", expect))) }; let mut err = self.fatal(&msg_exp); let sp = if self.token == token::Token::Eof { @@ -3190,7 +3192,7 @@ impl<'a> Parser<'a> { // return. This won't catch blocks with an explicit `return`, but that would be caught by // the dead code lint. if self.eat_keyword(keywords::Else) || !cond.returns() { - let sp = lo.next_point(); + let sp = self.sess.codemap().next_point(lo); let mut err = self.diagnostic() .struct_span_err(sp, "missing condition for `if` statemement"); err.span_label(sp, "expected if condition here"); diff --git a/src/libsyntax_pos/lib.rs b/src/libsyntax_pos/lib.rs index 5866d8e4aa932..dd1ec7284f690 100644 --- a/src/libsyntax_pos/lib.rs +++ b/src/libsyntax_pos/lib.rs @@ -216,22 +216,6 @@ impl Span { self.data().with_ctxt(ctxt) } - /// Returns a new span representing just the end-point of this span - pub fn end_point(self) -> Span { - let span = self.data(); - // We can avoid an ICE by checking if subtraction would cause an overflow. - let hi = if span.hi.0 == u32::min_value() { span.hi.0 } else { span.hi.0 - 1 }; - let lo = cmp::max(hi, span.lo.0); - span.with_lo(BytePos(lo)) - } - - /// Returns a new span representing the next character after the end-point of this span - pub fn next_point(self) -> Span { - let span = self.data(); - let lo = cmp::max(span.hi.0, span.lo.0 + 1); - Span::new(BytePos(lo), BytePos(lo), span.ctxt) - } - /// Returns `self` if `self` is not the dummy span, and `other` otherwise. pub fn substitute_dummy(self, other: Span) -> Span { if self.source_equal(&DUMMY_SP) { other } else { self } From 62356471b3746012df74db22479b03ad1f6ab8ab Mon Sep 17 00:00:00 2001 From: David Wood Date: Tue, 16 Jan 2018 20:41:00 +0000 Subject: [PATCH 5/9] Replaced multi-byte character handling in end_point with potentially more performant variant. --- src/libsyntax/codemap.rs | 82 +++++++++++++++++++++++++++++----------- 1 file changed, 59 insertions(+), 23 deletions(-) diff --git a/src/libsyntax/codemap.rs b/src/libsyntax/codemap.rs index e74066da0ac83..76050f8dc09f0 100644 --- a/src/libsyntax/codemap.rs +++ b/src/libsyntax/codemap.rs @@ -610,38 +610,74 @@ impl CodeMap { /// Returns a new span representing just the end-point of this span pub fn end_point(&self, sp: Span) -> Span { - let hi = sp.hi().0.checked_sub(1).unwrap_or(sp.hi().0); - let hi = self.get_start_of_char_bytepos(BytePos(hi)); - let lo = cmp::max(hi.0, sp.lo().0); - sp.with_lo(BytePos(lo)) + let pos = sp.hi().0; + + let width = self.find_width_of_character_at_span(sp, false); + let corrected_end_position = pos.checked_sub(width).unwrap_or(pos); + + let end_point = BytePos(cmp::max(corrected_end_position, sp.lo().0)); + sp.with_lo(end_point) } /// Returns a new span representing the next character after the end-point of this span pub fn next_point(&self, sp: Span) -> Span { - let hi = sp.lo().0.checked_add(1).unwrap_or(sp.lo().0); - let hi = self.get_start_of_char_bytepos(BytePos(hi)); - let lo = cmp::max(sp.hi().0, hi.0); - Span::new(BytePos(lo), BytePos(lo), sp.ctxt()) + let pos = sp.lo().0; + + let width = self.find_width_of_character_at_span(sp, true); + let corrected_next_position = pos.checked_add(width).unwrap_or(pos); + + let next_point = BytePos(cmp::max(sp.hi().0, corrected_next_position)); + Span::new(next_point, next_point, sp.ctxt()) } - fn get_start_of_char_bytepos(&self, bpos: BytePos) -> BytePos { - let idx = self.lookup_filemap_idx(bpos); - let files = self.files.borrow(); - let map = &(*files)[idx]; + /// Finds the width of a character, either before or after the provided span. + fn find_width_of_character_at_span(&self, sp: Span, forwards: bool) -> u32 { + // Disregard malformed spans and assume a one-byte wide character. + if sp.lo() > sp.hi() { + return 1; + } - for mbc in map.multibyte_chars.borrow().iter() { - if mbc.pos < bpos { - if bpos.to_usize() >= mbc.pos.to_usize() + mbc.bytes { - // If we do, then return the start of the character. - return mbc.pos; - } - } else { - break; - } + let local_begin = self.lookup_byte_offset(sp.lo()); + let local_end = self.lookup_byte_offset(sp.hi()); + + let start_index = local_begin.pos.to_usize(); + let end_index = local_end.pos.to_usize(); + + // Disregard indexes that are at the start or end of their spans, they can't fit bigger + // characters. + if (!forwards && end_index == usize::min_value()) || + (forwards && start_index == usize::max_value()) { + return 1; + } + + let source_len = (local_begin.fm.end_pos - local_begin.fm.start_pos).to_usize(); + // Ensure indexes are also not malformed. + if start_index > end_index || end_index > source_len { + return 1; } - // If this isn't a multibyte character, return the original position. - return bpos; + // We need to extend the snippet to the end of the src rather than to end_index so when + // searching forwards for boundaries we've got somewhere to search. + let snippet = if let Some(ref src) = local_begin.fm.src { + let len = src.len(); + (&src[start_index..len]).to_string() + } else if let Some(src) = local_begin.fm.external_src.borrow().get_source() { + let len = src.len(); + (&src[start_index..len]).to_string() + } else { + return 1; + }; + + let mut target = if forwards { end_index + 1 } else { end_index - 1 }; + while !snippet.is_char_boundary(target - start_index) { + target = if forwards { target + 1 } else { target - 1 }; + } + + if forwards { + (target - end_index) as u32 + } else { + (end_index - target) as u32 + } } pub fn get_filemap(&self, filename: &FileName) -> Option> { From be465b0b85746b2f56dc4bb1842e603e8489a0f3 Mon Sep 17 00:00:00 2001 From: David Wood Date: Wed, 17 Jan 2018 10:01:57 +0000 Subject: [PATCH 6/9] next_point now handles creating spans over multibyte characters. --- src/libsyntax/codemap.rs | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/libsyntax/codemap.rs b/src/libsyntax/codemap.rs index 76050f8dc09f0..cfb891f0faaf1 100644 --- a/src/libsyntax/codemap.rs +++ b/src/libsyntax/codemap.rs @@ -621,13 +621,17 @@ impl CodeMap { /// Returns a new span representing the next character after the end-point of this span pub fn next_point(&self, sp: Span) -> Span { - let pos = sp.lo().0; + let start_of_next_point = sp.hi().0; let width = self.find_width_of_character_at_span(sp, true); - let corrected_next_position = pos.checked_add(width).unwrap_or(pos); - - let next_point = BytePos(cmp::max(sp.hi().0, corrected_next_position)); - Span::new(next_point, next_point, sp.ctxt()) + // If the width is 1, then the next span should point to the same `lo` and `hi`. However, + // in the case of a multibyte character, where the width != 1, the next span should + // span multiple bytes to include the whole character. + let end_of_next_point = start_of_next_point.checked_add( + width - 1).unwrap_or(start_of_next_point); + + let end_of_next_point = BytePos(cmp::max(sp.lo().0 + 1, end_of_next_point)); + Span::new(BytePos(start_of_next_point), end_of_next_point, sp.ctxt()) } /// Finds the width of a character, either before or after the provided span. From 71b7500241d97e1e6fb9c9c163c98b17b30c6f1c Mon Sep 17 00:00:00 2001 From: David Wood Date: Wed, 17 Jan 2018 14:30:16 +0000 Subject: [PATCH 7/9] Fix new test from rebase. --- src/test/ui/nll/return-ref-mut-issue-46557.stderr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/ui/nll/return-ref-mut-issue-46557.stderr b/src/test/ui/nll/return-ref-mut-issue-46557.stderr index 763e2bfd89294..cd77569dae0bf 100644 --- a/src/test/ui/nll/return-ref-mut-issue-46557.stderr +++ b/src/test/ui/nll/return-ref-mut-issue-46557.stderr @@ -5,7 +5,7 @@ error[E0597]: borrowed value does not live long enough | ^^^^^^^ temporary value does not live long enough 18 | x 19 | } - | - temporary value only lives until here + | - temporary value only lives until here | = note: borrowed value must be valid for lifetime '_#2r... From 0c467d5d0924e705c8a4b84b250e127c62222239 Mon Sep 17 00:00:00 2001 From: David Wood Date: Wed, 17 Jan 2018 16:41:58 +0000 Subject: [PATCH 8/9] Now handling case where span has same lo and hi. --- src/libsyntax/codemap.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/libsyntax/codemap.rs b/src/libsyntax/codemap.rs index cfb891f0faaf1..8c1bdab28a9d5 100644 --- a/src/libsyntax/codemap.rs +++ b/src/libsyntax/codemap.rs @@ -637,7 +637,7 @@ impl CodeMap { /// Finds the width of a character, either before or after the provided span. fn find_width_of_character_at_span(&self, sp: Span, forwards: bool) -> u32 { // Disregard malformed spans and assume a one-byte wide character. - if sp.lo() > sp.hi() { + if sp.lo() >= sp.hi() { return 1; } @@ -671,11 +671,16 @@ impl CodeMap { } else { return 1; }; + debug!("DTW start {:?} end {:?}", start_index, end_index); + debug!("DTW snippet {:?}", snippet); let mut target = if forwards { end_index + 1 } else { end_index - 1 }; + debug!("DTW initial target {:?}", target); while !snippet.is_char_boundary(target - start_index) { target = if forwards { target + 1 } else { target - 1 }; + debug!("DTW update target {:?}", target); } + debug!("DTW final target {:?}", target); if forwards { (target - end_index) as u32 From 0bd96671f0312fdc1eb07885835e58d258f1f927 Mon Sep 17 00:00:00 2001 From: David Wood Date: Sat, 27 Jan 2018 13:30:34 +0000 Subject: [PATCH 9/9] Fixed infinite loop issues and added some improved logging. --- src/libsyntax/codemap.rs | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/src/libsyntax/codemap.rs b/src/libsyntax/codemap.rs index 8c1bdab28a9d5..a6a7f9e20b3a9 100644 --- a/src/libsyntax/codemap.rs +++ b/src/libsyntax/codemap.rs @@ -638,25 +638,33 @@ impl CodeMap { fn find_width_of_character_at_span(&self, sp: Span, forwards: bool) -> u32 { // Disregard malformed spans and assume a one-byte wide character. if sp.lo() >= sp.hi() { + debug!("find_width_of_character_at_span: early return malformed span"); return 1; } let local_begin = self.lookup_byte_offset(sp.lo()); let local_end = self.lookup_byte_offset(sp.hi()); + debug!("find_width_of_character_at_span: local_begin=`{:?}`, local_end=`{:?}`", + local_begin, local_end); let start_index = local_begin.pos.to_usize(); let end_index = local_end.pos.to_usize(); + debug!("find_width_of_character_at_span: start_index=`{:?}`, end_index=`{:?}`", + start_index, end_index); // Disregard indexes that are at the start or end of their spans, they can't fit bigger // characters. if (!forwards && end_index == usize::min_value()) || (forwards && start_index == usize::max_value()) { + debug!("find_width_of_character_at_span: start or end of span, cannot be multibyte"); return 1; } let source_len = (local_begin.fm.end_pos - local_begin.fm.start_pos).to_usize(); + debug!("find_width_of_character_at_span: source_len=`{:?}`", source_len); // Ensure indexes are also not malformed. if start_index > end_index || end_index > source_len { + debug!("find_width_of_character_at_span: source indexes are malformed"); return 1; } @@ -671,16 +679,22 @@ impl CodeMap { } else { return 1; }; - debug!("DTW start {:?} end {:?}", start_index, end_index); - debug!("DTW snippet {:?}", snippet); + debug!("find_width_of_character_at_span: snippet=`{:?}`", snippet); + + let file_start_pos = local_begin.fm.start_pos.to_usize(); + let file_end_pos = local_begin.fm.end_pos.to_usize(); + debug!("find_width_of_character_at_span: file_start_pos=`{:?}` file_end_pos=`{:?}`", + file_start_pos, file_end_pos); let mut target = if forwards { end_index + 1 } else { end_index - 1 }; - debug!("DTW initial target {:?}", target); - while !snippet.is_char_boundary(target - start_index) { + debug!("find_width_of_character_at_span: initial target=`{:?}`", target); + + while !snippet.is_char_boundary(target - start_index) + && target >= file_start_pos && target <= file_end_pos { target = if forwards { target + 1 } else { target - 1 }; - debug!("DTW update target {:?}", target); + debug!("find_width_of_character_at_span: target=`{:?}`", target); } - debug!("DTW final target {:?}", target); + debug!("find_width_of_character_at_span: final target=`{:?}`", target); if forwards { (target - end_index) as u32