Auto merge of #47420 - davidtwco:issue-46885, r=estebank

Fix off-by-one spans in MIR borrowck errors

Fixes #46885.

r? @nikomatsakis

Author: bors

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
                             };

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) =>

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<Span> {
         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
         }

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(),
             )
         }

src/librustc_mir/build/scope.rs

@@ -695,10 +695,12 @@ 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.
+                let scope_end = self.hir.tcx().sess.codemap().end_point(region_scope_span);
+
                 scope.drops.push(DropData {
                     span: scope_end,
                     location: place.clone(),

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))
                 }
             }
         }

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;

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());

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,101 @@ 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 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 start_of_next_point = sp.hi().0;
+
+        let width = self.find_width_of_character_at_span(sp, true);
+        // 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.
+    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;
+        }
+
+        // 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;
+        };
+        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!("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!("find_width_of_character_at_span: target=`{:?}`", target);
+        }
+        debug!("find_width_of_character_at_span: final target=`{:?}`", target);
+
+        if forwards {
+            (target - end_index) as u32
+        } else {
+            (end_index - target) as u32
+        }
+    }
+
     pub fn get_filemap(&self, filename: &FileName) -> Option<Rc<FileMap>> {
         for fm in self.files.borrow().iter() {
             if *filename == fm.name {

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");

src/libsyntax_pos/lib.rs

@@ -216,20 +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();
-        let lo = cmp::max(span.hi.0 - 1, 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 }

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

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...
 

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

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

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

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

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

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
 

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
 

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
 

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
 

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
 

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...