From 0acb7867c21bca6e3ba53e88ef895c2ed2d833a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Wed, 27 Mar 2019 17:04:58 -0700 Subject: [PATCH 1/6] Do not suggest borrowing when the span comes from a macro --- src/librustc_typeck/check/demand.rs | 7 ++++--- src/test/ui/span/coerce-suggestions.stderr | 5 +---- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/src/librustc_typeck/check/demand.rs b/src/librustc_typeck/check/demand.rs index b1a249d821b..302c093d12f 100644 --- a/src/librustc_typeck/check/demand.rs +++ b/src/librustc_typeck/check/demand.rs @@ -10,7 +10,7 @@ use rustc::hir::Node; use rustc::hir::{Item, ItemKind, print}; use rustc::ty::{self, Ty, AssociatedItem}; use rustc::ty::adjustment::AllowTwoPhase; -use errors::{Applicability, DiagnosticBuilder, SourceMapper}; +use errors::{Applicability, DiagnosticBuilder}; use super::method::probe; @@ -271,9 +271,10 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { expected: Ty<'tcx>) -> Option<(Span, &'static str, String)> { let cm = self.sess().source_map(); - // Use the callsite's span if this is a macro call. #41858 - let sp = cm.call_span_if_macro(expr.span); + let sp = expr.span; if !cm.span_to_filename(sp).is_real() { + // Ignore if span is from within a macro #41858, #58298. We previously used the macro + // call span, but that breaks down when the type error comes from multiple calls down. return None; } diff --git a/src/test/ui/span/coerce-suggestions.stderr b/src/test/ui/span/coerce-suggestions.stderr index cda500c3f5d..996d80a07e0 100644 --- a/src/test/ui/span/coerce-suggestions.stderr +++ b/src/test/ui/span/coerce-suggestions.stderr @@ -50,10 +50,7 @@ error[E0308]: mismatched types --> $DIR/coerce-suggestions.rs:21:9 | LL | s = format!("foo"); - | ^^^^^^^^^^^^^^ - | | - | expected mutable reference, found struct `std::string::String` - | help: consider mutably borrowing here: `&mut format!("foo")` + | ^^^^^^^^^^^^^^ expected mutable reference, found struct `std::string::String` | = note: expected type `&mut std::string::String` found type `std::string::String` From 925ca49cf12ef9ce328eb7ad66e63c4c86b02460 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Wed, 27 Mar 2019 17:05:35 -0700 Subject: [PATCH 2/6] Add test --- ...-suggest-deref-inside-macro-issue-58298.rs | 14 +++++++++++++ ...gest-deref-inside-macro-issue-58298.stderr | 21 +++++++++++++++++++ 2 files changed, 35 insertions(+) create mode 100644 src/test/ui/suggestions/dont-suggest-deref-inside-macro-issue-58298.rs create mode 100644 src/test/ui/suggestions/dont-suggest-deref-inside-macro-issue-58298.stderr diff --git a/src/test/ui/suggestions/dont-suggest-deref-inside-macro-issue-58298.rs b/src/test/ui/suggestions/dont-suggest-deref-inside-macro-issue-58298.rs new file mode 100644 index 00000000000..bf2d105f63f --- /dev/null +++ b/src/test/ui/suggestions/dont-suggest-deref-inside-macro-issue-58298.rs @@ -0,0 +1,14 @@ +fn warn(_: &str) {} + +macro_rules! intrinsic_match { + ($intrinsic:expr) => { + warn(format!("unsupported intrinsic {}", $intrinsic)); + //^~ ERROR mismatched types + }; +} + +fn main() { + intrinsic_match! { + "abc" + }; +} diff --git a/src/test/ui/suggestions/dont-suggest-deref-inside-macro-issue-58298.stderr b/src/test/ui/suggestions/dont-suggest-deref-inside-macro-issue-58298.stderr new file mode 100644 index 00000000000..75a08904e69 --- /dev/null +++ b/src/test/ui/suggestions/dont-suggest-deref-inside-macro-issue-58298.stderr @@ -0,0 +1,21 @@ +error[E0308]: mismatched types + --> $DIR/dont-suggest-deref-inside-macro-issue-58298.rs:10:5 + | +LL | intrinsic_match! { + | _____^ + | |_____| + | || +LL | || "abc" +LL | || }; + | || ^ + | ||______| + | |_______expected &str, found struct `std::string::String` + | in this macro invocation + | + = note: expected type `&str` + found type `std::string::String` + = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0308`. From 326ec800b952749d0afc90be0604dc0332d70324 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Wed, 27 Mar 2019 19:26:47 -0700 Subject: [PATCH 3/6] Account for fully overlapping multiline annotations When two multiline span labels point at the same span, we special case the output to avoid weird behavior: ``` foo( _____^ |_____| || bar, || ); || ^ ||______| |______foo baz ``` instead showing ``` foo( _____^ | bar, | ); | ^ | | |______foo baz ``` --- src/librustc_errors/emitter.rs | 53 ++++++++++++---- src/librustc_errors/snippet.rs | 7 +++ src/libsyntax/test_snippet.rs | 60 +++++++++++++++++++ ...-suggest-deref-inside-macro-issue-58298.rs | 2 +- ...gest-deref-inside-macro-issue-58298.stderr | 19 +++--- 5 files changed, 116 insertions(+), 25 deletions(-) diff --git a/src/librustc_errors/emitter.rs b/src/librustc_errors/emitter.rs index e9f269b6e24..c3ee1db8d30 100644 --- a/src/librustc_errors/emitter.rs +++ b/src/librustc_errors/emitter.rs @@ -243,6 +243,7 @@ impl EmitterWriter { end_col: hi.col_display, is_primary: span_label.is_primary, label: span_label.label.clone(), + overlaps: false, }; multiline_annotations.push((lo.file.clone(), ml.clone())); AnnotationType::Multiline(ml) @@ -258,10 +259,7 @@ impl EmitterWriter { }; if !ann.is_multiline() { - add_annotation_to_file(&mut output, - lo.file, - lo.line, - ann); + add_annotation_to_file(&mut output, lo.file, lo.line, ann); } } } @@ -274,10 +272,12 @@ impl EmitterWriter { let ref mut a = item.1; // Move all other multiline annotations overlapping with this one // one level to the right. - if &ann != a && + 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 = true; } else { break; } @@ -289,17 +289,44 @@ impl EmitterWriter { if ann.depth > max_depth { max_depth = ann.depth; } - add_annotation_to_file(&mut output, file.clone(), ann.line_start, ann.as_start()); - let middle = min(ann.line_start + 4, ann.line_end); - for line in ann.line_start + 1..middle { - 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 { + let mut end_ann = ann.as_end(); + if !ann.overlaps { + // 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()); + let middle = min(ann.line_start + 4, ann.line_end); + for line in ann.line_start + 1..middle { 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, ann.as_end()); + 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; diff --git a/src/librustc_errors/snippet.rs b/src/librustc_errors/snippet.rs index 0c62ff0ff89..60ee0c25727 100644 --- a/src/librustc_errors/snippet.rs +++ b/src/librustc_errors/snippet.rs @@ -18,6 +18,7 @@ pub struct MultilineAnnotation { pub end_col: usize, pub is_primary: bool, pub label: Option, + pub overlaps: bool, } impl MultilineAnnotation { @@ -25,6 +26,12 @@ impl MultilineAnnotation { self.depth += 1; } + /// Compare two `MultilineAnnotation`s considering only the `Span` they cover. + pub fn same_span(&self, other: &MultilineAnnotation) -> bool { + self.line_start == other.line_start && self.line_end == other.line_end + && self.start_col == other.start_col && self.end_col == other.end_col + } + pub fn as_start(&self) -> Annotation { Annotation { start_col: self.start_col, diff --git a/src/libsyntax/test_snippet.rs b/src/libsyntax/test_snippet.rs index 2b3d18835d5..86910ffd894 100644 --- a/src/libsyntax/test_snippet.rs +++ b/src/libsyntax/test_snippet.rs @@ -374,6 +374,66 @@ error: foo "#); } +#[test] +fn triple_exact_overlap() { + test_harness(r#" +fn foo() { + X0 Y0 Z0 + X1 Y1 Z1 + X2 Y2 Z2 +} +"#, + vec![ + SpanLabel { + start: Position { + string: "X0", + count: 1, + }, + end: Position { + string: "X2", + count: 1, + }, + label: "`X` is a good letter", + }, + SpanLabel { + start: Position { + string: "X0", + count: 1, + }, + end: Position { + string: "X2", + count: 1, + }, + label: "`Y` is a good letter too", + }, + SpanLabel { + start: Position { + string: "X0", + count: 1, + }, + end: Position { + string: "X2", + count: 1, + }, + label: "`Z` label", + }, + ], + r#" +error: foo + --> test.rs:3:3 + | +3 | / X0 Y0 Z0 +4 | | X1 Y1 Z1 +5 | | X2 Y2 Z2 + | | ^ + | | | + | | `X` is a good letter + | |____`Y` is a good letter too + | `Z` label + +"#); +} + #[test] fn minimum_depth() { test_harness(r#" diff --git a/src/test/ui/suggestions/dont-suggest-deref-inside-macro-issue-58298.rs b/src/test/ui/suggestions/dont-suggest-deref-inside-macro-issue-58298.rs index bf2d105f63f..ef1c09d2180 100644 --- a/src/test/ui/suggestions/dont-suggest-deref-inside-macro-issue-58298.rs +++ b/src/test/ui/suggestions/dont-suggest-deref-inside-macro-issue-58298.rs @@ -3,7 +3,7 @@ fn warn(_: &str) {} macro_rules! intrinsic_match { ($intrinsic:expr) => { warn(format!("unsupported intrinsic {}", $intrinsic)); - //^~ ERROR mismatched types + //~^ ERROR mismatched types }; } diff --git a/src/test/ui/suggestions/dont-suggest-deref-inside-macro-issue-58298.stderr b/src/test/ui/suggestions/dont-suggest-deref-inside-macro-issue-58298.stderr index 75a08904e69..bc7a7247a12 100644 --- a/src/test/ui/suggestions/dont-suggest-deref-inside-macro-issue-58298.stderr +++ b/src/test/ui/suggestions/dont-suggest-deref-inside-macro-issue-58298.stderr @@ -1,16 +1,13 @@ error[E0308]: mismatched types - --> $DIR/dont-suggest-deref-inside-macro-issue-58298.rs:10:5 + --> $DIR/dont-suggest-deref-inside-macro-issue-58298.rs:11:5 | -LL | intrinsic_match! { - | _____^ - | |_____| - | || -LL | || "abc" -LL | || }; - | || ^ - | ||______| - | |_______expected &str, found struct `std::string::String` - | in this macro invocation +LL | / intrinsic_match! { +LL | | "abc" +LL | | }; + | | ^ + | | | + | |______expected &str, found struct `std::string::String` + | in this macro invocation | = note: expected type `&str` found type `std::string::String` From e13e9a5d636116455aab6940d27bbc1b450aa003 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 28 Mar 2019 20:03:13 -0700 Subject: [PATCH 4/6] review comments --- src/librustc_errors/emitter.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/librustc_errors/emitter.rs b/src/librustc_errors/emitter.rs index c3ee1db8d30..d2c9db2db79 100644 --- a/src/librustc_errors/emitter.rs +++ b/src/librustc_errors/emitter.rs @@ -243,7 +243,7 @@ impl EmitterWriter { end_col: hi.col_display, is_primary: span_label.is_primary, label: span_label.label.clone(), - overlaps: false, + overlaps_exactly: false, }; multiline_annotations.push((lo.file.clone(), ml.clone())); AnnotationType::Multiline(ml) @@ -277,7 +277,7 @@ impl EmitterWriter { { a.increase_depth(); } else if ann.same_span(a) && &ann != a { - a.overlaps = true; + a.overlaps_exactly = true; } else { break; } @@ -290,7 +290,7 @@ impl EmitterWriter { max_depth = ann.depth; } let mut end_ann = ann.as_end(); - if !ann.overlaps { + if !ann.overlaps_exactly { // avoid output like // // | foo( From 8fad69c2001ebe78d67e193f05969bb6f681b109 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 28 Mar 2019 20:18:50 -0700 Subject: [PATCH 5/6] Add comemnts clarifying logic --- src/librustc_errors/emitter.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/librustc_errors/emitter.rs b/src/librustc_errors/emitter.rs index d2c9db2db79..98db0097c74 100644 --- a/src/librustc_errors/emitter.rs +++ b/src/librustc_errors/emitter.rs @@ -314,8 +314,13 @@ impl EmitterWriter { // | |______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 { From b5690c2cb86afc48e958e0d58ff1916c75f65b66 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Esteban=20K=C3=BCber?= Date: Thu, 28 Mar 2019 20:19:50 -0700 Subject: [PATCH 6/6] Fix MultilineAnnotation field name --- src/librustc_errors/snippet.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc_errors/snippet.rs b/src/librustc_errors/snippet.rs index 60ee0c25727..a0af604026d 100644 --- a/src/librustc_errors/snippet.rs +++ b/src/librustc_errors/snippet.rs @@ -18,7 +18,7 @@ pub struct MultilineAnnotation { pub end_col: usize, pub is_primary: bool, pub label: Option, - pub overlaps: bool, + pub overlaps_exactly: bool, } impl MultilineAnnotation {