Use more precise span when reporting semicolon hint

When reporting "consider removing this semicolon" hint message, the
offending semicolon may come from macro call site instead of macro
itself. Using the more appropriate span makes the hint more helpful.

Closes #13428.

diff --git a/src/librustc/middle/liveness.rs b/src/librustc/middle/liveness.rs
index a9cc45d2b13d5a8b581e514b159f4e89602ab576..65f1a48114e106f301562efe8cbdc47263f4694b 100644 (file)
--- a/src/librustc/middle/liveness.rs
+++ b/src/librustc/middle/liveness.rs
@@ -117,7 +117,7 @@
 use std::str;
 use std::uint;
 use syntax::ast::*;
-use syntax::codemap::Span;
+use syntax::codemap::{BytePos, original_sp, Span};
 use syntax::parse::token::special_idents;
 use syntax::parse::token;
 use syntax::print::pprust::{expr_to_str, block_to_str};
@@ -1473,10 +1473,11 @@ fn check_ret(&self,
                 };
                 if ends_with_stmt {
                     let last_stmt = body.stmts.last().unwrap();
+                    let original_span = original_sp(last_stmt.span, sp);
                     let span_semicolon = Span {
-                        lo: last_stmt.span.hi,
-                        hi: last_stmt.span.hi,
-                        expn_info: last_stmt.span.expn_info
+                        lo: original_span.hi - BytePos(1),
+                        hi: original_span.hi,
+                        expn_info: original_span.expn_info
                     };
                     self.ir.tcx.sess.span_note(
                         span_semicolon, "consider removing this semicolon:");

diff --git a/src/libsyntax/codemap.rs b/src/libsyntax/codemap.rs
index b174dffdfeccf72540a4ddd9a4b521d67d5daefc..3c890189ed952c68c7970f8ca82bfb647c228a8b 100644 (file)
--- a/src/libsyntax/codemap.rs
+++ b/src/libsyntax/codemap.rs
@@ -141,6 +141,17 @@ pub fn mk_sp(lo: BytePos, hi: BytePos) -> Span {
     Span {lo: lo, hi: hi, expn_info: None}
 }
 
+/// Return the span itself if it doesn't come from a macro expansion,
+/// otherwise return the call site span up to the `enclosing_sp` by
+/// following the `expn_info` chain.
+pub fn original_sp(sp: Span, enclosing_sp: Span) -> Span {
+    match (sp.expn_info, enclosing_sp.expn_info) {
+        (None, _) => sp,
+        (Some(expn1), Some(expn2)) if expn1.call_site == expn2.call_site => sp,
+        (Some(expn1), _) => original_sp(expn1.call_site, enclosing_sp),
+    }
+}
+
 /// A source code location used for error reporting
 pub struct Loc {
     /// Information about the original source

diff --git a/src/libsyntax/parse/mod.rs b/src/libsyntax/parse/mod.rs
index eca9f955d9392fa3131ecd55d24aad56cae058c0..f83ee200c9ec3f5466361edb934d7830bc7f30b8 100644 (file)
--- a/src/libsyntax/parse/mod.rs
+++ b/src/libsyntax/parse/mod.rs
@@ -686,7 +686,7 @@ fn parser_done(p: Parser){
                                                       }),
                                                 span: sp(17,18)},
                                                 ast::DUMMY_NODE_ID),
-                                            span: sp(17,18)}),
+                                            span: sp(17,19)}),
                                         expr: None,
                                         id: ast::DUMMY_NODE_ID,
                                         rules: ast::DefaultBlock, // no idea

diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs
index 379403e54097726f65794f56a570b26d53b009c4..6943d03e6f10c42a891582e75367b60efc6e4b6f 100644 (file)
--- a/src/libsyntax/parse/parser.rs
+++ b/src/libsyntax/parse/parser.rs
@@ -3260,9 +3260,14 @@ fn parse_block_tail_(&mut self, lo: BytePos, s: BlockCheckMode,
                             match self.token {
                                 token::SEMI => {
                                     self.bump();
+                                    let span_with_semi = Span {
+                                        lo: stmt.span.lo,
+                                        hi: self.last_span.hi,
+                                        expn_info: stmt.span.expn_info,
+                                    };
                                     stmts.push(@codemap::Spanned {
                                         node: StmtSemi(e, stmt_id),
-                                        span: stmt.span,
+                                        span: span_with_semi,
                                     });
                                 }
                                 token::RBRACE => {
@@ -3275,33 +3280,26 @@ fn parse_block_tail_(&mut self, lo: BytePos, s: BlockCheckMode,
                         }
                         StmtMac(ref m, _) => {
                             // statement macro; might be an expr
-                            let has_semi;
                             match self.token {
                                 token::SEMI => {
-                                    has_semi = true;
+                                    self.bump();
+                                    stmts.push(@codemap::Spanned {
+                                        node: StmtMac((*m).clone(), true),
+                                        span: stmt.span,
+                                    });
                                 }
                                 token::RBRACE => {
                                     // if a block ends in `m!(arg)` without
                                     // a `;`, it must be an expr
-                                    has_semi = false;
                                     expr = Some(
                                         self.mk_mac_expr(stmt.span.lo,
                                                          stmt.span.hi,
                                                          m.node.clone()));
                                 }
                                 _ => {
-                                    has_semi = false;
                                     stmts.push(stmt);
                                 }
                             }
-
-                            if has_semi {
-                                self.bump();
-                                stmts.push(@codemap::Spanned {
-                                    node: StmtMac((*m).clone(), true),
-                                    span: stmt.span,
-                                });
-                            }
                         }
                         _ => { // all other kinds of statements:
                             stmts.push(stmt);

diff --git a/src/test/compile-fail/issue-13428.rs b/src/test/compile-fail/issue-13428.rs
new file mode 100644 (file)
index 0000000..7e24d90
--- /dev/null
+++ b/src/test/compile-fail/issue-13428.rs
@@ -0,0 +1,26 @@
+// Copyright 2014 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.
+
+// Regression test for #13428
+
+fn foo() -> ~str {  //~ ERROR not all control paths return a value
+    format!("Hello {}",
+            "world")
+    // Put the trailing semicolon on its own line to test that the
+    // note message gets the offending semicolon exactly
+    ;   //~ NOTE consider removing this semicolon
+}
+
+fn bar() -> ~str {  //~ ERROR not all control paths return a value
+    "foobar".to_owned()
+    ;   //~ NOTE consider removing this semicolon
+}
+
+pub fn main() {}