From aaec60836761da35a8d0cf6179769eb9bc9f63c3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Esteban=20K=C3=BCber?= <esteban@kuber.com.ar>
Date: Thu, 1 Feb 2018 11:51:49 -0800
Subject: [PATCH] Minimize weird spans involving macro context

Sometimes the parser attempts to synthesize spans from within a macro
context with the span for the captured argument, leading to non-sensical
spans with very bad output. Given that an incorrect span is worse than
a partially incomplete span, when detecting this situation return only
one of the spans without mergin them.
---
 src/libsyntax_pos/lib.rs                      | 23 ++++++++++++++-----
 .../ui/macros/span-covering-argument-1.rs     | 23 +++++++++++++++++++
 .../ui/macros/span-covering-argument-1.stderr | 13 +++++++++++
 .../ui/span/macro-span-replacement.stderr     |  4 ++--
 4 files changed, 55 insertions(+), 8 deletions(-)
 create mode 100644 src/test/ui/macros/span-covering-argument-1.rs
 create mode 100644 src/test/ui/macros/span-covering-argument-1.stderr

diff --git a/src/libsyntax_pos/lib.rs b/src/libsyntax_pos/lib.rs
index 85f0925b982..09e2677eed6 100644
--- a/src/libsyntax_pos/lib.rs
+++ b/src/libsyntax_pos/lib.rs
@@ -361,13 +361,24 @@ impl Span {
 
     /// Return a `Span` that would enclose both `self` and `end`.
     pub fn to(self, end: Span) -> Span {
-        let span = self.data();
-        let end = end.data();
+        let span_data = self.data();
+        let end_data = end.data();
+        // FIXME(jseyfried): self.ctxt should always equal end.ctxt here (c.f. issue #23480)
+        // Return the macro span on its own to avoid weird diagnostic output. It is preferable to
+        // have an incomplete span than a completely nonsensical one.
+        if span_data.ctxt != end_data.ctxt {
+            if span_data.ctxt == SyntaxContext::empty() {
+                return end;
+            } else if end_data.ctxt == SyntaxContext::empty() {
+                return self;
+            }
+            // both span fall within a macro
+            // FIXME(estebank) check if it is the *same* macro
+        }
         Span::new(
-            cmp::min(span.lo, end.lo),
-            cmp::max(span.hi, end.hi),
-            // FIXME(jseyfried): self.ctxt should always equal end.ctxt here (c.f. issue #23480)
-            if span.ctxt == SyntaxContext::empty() { end.ctxt } else { span.ctxt },
+            cmp::min(span_data.lo, end_data.lo),
+            cmp::max(span_data.hi, end_data.hi),
+            if span_data.ctxt == SyntaxContext::empty() { end_data.ctxt } else { span_data.ctxt },
         )
     }
 
diff --git a/src/test/ui/macros/span-covering-argument-1.rs b/src/test/ui/macros/span-covering-argument-1.rs
new file mode 100644
index 00000000000..bfc137fc7b2
--- /dev/null
+++ b/src/test/ui/macros/span-covering-argument-1.rs
@@ -0,0 +1,23 @@
+// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+macro_rules! bad {
+    ($s:ident whatever) => {
+        {
+            let $s = 0;
+            *&mut $s = 0;
+            //~^ ERROR cannot borrow immutable local variable `foo` as mutable [E0596]
+        }
+    }
+}
+
+fn main() {
+    bad!(foo whatever);
+}
diff --git a/src/test/ui/macros/span-covering-argument-1.stderr b/src/test/ui/macros/span-covering-argument-1.stderr
new file mode 100644
index 00000000000..677d2f10fd6
--- /dev/null
+++ b/src/test/ui/macros/span-covering-argument-1.stderr
@@ -0,0 +1,13 @@
+error[E0596]: cannot borrow immutable local variable `foo` as mutable
+  --> $DIR/span-covering-argument-1.rs:15:19
+   |
+14 |             let $s = 0;
+   |                 -- consider changing this to `mut $s`
+15 |             *&mut $s = 0;
+   |                   ^^ cannot borrow mutably
+...
+22 |     bad!(foo whatever);
+   |     ------------------- in this macro invocation
+
+error: aborting due to previous error
+
diff --git a/src/test/ui/span/macro-span-replacement.stderr b/src/test/ui/span/macro-span-replacement.stderr
index 2a0e71e192c..728cd12e2c6 100644
--- a/src/test/ui/span/macro-span-replacement.stderr
+++ b/src/test/ui/span/macro-span-replacement.stderr
@@ -1,8 +1,8 @@
 warning: struct is never used: `S`
-  --> $DIR/macro-span-replacement.rs:17:9
+  --> $DIR/macro-span-replacement.rs:17:14
    |
 17 |         $b $a; //~ WARN struct is never used
-   |         ^^^^^^
+   |              ^
 ...
 22 |     m!(S struct);
    |     ------------- in this macro invocation