Skip to content

Rollup of 4 pull requests #61309

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

Closed
wants to merge 12 commits into from
+286 −211
Closed

Rollup of 4 pull requests #61309

Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
4890ae2
Do not ICE on missing access place description during mutability erro…
estebank May 25, 2019
285064c
Probe for CompilerDesugaringKind::Async argument
estebank May 26, 2019
025a559
Move tests to async-await directory
estebank May 26, 2019
24b2e20
Account for short-hand init structs when suggesting conversion
estebank May 26, 2019
96e3fb2
librustc_errors: Move annotation collection to own impl
phansch May 25, 2019
1b86bd7
is_union returns ty to avoid computing it twice
spastorino May 28, 2019
bb94fc0
Use closure to avoid self.describe_place(...).unwrap_or_else(...) rep…
spastorino May 28, 2019
d72f97d
review comment: tweak error wording
estebank May 28, 2019
73b767b
Rollup merge of #61191 - phansch:annotate_snippet_refactorings1, r=es…
Centril May 29, 2019
d6303ef
Rollup merge of #61192 - estebank:issue-61187, r=oli-obk
Centril May 29, 2019
cf42d11
Rollup merge of #61217 - estebank:issue-52820, r=Centril
Centril May 29, 2019
60c8d20
Rollup merge of #61261 - spastorino:is-union-return-ty, r=oli-obk
Centril May 29, 2019
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
341 changes: 174 additions & 167 deletions src/librustc_errors/emitter.rs
View file
Original file line number Diff line number Diff line change
@@ -162,6 +162,7 @@ impl ColorConfig {
}
}

/// Handles the writing of `HumanReadableErrorType::Default` and `HumanReadableErrorType::Short`
pub struct EmitterWriter {
dst: Destination,
sm: Option<Lrc<SourceMapperDyn>>,
@@ -170,7 +171,8 @@ pub struct EmitterWriter {
ui_testing: bool,
}

struct FileWithAnnotatedLines {
#[derive(Debug)]
pub struct FileWithAnnotatedLines {
file: Lrc<SourceFile>,
lines: Vec<Line>,
multiline_depth: usize,
@@ -221,169 +223,6 @@ impl EmitterWriter {
}
}

fn preprocess_annotations(&mut self, msp: &MultiSpan) -> Vec<FileWithAnnotatedLines> {
fn add_annotation_to_file(file_vec: &mut Vec<FileWithAnnotatedLines>,
file: Lrc<SourceFile>,
line_index: usize,
ann: Annotation) {

for slot in file_vec.iter_mut() {
// Look through each of our files for the one we're adding to
if slot.file.name == file.name {
// See if we already have a line for it
for line_slot in &mut slot.lines {
if line_slot.line_index == line_index {
line_slot.annotations.push(ann);
return;
}
}
// We don't have a line yet, create one
slot.lines.push(Line {
line_index,
annotations: vec![ann],
});
slot.lines.sort();
return;
}
}
// This is the first time we're seeing the file
file_vec.push(FileWithAnnotatedLines {
file,
lines: vec![Line {
line_index,
annotations: vec![ann],
}],
multiline_depth: 0,
});
}

let mut output = vec![];
let mut multiline_annotations = vec![];

if let Some(ref sm) = self.sm {
for span_label in msp.span_labels() {
if span_label.span.is_dummy() {
continue;
}

let lo = sm.lookup_char_pos(span_label.span.lo());
let mut hi = sm.lookup_char_pos(span_label.span.hi());

// Watch out for "empty spans". If we get a span like 6..6, we
// want to just display a `^` at 6, so convert that to
// 6..7. This is degenerate input, but it's best to degrade
// gracefully -- and the parser likes to supply a span like
// that for EOF, in particular.

if lo.col_display == hi.col_display && lo.line == hi.line {
hi.col_display += 1;
}

let ann_type = if lo.line != hi.line {
let ml = MultilineAnnotation {
depth: 1,
line_start: lo.line,
line_end: hi.line,
start_col: lo.col_display,
end_col: hi.col_display,
is_primary: span_label.is_primary,
label: span_label.label.clone(),
overlaps_exactly: false,
};
multiline_annotations.push((lo.file.clone(), ml.clone()));
AnnotationType::Multiline(ml)
} else {
AnnotationType::Singleline
};
let ann = Annotation {
start_col: lo.col_display,
end_col: hi.col_display,
is_primary: span_label.is_primary,
label: span_label.label.clone(),
annotation_type: ann_type,
};

if !ann.is_multiline() {
add_annotation_to_file(&mut output, lo.file, lo.line, ann);
}
}
}

// Find overlapping multiline annotations, put them at different depths
multiline_annotations.sort_by_key(|&(_, ref ml)| (ml.line_start, ml.line_end));
for item in multiline_annotations.clone() {
let ann = item.1;
for item in multiline_annotations.iter_mut() {
let ref mut a = item.1;
// Move all other multiline annotations overlapping with this one
// one level to the right.
if !(ann.same_span(a)) &&
num_overlap(ann.line_start, ann.line_end, a.line_start, a.line_end, true)
{
a.increase_depth();
} else if ann.same_span(a) && &ann != a {
a.overlaps_exactly = true;
} else {
break;
}
}
}

let mut max_depth = 0; // max overlapping multiline spans
for (file, ann) in multiline_annotations {
if ann.depth > max_depth {
max_depth = ann.depth;
}
let mut end_ann = ann.as_end();
if !ann.overlaps_exactly {
// avoid output like
//
// | foo(
// | _____^
// | |_____|
// | || bar,
// | || );
// | || ^
// | ||______|
// | |______foo
// | baz
//
// and instead get
//
// | foo(
// | _____^
// | | bar,
// | | );
// | | ^
// | | |
// | |______foo
// | baz
add_annotation_to_file(&mut output, file.clone(), ann.line_start, ann.as_start());
// 4 is the minimum vertical length of a multiline span when presented: two lines
// of code and two lines of underline. This is not true for the special case where
// the beginning doesn't have an underline, but the current logic seems to be
// working correctly.
let middle = min(ann.line_start + 4, ann.line_end);
for line in ann.line_start + 1..middle {
// Every `|` that joins the beginning of the span (`___^`) to the end (`|__^`).
add_annotation_to_file(&mut output, file.clone(), line, ann.as_line());
}
if middle < ann.line_end - 1 {
for line in ann.line_end - 1..ann.line_end {
add_annotation_to_file(&mut output, file.clone(), line, ann.as_line());
}
}
} else {
end_ann.annotation_type = AnnotationType::Singleline;
}
add_annotation_to_file(&mut output, file, ann.line_end, end_ann);
}
for file_vec in output.iter_mut() {
file_vec.multiline_depth = max_depth;
}
output
}

fn render_source_line(&self,
buffer: &mut StyledBuffer,
file: Lrc<SourceFile>,
@@ -1093,9 +932,7 @@ impl EmitterWriter {
}
}

// Preprocess all the annotations so that they are grouped by file and by line number
// This helps us quickly iterate over the whole message (including secondary file spans)
let mut annotated_files = self.preprocess_annotations(msp);
let mut annotated_files = FileWithAnnotatedLines::collect_annotations(msp, &self.sm);

// Make sure our primary file comes first
let (primary_lo, sm) = if let (Some(sm), Some(ref primary_span)) =
@@ -1503,6 +1340,176 @@ impl EmitterWriter {
}
}

impl FileWithAnnotatedLines {
/// Preprocess all the annotations so that they are grouped by file and by line number
/// This helps us quickly iterate over the whole message (including secondary file spans)
pub fn collect_annotations(
msp: &MultiSpan,
source_map: &Option<Lrc<SourceMapperDyn>>
) -> Vec<FileWithAnnotatedLines> {
fn add_annotation_to_file(file_vec: &mut Vec<FileWithAnnotatedLines>,
file: Lrc<SourceFile>,
line_index: usize,
ann: Annotation) {

for slot in file_vec.iter_mut() {
// Look through each of our files for the one we're adding to
if slot.file.name == file.name {
// See if we already have a line for it
for line_slot in &mut slot.lines {
if line_slot.line_index == line_index {
line_slot.annotations.push(ann);
return;
}
}
// We don't have a line yet, create one
slot.lines.push(Line {
line_index,
annotations: vec![ann],
});
slot.lines.sort();
return;
}
}
// This is the first time we're seeing the file
file_vec.push(FileWithAnnotatedLines {
file,
lines: vec![Line {
line_index,
annotations: vec![ann],
}],
multiline_depth: 0,
});
}

let mut output = vec![];
let mut multiline_annotations = vec![];

if let Some(ref sm) = source_map {
for span_label in msp.span_labels() {
if span_label.span.is_dummy() {
continue;
}

let lo = sm.lookup_char_pos(span_label.span.lo());
let mut hi = sm.lookup_char_pos(span_label.span.hi());

// Watch out for "empty spans". If we get a span like 6..6, we
// want to just display a `^` at 6, so convert that to
// 6..7. This is degenerate input, but it's best to degrade
// gracefully -- and the parser likes to supply a span like
// that for EOF, in particular.

if lo.col_display == hi.col_display && lo.line == hi.line {
hi.col_display += 1;
}

let ann_type = if lo.line != hi.line {
let ml = MultilineAnnotation {
depth: 1,
line_start: lo.line,
line_end: hi.line,
start_col: lo.col_display,
end_col: hi.col_display,
is_primary: span_label.is_primary,
label: span_label.label.clone(),
overlaps_exactly: false,
};
multiline_annotations.push((lo.file.clone(), ml.clone()));
AnnotationType::Multiline(ml)
} else {
AnnotationType::Singleline
};
let ann = Annotation {
start_col: lo.col_display,
end_col: hi.col_display,
is_primary: span_label.is_primary,
label: span_label.label.clone(),
annotation_type: ann_type,
};

if !ann.is_multiline() {
add_annotation_to_file(&mut output, lo.file, lo.line, ann);
}
}
}

// Find overlapping multiline annotations, put them at different depths
multiline_annotations.sort_by_key(|&(_, ref ml)| (ml.line_start, ml.line_end));
for item in multiline_annotations.clone() {
let ann = item.1;
for item in multiline_annotations.iter_mut() {
let ref mut a = item.1;
// Move all other multiline annotations overlapping with this one
// one level to the right.
if !(ann.same_span(a)) &&
num_overlap(ann.line_start, ann.line_end, a.line_start, a.line_end, true)
{
a.increase_depth();
} else if ann.same_span(a) && &ann != a {
a.overlaps_exactly = true;
} else {
break;
}
}
}

let mut max_depth = 0; // max overlapping multiline spans
for (file, ann) in multiline_annotations {
if ann.depth > max_depth {
max_depth = ann.depth;
}
let mut end_ann = ann.as_end();
if !ann.overlaps_exactly {
// avoid output like
//
// | foo(
// | _____^
// | |_____|
// | || bar,
// | || );
// | || ^
// | ||______|
// | |______foo
// | baz
//
// and instead get
//
// | foo(
// | _____^
// | | bar,
// | | );
// | | ^
// | | |
// | |______foo
// | baz
add_annotation_to_file(&mut output, file.clone(), ann.line_start, ann.as_start());
// 4 is the minimum vertical length of a multiline span when presented: two lines
// of code and two lines of underline. This is not true for the special case where
// the beginning doesn't have an underline, but the current logic seems to be
// working correctly.
let middle = min(ann.line_start + 4, ann.line_end);
for line in ann.line_start + 1..middle {
// Every `|` that joins the beginning of the span (`___^`) to the end (`|__^`).
add_annotation_to_file(&mut output, file.clone(), line, ann.as_line());
}
if middle < ann.line_end - 1 {
for line in ann.line_end - 1..ann.line_end {
add_annotation_to_file(&mut output, file.clone(), line, ann.as_line());
}
}
} else {
end_ann.annotation_type = AnnotationType::Singleline;
}
add_annotation_to_file(&mut output, file, ann.line_end, end_ann);
}
for file_vec in output.iter_mut() {
file_vec.multiline_depth = max_depth;
}
output
}
}

fn draw_col_separator(buffer: &mut StyledBuffer, line: usize, col: usize) {
buffer.puts(line, col, "| ", Style::LineNumber);
}
51 changes: 24 additions & 27 deletions src/librustc_mir/borrow_check/conflict_errors.rs
View file
Original file line number Diff line number Diff line change
@@ -595,12 +595,11 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
) -> (String, String, String, String) {
// Define a small closure that we can use to check if the type of a place
// is a union.
let is_union = |place: &Place<'tcx>| -> bool {
place.ty(self.mir, self.infcx.tcx).ty
.ty_adt_def()
.map(|adt| adt.is_union())
.unwrap_or(false)
let union_ty = |place: &Place<'tcx>| -> Option<Ty<'tcx>> {
let ty = place.ty(self.mir, self.infcx.tcx).ty;
ty.ty_adt_def().filter(|adt| adt.is_union()).map(|_| ty)
};
let describe_place = |place| self.describe_place(place).unwrap_or_else(|| "_".to_owned());

// Start with an empty tuple, so we can use the functions on `Option` to reduce some
// code duplication (particularly around returning an empty description in the failure
@@ -619,7 +618,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
let mut current = first_borrowed_place;
while let Place::Projection(box Projection { base, elem }) = current {
match elem {
ProjectionElem::Field(field, _) if is_union(base) => {
ProjectionElem::Field(field, _) if union_ty(base).is_some() => {
return Some((base, field));
},
_ => current = base,
@@ -632,34 +631,32 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
// borrowed place and look for a access to a different field of the same union.
let mut current = second_borrowed_place;
while let Place::Projection(box Projection { base, elem }) = current {
match elem {
ProjectionElem::Field(field, _) if {
is_union(base) && field != target_field && base == target_base
} => {
let desc_base = self.describe_place(base)
.unwrap_or_else(|| "_".to_owned());
let desc_first = self.describe_place(first_borrowed_place)
.unwrap_or_else(|| "_".to_owned());
let desc_second = self.describe_place(second_borrowed_place)
.unwrap_or_else(|| "_".to_owned());

// Also compute the name of the union type, eg. `Foo` so we
// can add a helpful note with it.
let ty = base.ty(self.mir, self.infcx.tcx).ty;

return Some((desc_base, desc_first, desc_second, ty.to_string()));
},
_ => current = base,
if let ProjectionElem::Field(field, _) = elem {
if let Some(union_ty) = union_ty(base) {
if field != target_field && base == target_base {
return Some((
describe_place(base),
describe_place(first_borrowed_place),
describe_place(second_borrowed_place),
union_ty.to_string(),
));
}
}
}

current = base;
}
None
})
.unwrap_or_else(|| {
// If we didn't find a field access into a union, or both places match, then
// only return the description of the first place.
let desc_place = self.describe_place(first_borrowed_place)
.unwrap_or_else(|| "_".to_owned());
(desc_place, "".to_string(), "".to_string(), "".to_string())
(
describe_place(first_borrowed_place),
"".to_string(),
"".to_string(),
"".to_string(),
)
})
}

20 changes: 10 additions & 10 deletions src/librustc_mir/borrow_check/mutability_errors.rs
View file
Original file line number Diff line number Diff line change
@@ -7,7 +7,7 @@ use rustc::mir::{
use rustc::mir::{Terminator, TerminatorKind};
use rustc::ty::{self, Const, DefIdTree, Ty, TyS, TyCtxt};
use rustc_data_structures::indexed_vec::Idx;
use syntax_pos::Span;
use syntax_pos::{Span, CompilerDesugaringKind};
use syntax_pos::symbol::kw;

use crate::dataflow::move_paths::InitLocation;
@@ -41,14 +41,19 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> {
);

let mut err;
let item_msg;
let reason;
let access_place_desc = self.describe_place(access_place);
debug!("report_mutability_error: access_place_desc={:?}", access_place_desc);

let mut item_msg = match (&access_place_desc, &the_place_err) {
(Some(desc), _) => format!("`{}`", desc),
(None, Place::Base(PlaceBase::Local(local))) if self.mir.local_decls[*local]
.source_info.span.is_compiler_desugaring(CompilerDesugaringKind::Async)
=> "`async fn` parameter".to_string(),
(None, _) => "temporary place".to_string(),
};
match the_place_err {
Place::Base(PlaceBase::Local(local)) => {
item_msg = format!("`{}`", access_place_desc.unwrap());
if let Place::Base(PlaceBase::Local(_)) = access_place {
reason = ", as it is not declared as mutable".to_string();
} else {
@@ -67,7 +72,6 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> {
base.ty(self.mir, self.infcx.tcx).ty
));

item_msg = format!("`{}`", access_place_desc.unwrap());
if self.is_upvar_field_projection(access_place).is_some() {
reason = ", as it is not declared as mutable".to_string();
} else {
@@ -82,7 +86,6 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> {
}) => {
if *base == Place::Base(PlaceBase::Local(Local::new(1))) &&
!self.upvars.is_empty() {
item_msg = format!("`{}`", access_place_desc.unwrap());
debug_assert!(self.mir.local_decls[Local::new(1)].ty.is_region_ptr());
debug_assert!(is_closure_or_generator(
the_place_err.ty(self.mir, self.infcx.tcx).ty
@@ -105,7 +108,6 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> {
false
}
} {
item_msg = format!("`{}`", access_place_desc.unwrap());
reason = ", as it is immutable for the pattern guard".to_string();
} else {
let pointer_type =
@@ -114,8 +116,7 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> {
} else {
"`*const` pointer"
};
if let Some(desc) = access_place_desc {
item_msg = format!("`{}`", desc);
if access_place_desc.is_some() {
reason = match error_access {
AccessKind::Move |
AccessKind::Mutate => format!(" which is behind a {}", pointer_type),
@@ -135,10 +136,9 @@ impl<'a, 'gcx, 'tcx> MirBorrowckCtxt<'a, 'gcx, 'tcx> {

Place::Base(PlaceBase::Static(box Static { kind: StaticKind::Static(def_id), .. })) => {
if let Place::Base(PlaceBase::Static(_)) = access_place {
item_msg = format!("immutable static item `{}`", access_place_desc.unwrap());
item_msg = format!("immutable static item {}", item_msg);
reason = String::new();
} else {
item_msg = format!("`{}`", access_place_desc.unwrap());
let static_name = &self.infcx.tcx.item_name(*def_id);
reason = format!(", as `{}` is an immutable static item", static_name);
}
6 changes: 5 additions & 1 deletion src/librustc_typeck/check/demand.rs
View file
Original file line number Diff line number Diff line change
@@ -270,7 +270,11 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
None
}

fn is_hir_id_from_struct_pattern_shorthand_field(&self, hir_id: hir::HirId, sp: Span) -> bool {
crate fn is_hir_id_from_struct_pattern_shorthand_field(
&self,
hir_id: hir::HirId,
sp: Span,
) -> bool {
let cm = self.sess().source_map();
let parent_id = self.tcx.hir().get_parent_node_by_hir_id(hir_id);
if let Some(parent) = self.tcx.hir().find_by_hir_id(parent_id) {
20 changes: 14 additions & 6 deletions src/librustc_typeck/check/mod.rs
View file
Original file line number Diff line number Diff line change
@@ -5010,6 +5010,10 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
Applicability::MachineApplicable,
);
} else if !self.check_for_cast(err, expr, found, expected) {
let is_struct_pat_shorthand_field = self.is_hir_id_from_struct_pattern_shorthand_field(
expr.hir_id,
expr.span,
);
let methods = self.get_conversion_methods(expr.span, expected, found);
if let Ok(expr_text) = self.sess().source_map().span_to_snippet(expr.span) {
let mut suggestions = iter::repeat(&expr_text).zip(methods.iter())
@@ -5019,14 +5023,18 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
None // do not suggest code that is already there (#53348)
} else {
let method_call_list = [".to_vec()", ".to_string()"];
if receiver.ends_with(".clone()")
let sugg = if receiver.ends_with(".clone()")
&& method_call_list.contains(&method_call.as_str()) {
let max_len = receiver.rfind(".").unwrap();
Some(format!("{}{}", &receiver[..max_len], method_call))
}
else {
Some(format!("{}{}", receiver, method_call))
}
format!("{}{}", &receiver[..max_len], method_call)
} else {
format!("{}{}", receiver, method_call)
};
Some(if is_struct_pat_shorthand_field {
format!("{}: {}", receiver, sugg)
} else {
sugg
})
}
}).peekable();
if suggestions.peek().is_some() {
9 changes: 9 additions & 0 deletions src/test/ui/async-await/issues/issue-61187.rs
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// edition:2018
#![feature(async_await)]

fn main() {
}

async fn response(data: Vec<u8>) {
data.reverse(); //~ ERROR E0596
}
11 changes: 11 additions & 0 deletions src/test/ui/async-await/issues/issue-61187.stderr
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
error[E0596]: cannot borrow `async fn` parameter as mutable, as it is not declared as mutable
--> $DIR/issue-61187.rs:8:5
|
LL | async fn response(data: Vec<u8>) {
| ---- help: consider changing this to be mutable: `mut data`
LL | data.reverse();
| ^^^^ cannot borrow as mutable

error: aborting due to previous error

For more information about this error, try `rustc --explain E0596`.
12 changes: 12 additions & 0 deletions src/test/ui/suggestions/issue-52820.rs
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
struct Bravery {
guts: String,
brains: String,
}

fn main() {
let guts = "mettle";
let _ = Bravery {
guts, //~ ERROR mismatched types
brains: guts.clone(), //~ ERROR mismatched types
};
}
27 changes: 27 additions & 0 deletions src/test/ui/suggestions/issue-52820.stderr
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
error[E0308]: mismatched types
--> $DIR/issue-52820.rs:9:9
|
LL | guts,
| ^^^^
| |
| expected struct `std::string::String`, found &str
| help: try using a conversion method: `guts: guts.to_string()`
|
= note: expected type `std::string::String`
found type `&str`

error[E0308]: mismatched types
--> $DIR/issue-52820.rs:10:17
|
LL | brains: guts.clone(),
| ^^^^^^^^^^^^
| |
| expected struct `std::string::String`, found &str
| help: try using a conversion method: `guts.to_string()`
|
= note: expected type `std::string::String`
found type `&str`

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0308`.