Skip to content

Commit

Permalink
Rollup merge of rust-lang#32756 - nikomatsakis:borrowck-snippet, r=nrc
Browse files Browse the repository at this point in the history
Overhaul borrowck error messages and compiler error formatting generally

This is a major overhaul of how the compiler reports errors. The primary goal is to be able to give many spans within the same overall context, such as this:

```
./borrow-errors.rs:73:17: 73:20: error: cannot borrow `*vec` as immutable because previous closure requires unique access [E0501]
70     let append = |e| {
                    ~~~ closure construction occurs here
71         vec.push(e)
           ~~~ previous borrow occurs due to use of `vec` in closure
72     };
73     let data = &vec[3];
                   ~~~ borrow occurs here
74 }
   ~ borrow from closure ends here
```

However, in the process we made a number of other changes:

- Removed the repetitive filenames from snippets and just give the line number.
- Color the line numbers blue so they "fade away"
- Remove the file name and line number from the error code suggestions since they don't seem to fit anymore. (This should probably happen in more places, like existing notes.)
- Newlines in between errors to help group them better.

This PR is not quite ready to land, but we thought it made sense to stop here and get some feedback from people at large. It'd be great if people can check out the branch and play with it. We'd be especially interested in hearing about cases that don't look good with the new formatting (I suspect they exist).

Here is a checklist of some pending work items for this PR. Some of them may be best left for follow-up PRs:

- [x] Accommodate multiple files in a `MultiSpan` (this should be easy)
  - In this case, we want to print filenames though.
- [x] Remove duplicate E0500 code.
- [x] Make the header message bold, rather than current hack that makes all errors/warnings bold
- [x] Update warning text color (yellow is hard to read w/ a white background)

Moved numerous follow-ups to: rust-lang#33240

Joint work with @jonathandturner.

Fixes rust-lang#3533
  • Loading branch information
Manishearth committed May 3, 2016
2 parents d80497e + 9355a91 commit 40199f6
Show file tree
Hide file tree
Showing 169 changed files with 2,956 additions and 2,201 deletions.
2 changes: 1 addition & 1 deletion src/librustc/diagnostics.rs
Expand Up @@ -1569,5 +1569,5 @@ register_diagnostics! {
E0490, // a value of type `..` is borrowed for too long
E0491, // in type `..`, reference has a longer lifetime than the data it...
E0495, // cannot infer an appropriate lifetime due to conflicting requirements
E0524, // expected a closure that implements `..` but this closure only implements `..`
E0525, // expected a closure that implements `..` but this closure only implements `..`
}
57 changes: 26 additions & 31 deletions src/librustc/infer/error_reporting.rs
Expand Up @@ -249,12 +249,12 @@ pub trait ErrorReporting<'tcx> {
terr: &TypeError<'tcx>)
-> DiagnosticBuilder<'tcx>;

fn values_str(&self, values: &ValuePairs<'tcx>) -> Option<String>;
fn values_str(&self, values: &ValuePairs<'tcx>) -> Option<(String, String)>;

fn expected_found_str<T: fmt::Display + Resolvable<'tcx> + TypeFoldable<'tcx>>(
&self,
exp_found: &ty::error::ExpectedFound<T>)
-> Option<String>;
-> Option<(String, String)>;

fn report_concrete_failure(&self,
origin: SubregionOrigin<'tcx>,
Expand Down Expand Up @@ -535,7 +535,7 @@ impl<'a, 'tcx> ErrorReporting<'tcx> for InferCtxt<'a, 'tcx> {
trace: TypeTrace<'tcx>,
terr: &TypeError<'tcx>)
-> DiagnosticBuilder<'tcx> {
let expected_found_str = match self.values_str(&trace.values) {
let (expected, found) = match self.values_str(&trace.values) {
Some(v) => v,
None => {
return self.tcx.sess.diagnostic().struct_dummy(); /* derived error */
Expand All @@ -548,18 +548,17 @@ impl<'a, 'tcx> ErrorReporting<'tcx> for InferCtxt<'a, 'tcx> {
false
};

let expected_found_str = if is_simple_error {
expected_found_str
} else {
format!("{} ({})", expected_found_str, terr)
};

let mut err = struct_span_err!(self.tcx.sess,
trace.origin.span(),
E0308,
"{}: {}",
trace.origin,
expected_found_str);
"{}",
trace.origin);

if !is_simple_error {
err = err.note_expected_found(&"type", &expected, &found);
}

err = err.span_label(trace.origin.span(), &terr);

self.check_and_note_conflicting_crates(&mut err, terr, trace.origin.span());

Expand All @@ -574,6 +573,7 @@ impl<'a, 'tcx> ErrorReporting<'tcx> for InferCtxt<'a, 'tcx> {
},
_ => ()
}

err
}

Expand Down Expand Up @@ -631,7 +631,7 @@ impl<'a, 'tcx> ErrorReporting<'tcx> for InferCtxt<'a, 'tcx> {

/// Returns a string of the form "expected `{}`, found `{}`", or None if this is a derived
/// error.
fn values_str(&self, values: &ValuePairs<'tcx>) -> Option<String> {
fn values_str(&self, values: &ValuePairs<'tcx>) -> Option<(String, String)> {
match *values {
infer::Types(ref exp_found) => self.expected_found_str(exp_found),
infer::TraitRefs(ref exp_found) => self.expected_found_str(exp_found),
Expand All @@ -642,7 +642,7 @@ impl<'a, 'tcx> ErrorReporting<'tcx> for InferCtxt<'a, 'tcx> {
fn expected_found_str<T: fmt::Display + Resolvable<'tcx> + TypeFoldable<'tcx>>(
&self,
exp_found: &ty::error::ExpectedFound<T>)
-> Option<String>
-> Option<(String, String)>
{
let expected = exp_found.expected.resolve(self);
if expected.references_error() {
Expand All @@ -654,9 +654,7 @@ impl<'a, 'tcx> ErrorReporting<'tcx> for InferCtxt<'a, 'tcx> {
return None;
}

Some(format!("expected `{}`, found `{}`",
expected,
found))
Some((format!("{}", expected), format!("{}", found)))
}

fn report_generic_bound_failure(&self,
Expand Down Expand Up @@ -684,10 +682,9 @@ impl<'a, 'tcx> ErrorReporting<'tcx> for InferCtxt<'a, 'tcx> {
E0309,
"{} may not live long enough",
labeled_user_string);
err.fileline_help(origin.span(),
&format!("consider adding an explicit lifetime bound `{}: {}`...",
bound_kind,
sub));
err.help(&format!("consider adding an explicit lifetime bound `{}: {}`...",
bound_kind,
sub));
err
}

Expand All @@ -698,10 +695,9 @@ impl<'a, 'tcx> ErrorReporting<'tcx> for InferCtxt<'a, 'tcx> {
E0310,
"{} may not live long enough",
labeled_user_string);
err.fileline_help(origin.span(),
&format!("consider adding an explicit lifetime \
bound `{}: 'static`...",
bound_kind));
err.help(&format!("consider adding an explicit lifetime \
bound `{}: 'static`...",
bound_kind));
err
}

Expand All @@ -712,9 +708,8 @@ impl<'a, 'tcx> ErrorReporting<'tcx> for InferCtxt<'a, 'tcx> {
E0311,
"{} may not live long enough",
labeled_user_string);
err.fileline_help(origin.span(),
&format!("consider adding an explicit lifetime bound for `{}`",
bound_kind));
err.help(&format!("consider adding an explicit lifetime bound for `{}`",
bound_kind));
self.tcx.note_and_explain_region(
&mut err,
&format!("{} must be valid for ", labeled_user_string),
Expand Down Expand Up @@ -1751,11 +1746,11 @@ impl<'a, 'tcx> ErrorReportingHelpers<'tcx> for InferCtxt<'a, 'tcx> {
};

match self.values_str(&trace.values) {
Some(values_str) => {
Some((expected, found)) => {
err.span_note(
trace.origin.span(),
&format!("...so that {} ({})",
desc, values_str));
&format!("...so that {} (expected {}, found {})",
desc, expected, found));
}
None => {
// Really should avoid printing this error at
Expand Down
14 changes: 5 additions & 9 deletions src/librustc/lint/context.rs
Expand Up @@ -456,17 +456,13 @@ pub fn raw_struct_lint<'a>(sess: &'a Session,
it will become a hard error in a future release!");
let citation = format!("for more information, see {}",
future_incompatible.reference);
if let Some(sp) = span {
err.fileline_warn(sp, &explanation);
err.fileline_note(sp, &citation);
} else {
err.warn(&explanation);
err.note(&citation);
}
err.warn(&explanation);
err.note(&citation);
}

if let Some(span) = def {
err.span_note(span, "lint level defined here");
let explanation = "lint level defined here";
err.span_note(span, &explanation);
}

err
Expand Down Expand Up @@ -542,7 +538,7 @@ pub trait LintContext: Sized {
let mut err = self.lookup(lint, Some(span), msg);
if self.current_level(lint) != Level::Allow {
if note_span == span {
err.fileline_note(note_span, note);
err.note(note);
} else {
err.span_note(note_span, note);
}
Expand Down
4 changes: 2 additions & 2 deletions src/librustc/session/mod.rs
Expand Up @@ -567,7 +567,7 @@ pub fn early_error(output: config::ErrorOutputType, msg: &str) -> ! {
}
config::ErrorOutputType::Json => Box::new(JsonEmitter::basic()),
};
emitter.emit(None, msg, None, errors::Level::Fatal);
emitter.emit(&MultiSpan::new(), msg, None, errors::Level::Fatal);
panic!(errors::FatalError);
}

Expand All @@ -578,7 +578,7 @@ pub fn early_warn(output: config::ErrorOutputType, msg: &str) {
}
config::ErrorOutputType::Json => Box::new(JsonEmitter::basic()),
};
emitter.emit(None, msg, None, errors::Level::Warning);
emitter.emit(&MultiSpan::new(), msg, None, errors::Level::Warning);
}

// Err(0) means compilation was stopped, but no errors were found.
Expand Down

0 comments on commit 40199f6

Please sign in to comment.