Skip to content

Batch of improvements to errors for new error format #33619

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
May 16, 2016
Merged

Batch of improvements to errors for new error format #33619

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f3054ce
Set of fixes to improve borrowcks that weren't updated
May 9, 2016
104fe1c
Updating a few more errors with labels
May 11, 2016
1b6afd1
Update errors to use new error format
May 12, 2016
ecadd7e
Update span_label call to new api
May 13, 2016
6793cac
Address nit
May 13, 2016
65cb5f7
Add space after equals
May 15, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Update errors to use new error format
  • Loading branch information
Jonathan Turner committed May 12, 2016
commit 1b6afd1e42ba2a809c2b7255ed61784d9901555e
4 changes: 2 additions & 2 deletions src/librustc/infer/error_reporting.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -482,10 +482,10 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
trace.origin);

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

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

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

Expand Down
141 changes: 74 additions & 67 deletions src/librustc_borrowck/borrowck/check_loans.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -475,99 +475,104 @@ impl<'a, 'tcx> CheckLoanCtxt<'a, 'tcx> {

let mut err = match (new_loan.kind, old_loan.kind) {
(ty::MutBorrow, ty::MutBorrow) => {
struct_span_err!(self.bccx, new_loan.span, E0499,
"cannot borrow `{}`{} as mutable \
more than once at a time",
nl, new_loan_msg)
.span_label(
let mut err =struct_span_err!(self.bccx, new_loan.span, E0499,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

space after =

"cannot borrow `{}`{} as mutable \
more than once at a time",
nl, new_loan_msg);
err.span_label(
old_loan.span,
&format!("first mutable borrow occurs here{}", old_loan_msg))
.span_label(
&format!("first mutable borrow occurs here{}", old_loan_msg));
err.span_label(
new_loan.span,
&format!("second mutable borrow occurs here{}", new_loan_msg))
.span_label(
&format!("second mutable borrow occurs here{}", new_loan_msg));
err.span_label(
previous_end_span,
&format!("first borrow ends here"))
&format!("first borrow ends here"));
err
}

(ty::UniqueImmBorrow, ty::UniqueImmBorrow) => {
struct_span_err!(self.bccx, new_loan.span, E0524,
let mut err = struct_span_err!(self.bccx, new_loan.span, E0524,
"two closures require unique access to `{}` \
at the same time",
nl)
.span_label(
nl);
err.span_label(
old_loan.span,
&format!("first closure is constructed here"))
.span_label(
&format!("first closure is constructed here"));
err.span_label(
new_loan.span,
&format!("second closure is constructed here"))
.span_label(
&format!("second closure is constructed here"));
err.span_label(
previous_end_span,
&format!("borrow from first closure ends here"))
&format!("borrow from first closure ends here"));
err
}

(ty::UniqueImmBorrow, _) => {
struct_span_err!(self.bccx, new_loan.span, E0500,
"closure requires unique access to `{}` \
but {} is already borrowed{}",
nl, ol_pronoun, old_loan_msg)
.span_label(
let mut err = struct_span_err!(self.bccx, new_loan.span, E0500,
"closure requires unique access to `{}` \
but {} is already borrowed{}",
nl, ol_pronoun, old_loan_msg);
err.span_label(
new_loan.span,
&format!("closure construction occurs here{}", new_loan_msg))
.span_label(
&format!("closure construction occurs here{}", new_loan_msg));
err.span_label(
old_loan.span,
&format!("borrow occurs here{}", old_loan_msg))
.span_label(
&format!("borrow occurs here{}", old_loan_msg));
err.span_label(
previous_end_span,
&format!("borrow ends here"))
&format!("borrow ends here"));
err
}

(_, ty::UniqueImmBorrow) => {
struct_span_err!(self.bccx, new_loan.span, E0501,
"cannot borrow `{}`{} as {} because \
previous closure requires unique access",
nl, new_loan_msg, new_loan.kind.to_user_str())
.span_label(
let mut err = struct_span_err!(self.bccx, new_loan.span, E0501,
"cannot borrow `{}`{} as {} because \
previous closure requires unique access",
nl, new_loan_msg, new_loan.kind.to_user_str());
err.span_label(
new_loan.span,
&format!("borrow occurs here{}", new_loan_msg))
.span_label(
&format!("borrow occurs here{}", new_loan_msg));
err.span_label(
old_loan.span,
&format!("closure construction occurs here{}", old_loan_msg))
.span_label(
&format!("closure construction occurs here{}", old_loan_msg));
err.span_label(
previous_end_span,
&format!("borrow from closure ends here"))
&format!("borrow from closure ends here"));
err
}

(_, _) => {
struct_span_err!(self.bccx, new_loan.span, E0502,
"cannot borrow `{}`{} as {} because \
{} is also borrowed as {}{}",
nl,
new_loan_msg,
new_loan.kind.to_user_str(),
ol_pronoun,
old_loan.kind.to_user_str(),
old_loan_msg)
.span_label(
let mut err = struct_span_err!(self.bccx, new_loan.span, E0502,
"cannot borrow `{}`{} as {} because \
{} is also borrowed as {}{}",
nl,
new_loan_msg,
new_loan.kind.to_user_str(),
ol_pronoun,
old_loan.kind.to_user_str(),
old_loan_msg);
err.span_label(
new_loan.span,
&format!("{} borrow occurs here{}",
new_loan.kind.to_user_str(),
new_loan_msg))
.span_label(
new_loan_msg));
err.span_label(
old_loan.span,
&format!("{} borrow occurs here{}",
old_loan.kind.to_user_str(),
old_loan_msg))
.span_label(
old_loan_msg));
err.span_label(
previous_end_span,
&format!("{} borrow ends here",
old_loan.kind.to_user_str()))
old_loan.kind.to_user_str()));
err
}
};

match new_loan.cause {
euv::ClosureCapture(span) => {
err = err.span_label(
err.span_label(
span,
&format!("borrow occurs due to use of `{}` in closure", nl));
}
Expand All @@ -576,7 +581,7 @@ impl<'a, 'tcx> CheckLoanCtxt<'a, 'tcx> {

match old_loan.cause {
euv::ClosureCapture(span) => {
err = err.span_label(
err.span_label(
span,
&format!("previous borrow occurs due to use of `{}` in closure",
ol));
Expand Down Expand Up @@ -664,35 +669,37 @@ impl<'a, 'tcx> CheckLoanCtxt<'a, 'tcx> {
UseWhileBorrowed(loan_path, loan_span) => {
let mut err = match move_kind {
move_data::Captured => {
struct_span_err!(self.bccx, span, E0504,
let mut err = struct_span_err!(self.bccx, span, E0504,
"cannot move `{}` into closure because it is borrowed",
&self.bccx.loan_path_to_string(move_path))
.span_label(
&self.bccx.loan_path_to_string(move_path));
err.span_label(
loan_span,
&format!("borrow of `{}` occurs here",
&self.bccx.loan_path_to_string(&loan_path))
)
.span_label(
);
err.span_label(
span,
&format!("move into closure occurs here")
)
);
err
}
move_data::Declared |
move_data::MoveExpr |
move_data::MovePat => {
struct_span_err!(self.bccx, span, E0505,
let mut err = struct_span_err!(self.bccx, span, E0505,
"cannot move out of `{}` because it is borrowed",
&self.bccx.loan_path_to_string(move_path))
.span_label(
&self.bccx.loan_path_to_string(move_path));
err.span_label(
loan_span,
&format!("borrow of `{}` occurs here",
&self.bccx.loan_path_to_string(&loan_path))
)
.span_label(
);
err.span_label(
span,
&format!("move out of `{}` occurs here",
&self.bccx.loan_path_to_string(move_path))
)
);
err
}
};

Expand Down
31 changes: 18 additions & 13 deletions src/librustc_borrowck/borrowck/gather_loans/move_error.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,22 +121,25 @@ fn report_cannot_move_out_of<'a, 'tcx>(bccx: &BorrowckCtxt<'a, 'tcx>,
Categorization::Deref(_, _, mc::Implicit(..)) |
Categorization::Deref(_, _, mc::UnsafePtr(..)) |
Categorization::StaticItem => {
struct_span_err!(bccx, move_from.span, E0507,
let mut err = struct_span_err!(bccx, move_from.span, E0507,
"cannot move out of {}",
move_from.descriptive_string(bccx.tcx))
.span_label(
move_from.descriptive_string(bccx.tcx));
err.span_label(
move_from.span,
&format!("move occurs here")
)
);
err
}

Categorization::Interior(ref b, mc::InteriorElement(Kind::Index, _)) => {
let expr = bccx.tcx.map.expect_expr(move_from.id);
if let hir::ExprIndex(..) = expr.node {
struct_span_err!(bccx, move_from.span, E0508,
"cannot move out of type `{}`, \
a non-copy fixed-size array",
b.ty)
let mut err = struct_span_err!(bccx, move_from.span, E0508,
"cannot move out of type `{}`, \
a non-copy fixed-size array",
b.ty);
err.span_label(move_from.span, &format!("can not move out of here"));
err
} else {
span_bug!(move_from.span, "this path should not cause illegal move");
}
Expand All @@ -147,10 +150,12 @@ fn report_cannot_move_out_of<'a, 'tcx>(bccx: &BorrowckCtxt<'a, 'tcx>,
match b.ty.sty {
ty::TyStruct(def, _) |
ty::TyEnum(def, _) if def.has_dtor() => {
struct_span_err!(bccx, move_from.span, E0509,
"cannot move out of type `{}`, \
which defines the `Drop` trait",
b.ty)
let mut err = struct_span_err!(bccx, move_from.span, E0509,
"cannot move out of type `{}`, \
which defines the `Drop` trait",
b.ty);
err.span_label(move_from.span, &format!("can not move out of here"));
err
},
_ => {
span_bug!(move_from.span, "this path should not cause illegal move");
Expand All @@ -168,7 +173,7 @@ fn note_move_destination(mut err: DiagnosticBuilder,
pat_name: ast::Name,
is_first_note: bool) -> DiagnosticBuilder {
if is_first_note {
err = err.span_label(
err.span_label(
move_to_span,
&format!("attempting to move value to here"));
err.help(
Expand Down
23 changes: 12 additions & 11 deletions src/librustc_borrowck/borrowck/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -624,9 +624,8 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> {
let mut db = self.struct_span_err(
err.span,
&self.bckerr_to_string(&err));
self.note_and_explain_bckerr(&mut db, err);
db.span_label(span, &format!("cannot borrow"))
.emit();
self.note_and_explain_bckerr(&mut db, err, span);
db.emit();
}

pub fn report_use_of_moved_value(&self,
Expand Down Expand Up @@ -721,10 +720,12 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> {
err.span_label(
use_span,
&format!("value moved{} here in previous iteration of loop",
move_note))
move_note));
err
} else {
err.span_label(use_span, &format!("value {} here after move", verb_participle))
.span_label(move_span, &format!("value moved{} here", move_note))
.span_label(move_span, &format!("value moved{} here", move_note));
err
};

err.note(&format!("move occurs because `{}` has type `{}`, \
Expand Down Expand Up @@ -951,7 +952,8 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> {
.emit();
}

pub fn note_and_explain_bckerr(&self, db: &mut DiagnosticBuilder, err: BckError<'tcx>) {
pub fn note_and_explain_bckerr(&self, db: &mut DiagnosticBuilder, err: BckError<'tcx>,
error_span: Span) {
let code = err.code;
match code {
err_mutbl => {
Expand All @@ -976,13 +978,11 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> {
let span = self.tcx.map.span(local_id);
if let Ok(snippet) = self.tcx.sess.codemap().span_to_snippet(span) {
if snippet != "self" {
db.span_suggestion(
span,
&format!("to make the {} mutable, use `mut` as shown:",
self.cmt_to_string(&err.cmt)),
format!("mut {}", snippet));
db.span_label(span,
&format!("use `mut {}` here to make mutable", snippet));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This replaced span_suggestion with span_label, which is undesirable. See #33691.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should chat about lint implications. For the actual error, moving it to a label text appears to improve readability "at a glance" of the errors. That said, we may want to distinguish them in some way that lints and autocorrection type tools can find and use them.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think span_suggestion should be used so that machines can read them, but it should be rendered like label for humans.

}
}
db.span_label(error_span, &format!("cannot borrow mutably"));
}
}
}
Expand All @@ -1000,6 +1000,7 @@ impl<'a, 'tcx> BorrowckCtxt<'a, 'tcx> {
super_scope,
"");
if let Some(span) = statement_scope_span(self.tcx, super_scope) {
db.span_label(error_span, &format!("does not live long enough"));
db.span_help(span,
"consider using a `let` binding to increase its lifetime");
}
Expand Down
Loading