Skip to content

Commit 1e08988

Browse files
ManishearthManishearth
committed
Rollup merge of rust-lang#24242 - nikomatsakis:escaping-closure-error-message, r=brson
 Example showing sample inputs, old message, new message: https://gist.github.com/nikomatsakis/11126784ac678b7eb6ba Also adds infrastructure for reporting suggestions \"in situ\" and does some (minor) cleanups to `CodeMap`. r? @brson
2 parents dac858d + e313b33 commit 1e08988

File tree

11 files changed

+370
-84
lines changed

11 files changed

+370
-84
lines changed

‎src/librustc/session/mod.rs‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,13 @@ impl Session {
143143
pub fn span_end_note(&self, sp: Span, msg: &str) {
144144
self.diagnostic().span_end_note(sp, msg)
145145
}
146+
147+
/// Prints out a message with a suggested edit of the code.
148+
///
149+
/// See `diagnostic::RenderSpan::Suggestion` for more information.
150+
pub fn span_suggestion(&self, sp: Span, msg: &str, suggestion: String) {
151+
self.diagnostic().span_suggestion(sp, msg, suggestion)
152+
}
146153
pub fn span_help(&self, sp: Span, msg: &str) {
147154
self.diagnostic().span_help(sp, msg)
148155
}

‎src/librustc_borrowck/borrowck/mod.rs‎

Lines changed: 53 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -522,6 +522,16 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> {
522522
}
523523

524524
pub fn report(&self, err: BckError<'tcx>) {
525+
// Catch and handle some particular cases.
526+
match (&err.code, &err.cause) {
527+
(&err_out_of_scope(ty::ReScope(_), ty::ReStatic), &euv::ClosureCapture(span)) |
528+
(&err_out_of_scope(ty::ReScope(_), ty::ReFree(..)), &euv::ClosureCapture(span)) => {
529+
return self.report_out_of_scope_escaping_closure_capture(&err, span);
530+
}
531+
_ => { }
532+
}
533+
534+
// General fallback.
525535
self.span_err(
526536
err.span,
527537
&self.bckerr_to_string(&err));
@@ -796,16 +806,10 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> {
796806
format!("{} does not live long enough", msg)
797807
}
798808
err_borrowed_pointer_too_short(..) => {
799-
let descr = match opt_loan_path(&err.cmt) {
800-
Some(lp) => {
801-
format!("`{}`", self.loan_path_to_string(&*lp))
802-
}
803-
None => self.cmt_to_string(&*err.cmt),
804-
};
805-
809+
let descr = self.cmt_to_path_or_string(&err.cmt);
806810
format!("lifetime of {} is too short to guarantee \
807-
its contents can be safely reborrowed",
808-
descr)
811+
its contents can be safely reborrowed",
812+
descr)
809813
}
810814
}
811815
}
@@ -888,6 +892,39 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> {
888892
}
889893
}
890894

895+
fn report_out_of_scope_escaping_closure_capture(&self,
896+
err: &BckError<'tcx>,
897+
capture_span: Span)
898+
{
899+
let cmt_path_or_string = self.cmt_to_path_or_string(&err.cmt);
900+
901+
span_err!(
902+
self.tcx.sess, err.span, E0373,
903+
"closure may outlive the current function, \
904+
but it borrows {}, \
905+
which is owned by the current function",
906+
cmt_path_or_string);
907+
908+
self.tcx.sess.span_note(
909+
capture_span,
910+
&format!("{} is borrowed here",
911+
cmt_path_or_string));
912+
913+
let suggestion =
914+
match self.tcx.sess.codemap().span_to_snippet(err.span) {
915+
Ok(string) => format!("move {}", string),
916+
Err(_) => format!("move |<args>| <body>")
917+
};
918+
919+
self.tcx.sess.span_suggestion(
920+
err.span,
921+
&format!("to force the closure to take ownership of {} \
922+
(and any other referenced variables), \
923+
use the `move` keyword, as shown:",
924+
cmt_path_or_string),
925+
suggestion);
926+
}
927+
891928
pub fn note_and_explain_bckerr(&self, err: BckError<'tcx>) {
892929
let code = err.code;
893930
match code {
@@ -1035,6 +1072,13 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> {
10351072
pub fn cmt_to_string(&self, cmt: &mc::cmt_<'tcx>) -> String {
10361073
cmt.descriptive_string(self.tcx)
10371074
}
1075+
1076+
pub fn cmt_to_path_or_string(&self, cmt: &mc::cmt<'tcx>) -> String {
1077+
match opt_loan_path(cmt) {
1078+
Some(lp) => format!("`{}`", self.loan_path_to_string(&lp)),
1079+
None => self.cmt_to_string(cmt),
1080+
}
1081+
}
10381082
}
10391083

10401084
fn is_statement_scope(tcx: &ty::ctxt, region: ty::Region) -> bool {

‎src/librustc_borrowck/diagnostics.rs‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
// Copyright 2014 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
#![allow(non_snake_case)]
12+
13+
register_diagnostics! {
14+
E0373 // closure may outlive current fn, but it borrows {}, which is owned by current fn
15+
}
16+
17+
__build_diagnostic_array! { DIAGNOSTICS }

‎src/librustc_borrowck/lib.rs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,10 @@ pub use borrowck::check_crate;
4040
pub use borrowck::build_borrowck_dataflow_data_for_fn;
4141
pub use borrowck::FnPartsWithCFG;
4242

43+
// NB: This module needs to be declared first so diagnostics are
44+
// registered before they are used.
45+
pub mod diagnostics;
46+
4347
mod borrowck;
4448

4549
pub mod graphviz;

‎src/libsyntax/codemap.rs‎

Lines changed: 92 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ pub struct BytePos(pub u32);
4949
/// A character offset. Because of multibyte utf8 characters, a byte offset
5050
/// is not equivalent to a character offset. The CodeMap will convert BytePos
5151
/// values to CharPos values as necessary.
52-
#[derive(Copy, Clone, PartialEq, Hash, PartialOrd, Debug)]
52+
#[derive(Copy, Clone, PartialEq, Eq, Hash, PartialOrd, Debug)]
5353
pub struct CharPos(pub usize);
5454

5555
// FIXME: Lots of boilerplate in these impls, but so far my attempts to fix
@@ -305,9 +305,21 @@ impl ExpnId {
305305

306306
pub type FileName = String;
307307

308+
#[derive(Copy, Clone, Debug, PartialEq, Eq)]
309+
pub struct LineInfo {
310+
/// Index of line, starting from 0.
311+
pub line_index: usize,
312+
313+
/// Column in line where span begins, starting from 0.
314+
pub start_col: CharPos,
315+
316+
/// Column in line where span ends, starting from 0, exclusive.
317+
pub end_col: CharPos,
318+
}
319+
308320
pub struct FileLines {
309321
pub file: Rc<FileMap>,
310-
pub lines: Vec<usize>
322+
pub lines: Vec<LineInfo>
311323
}
312324

313325
/// Identifies an offset of a multi-byte character in a FileMap
@@ -479,9 +491,9 @@ impl FileMap {
479491
lines.push(pos);
480492
}
481493

482-
/// get a line from the list of pre-computed line-beginnings
483-
///
484-
pub fn get_line(&self, line_number: usize) -> Option<String> {
494+
/// get a line from the list of pre-computed line-beginnings.
495+
/// line-number here is 0-based.
496+
pub fn get_line(&self, line_number: usize) -> Option<&str> {
485497
match self.src {
486498
Some(ref src) => {
487499
let lines = self.lines.borrow();
@@ -492,7 +504,7 @@ impl FileMap {
492504
match slice.find('\n') {
493505
Some(e) => &slice[..e],
494506
None => slice
495-
}.to_string()
507+
}
496508
})
497509
}
498510
None => None
@@ -661,10 +673,29 @@ impl CodeMap {
661673
pub fn span_to_lines(&self, sp: Span) -> FileLines {
662674
let lo = self.lookup_char_pos(sp.lo);
663675
let hi = self.lookup_char_pos(sp.hi);
664-
let mut lines = Vec::new();
665-
for i in lo.line - 1..hi.line {
666-
lines.push(i);
667-
};
676+
let mut lines = Vec::with_capacity(hi.line - lo.line + 1);
677+
678+
// The span starts partway through the first line,
679+
// but after that it starts from offset 0.
680+
let mut start_col = lo.col;
681+
682+
// For every line but the last, it extends from `start_col`
683+
// and to the end of the line. Be careful because the line
684+
// numbers in Loc are 1-based, so we subtract 1 to get 0-based
685+
// lines.
686+
for line_index in lo.line-1 .. hi.line-1 {
687+
let line_len = lo.file.get_line(line_index).map(|s| s.len()).unwrap_or(0);
688+
lines.push(LineInfo { line_index: line_index,
689+
start_col: start_col,
690+
end_col: CharPos::from_usize(line_len) });
691+
start_col = CharPos::from_usize(0);
692+
}
693+
694+
// For the last line, it extends from `start_col` to `hi.col`:
695+
lines.push(LineInfo { line_index: hi.line - 1,
696+
start_col: start_col,
697+
end_col: hi.col });
698+
668699
FileLines {file: lo.file, lines: lines}
669700
}
670701

@@ -919,17 +950,18 @@ pub struct MalformedCodemapPositions {
919950
#[cfg(test)]
920951
mod test {
921952
use super::*;
953+
use std::rc::Rc;
922954

923955
#[test]
924956
fn t1 () {
925957
let cm = CodeMap::new();
926958
let fm = cm.new_filemap("blork.rs".to_string(),
927959
"first line.\nsecond line".to_string());
928960
fm.next_line(BytePos(0));
929-
assert_eq!(fm.get_line(0), Some("first line.".to_string()));
961+
assert_eq!(fm.get_line(0), Some("first line."));
930962
// TESTING BROKEN BEHAVIOR:
931963
fm.next_line(BytePos(10));
932-
assert_eq!(fm.get_line(1), Some(".".to_string()));
964+
assert_eq!(fm.get_line(1), Some("."));
933965
}
934966

935967
#[test]
@@ -1057,7 +1089,54 @@ mod test {
10571089

10581090
assert_eq!(file_lines.file.name, "blork.rs");
10591091
assert_eq!(file_lines.lines.len(), 1);
1060-
assert_eq!(file_lines.lines[0], 1);
1092+
assert_eq!(file_lines.lines[0].line_index, 1);
1093+
}
1094+
1095+
/// Given a string like " ^~~~~~~~~~~~ ", produces a span
1096+
/// coverting that range. The idea is that the string has the same
1097+
/// length as the input, and we uncover the byte positions. Note
1098+
/// that this can span lines and so on.
1099+
fn span_from_selection(input: &str, selection: &str) -> Span {
1100+
assert_eq!(input.len(), selection.len());
1101+
let left_index = selection.find('^').unwrap() as u32;
1102+
let right_index = selection.rfind('~').unwrap() as u32;
1103+
Span { lo: BytePos(left_index), hi: BytePos(right_index + 1), expn_id: NO_EXPANSION }
1104+
}
1105+
1106+
fn new_filemap_and_lines(cm: &CodeMap, filename: &str, input: &str) -> Rc<FileMap> {
1107+
let fm = cm.new_filemap(filename.to_string(), input.to_string());
1108+
let mut byte_pos: u32 = 0;
1109+
for line in input.lines() {
1110+
// register the start of this line
1111+
fm.next_line(BytePos(byte_pos));
1112+
1113+
// update byte_pos to include this line and the \n at the end
1114+
byte_pos += line.len() as u32 + 1;
1115+
}
1116+
fm
1117+
}
1118+
1119+
/// Test span_to_snippet and span_to_lines for a span coverting 3
1120+
/// lines in the middle of a file.
1121+
#[test]
1122+
fn span_to_snippet_and_lines_spanning_multiple_lines() {
1123+
let cm = CodeMap::new();
1124+
let inputtext = "aaaaa\nbbbbBB\nCCC\nDDDDDddddd\neee\n";
1125+
let selection = " \n ^~\n~~~\n~~~~~ \n \n";
1126+
new_filemap_and_lines(&cm, "blork.rs", inputtext);
1127+
let span = span_from_selection(inputtext, selection);
1128+
1129+
// check that we are extracting the text we thought we were extracting
1130+
assert_eq!(&cm.span_to_snippet(span).unwrap(), "BB\nCCC\nDDDDD");
1131+
1132+
// check that span_to_lines gives us the complete result with the lines/cols we expected
1133+
let lines = cm.span_to_lines(span);
1134+
let expected = vec![
1135+
LineInfo { line_index: 1, start_col: CharPos(4), end_col: CharPos(6) },
1136+
LineInfo { line_index: 2, start_col: CharPos(0), end_col: CharPos(3) },
1137+
LineInfo { line_index: 3, start_col: CharPos(0), end_col: CharPos(5) }
1138+
];
1139+
assert_eq!(lines.lines, expected);
10611140
}
10621141

10631142
#[test]

0 commit comments

Comments
 (0)