Skip to content
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.

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

Add line numbers and columns to error messages spanning multiple files #47780

Merged
merged 3 commits into from
Jan 30, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
15 changes: 14 additions & 1 deletion src/librustc_errors/emitter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1013,8 +1013,21 @@ impl EmitterWriter {

// Then, the secondary file indicator
buffer.prepend(buffer_msg_line_offset + 1, "::: ", Style::LineNumber);
let loc = if let Some(first_line) = annotated_file.lines.first() {
let col = if let Some(first_annotation) = first_line.annotations.first() {
format!(":{}", first_annotation.start_col + 1)
} else {
"".to_string()
};
format!("{}:{}{}",
annotated_file.file.name,
cm.doctest_offset_line(first_line.line_index),
col)
} else {
annotated_file.file.name.to_string()
};
buffer.append(buffer_msg_line_offset + 1,
&annotated_file.file.name.to_string(),
&loc,
Style::LineAndColumn);
for _ in 0..max_line_num_len {
buffer.prepend(buffer_msg_line_offset + 1, " ", Style::NoStyle);
Expand Down
3 changes: 2 additions & 1 deletion src/librustc_errors/snippet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ pub struct FileInfo {

/// The "primary file", if any, gets a `-->` marker instead of
/// `>>>`, and has a line-number/column printed and not just a
/// filename. It appears first in the listing. It is known to
/// filename (other files are not guaranteed to have line numbers
/// or columns). It appears first in the listing. It is known to
/// contain at least one primary span, though primary spans (which
/// are designated with `^^^`) may also occur in other files.
primary_span: Option<Span>,
Expand Down
16 changes: 16 additions & 0 deletions src/test/ui/cross-file-errors/main.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// 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_use]
mod underscore;

fn main() {
underscore!();
}
11 changes: 11 additions & 0 deletions src/test/ui/cross-file-errors/main.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
error: expected expression, found `_`
--> $DIR/underscore.rs:18:9
|
18 | _
| ^
|
::: $DIR/main.rs:15:5
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice! =)

|
15 | underscore!();
| -------------- in this macro invocation

20 changes: 20 additions & 0 deletions src/test/ui/cross-file-errors/underscore.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// 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.

// We want this file only so we can test cross-file error
// messages, but we don't want it in an external crate.
// ignore-test
#![crate_type = "lib"]

macro_rules! underscore {
() => (
_
)
}
10 changes: 5 additions & 5 deletions src/test/ui/macro_backtrace/main.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ error: expected one of `!`, `.`, `::`, `;`, `?`, `{`, `}`, or an operator, found
27 | ping!();
| -------- in this macro invocation
|
::: <ping macros>
::: <ping macros>:1:1
|
1 | ( ) => { pong ! ( ) ; }
| -------------------------
Expand All @@ -42,31 +42,31 @@ error: expected one of `!`, `.`, `::`, `;`, `?`, `{`, `}`, or an operator, found
28 | deep!();
| -------- in this macro invocation (#1)
|
::: <deep macros>
::: <deep macros>:1:1
Copy link
Contributor

@nikomatsakis nikomatsakis Jan 26, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this doesn't really feel like a win here... but I guess it's ok? Is an actual file line reflected here?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could check if annotated_file.file.name is FileName::Real(path_buf) to present the line and col, but honestly in reality I don't think it is going to be that much of a problem, and actually beneficial in real, verbose, macros.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I was ambivalent about this case: since it didn't seem to be decreasing readability, I decided not to special case it here. The line number is relative to the start of the macro, rather than the file in this case. It's also behind a -Z flag, so I thought it would affect people minimally regardless.

|
1 | ( ) => { foo ! ( ) ; }
| ------------------------
| | |
| | in this macro invocation (#2)
| in this expansion of `deep!` (#1)
|
::: <foo macros>
::: <foo macros>:1:1
|
1 | ( ) => { bar ! ( ) ; }
| ------------------------
| | |
| | in this macro invocation (#3)
| in this expansion of `foo!` (#2)
|
::: <bar macros>
::: <bar macros>:1:1
|
1 | ( ) => { ping ! ( ) ; }
| -------------------------
| | |
| | in this macro invocation (#4)
| in this expansion of `bar!` (#3)
|
::: <ping macros>
::: <ping macros>:1:1
|
1 | ( ) => { pong ! ( ) ; }
| -------------------------
Expand Down
2 changes: 1 addition & 1 deletion src/tools/compiletest/src/runtest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1402,7 +1402,7 @@ impl<'test> TestCx<'test> {
}

/// For each `aux-build: foo/bar` annotation, we check to find the
/// file in a `aux` directory relative to the test itself.
/// file in a `auxiliary` directory relative to the test itself.
fn compute_aux_test_paths(&self, rel_ab: &str) -> TestPaths {
let test_ab = self.testpaths
.file
Expand Down