Rollup merge of #59473 - estebank:borrow-sugg-inside-macro, r=davidtwco

Do not emit incorrect borrow suggestion involving macros and fix overlapping multiline spans

Fix #58298.

7 files changed, 149 insertions, 20 deletions

diff --git a/src/librustc_errors/emitter.rs b/src/librustc_errors/emitter.rs
index e9f269b6e24..98db0097c74 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_exactly: 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_exactly = true;
                 } else {
                     break;
                 }
@@ -289,17 +289,49 @@ 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_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, 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..a0af604026d 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<String>,
+    pub overlaps_exactly: 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/librustc_typeck/check/demand.rs b/src/librustc_typeck/check/demand.rs
index 60fa266f0bb..1fc63a93022 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;
 
@@ -292,9 +292,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/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
@@ -375,6 +375,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#"
 fn foo() {
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`
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..ef1c09d2180
--- /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..bc7a7247a12
--- /dev/null
+++ b/src/test/ui/suggestions/dont-suggest-deref-inside-macro-issue-58298.stderr
@@ -0,0 +1,18 @@
+error[E0308]: mismatched types
+  --> $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
+   |
+   = 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`.