Propagate macro backtraces more often, improve formatting diagnostics #24003

rprichard · 2015-04-03T10:40:32Z

In noop_fold_expr, call new_span in these cases:
- ExprMethodCall's identifier
- ExprField's identifier
- ExprTupField's integer
Calling new_span for ExprMethodCall's identifier is necessary to print
an acceptable diagnostic for write!(&2, ""). We see this error:
```
<std macros>:2:20: 2:66 error: type `&mut _` does not implement any method in scope named `write_fmt`
<std macros>:2 ( & mut * $ dst ) . write_fmt ( format_args ! ( $ ( $ arg ) * ) ) )
                                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
```
With this change, we also see a macro expansion backtrace leading to
the write!(&2, "") call site.
After fully expanding a macro, we replace the expansion expression's
span with the original span. Call fld.new_span to add a backtrace to
this span. (Note that I'm call new_span after bt.pop(), so the macro
just expanded isn't on the backtrace.)

The motivating example for this change is println!("{}"). The format
string literal is concat!($fmt, "arg") and is inside the libstd macro.
We need to see the backtrace to find the println! call site.
Add a backtrace to the format_args! format expression span.

r? alexcrichton

Addresses #23459

rust-highfive · 2015-04-03T10:40:37Z

r? @pcwalton

(rust_highfive has picked a reviewer for you, use r? to override)

rprichard · 2015-04-03T10:41:51Z

r? @alexcrichton

cc @huonw @alexcrichton

rprichard · 2015-04-03T10:53:27Z

It occurs to me that #23459 actually had a much better motivating example than write!(&2, ...) -- rather than 2, the argument could be an std::fs::File, but std::io::Write hasn't been used. That's far more likely to happen.

huonw · 2015-04-03T11:17:47Z

Great! Could you add some tests for these? E.g. for the write! case, one could test it by creating a file like macro-backtrace-invalid-internals.rs in src/test/compile-fail.

macro_rules! foo { //~ NODE in expansion of
     () => {
          1.fake(); //~ ERROR does not implement any method
     }
}

fn main() {
     foo!(); //~ NOTE expansion site
}

Similarly one could have macro-backtrace-println.rs etc.

rprichard · 2015-04-03T11:36:42Z

Yes, I can add some tests.

rprichard · 2015-04-04T03:38:52Z

I added new tests:

I expanded the internal-unstable compile-fail test to also test method calls and field accesses, which are both fixed in this PR.
I added three new compile-fail tests, macro-backtrace-{invalid-internals,nested,println}.rs

rprichard · 2015-04-05T06:43:24Z

I've noticed that the backtraces introduced with this PR can appear repeatedly:

rprichard@ryan:~$ fixld ~/work/rust-staging2/build/install/bin/rustc test.rs 
<std macros>:2:20: 2:66 error: type `&mut std::fs::File` does not implement any method in scope named `write_fmt`
<std macros>:2 ( & mut * $ dst ) . write_fmt ( format_args ! ( $ ( $ arg ) * ) ) )
                                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
<std macros>:1:1: 2:68 note: in expansion of write!
<std macros>:4:1: 4:62 note: expansion site
<std macros>:1:1: 4:66 note: in expansion of writeln!
test.rs:9:13: 9:45 note: expansion site
<std macros>:2:66: 2:66 help: methods from traits can only be called if the trait is in scope; the following trait is implemented but not in scope, perhaps add a `use` for it:
<std macros>:1:1: 2:68 note: in expansion of write!
<std macros>:4:1: 4:62 note: expansion site
<std macros>:1:1: 4:66 note: in expansion of writeln!
test.rs:9:13: 9:45 note: expansion site
<std macros>:2:66: 2:66 help: candidate #1: use `std::io::Write`
<std macros>:1:1: 2:68 note: in expansion of write!
<std macros>:4:1: 4:62 note: expansion site
<std macros>:1:1: 4:66 note: in expansion of writeln!
test.rs:9:13: 9:45 note: expansion site
error: aborting due to previous error

The second two backtraces are obnoxious. rustc is trying to print a list of candidates, but it's hard to read because we output a backtrace after each line. It's the same span for all the lines, too, so the backtraces are redundant.

I suspect the fix is for fileline_note and fileline_help to omit the macro backtrace.

I'm also wondering about the first span_note call in rustc_typeck::check::method::suggest::report_error. Why isn't it fileline_note?

Also: ExtCtxt::print_backtrace was added in 2011-08-05 and reduced to a no-op ten days later. I think I'm going to remove it. Probably needs a separate PR.

huonw · 2015-04-06T04:41:42Z

I suspect the fix is for fileline_note and fileline_help to omit the macro backtrace.

I agree.

I'm also wondering about the first span_note call in rustc_typeck::check::method::suggest::report_error. Why isn't it fileline_note?

We haven't been very good about using fileline_* when it would be more appropriate. That is definitely one example.

huonw · 2015-04-06T04:44:25Z

src/libsyntax/diagnostic.rs

@@ -453,7 +453,7 @@ fn emit(dst: &mut EmitterWriter, cm: &codemap::CodeMap, rsp: RenderSpan,
            try!(highlight_lines(dst, cm, sp, lvl, cm.span_to_lines(sp)));
        }
    }
-    if sp != COMMAND_LINE_SP {
+    if sp.expn_id != COMMAND_LINE_EXPN && rsp.is_full_span() {


Hm, could you add a brief comment about this?

There's some code a few lines up doing the same sort of sp.expn_id to COMMAND_LINE_EXPN comparison:

// We cannot check equality directly with COMMAND_LINE_SP // since PartialEq is manually implemented to ignore the ExpnId let ss = if sp.expn_id == COMMAND_LINE_EXPN { ...

I'll factor out the sp.expn_id == COMMAND_LINE_EXPN to a local variable and put the comment there.

huonw · 2015-04-06T04:44:50Z

Looks good, r=me with one little comment.

bors · 2015-04-07T01:01:24Z

☔ The latest upstream changes (presumably #23857) made this pull request unmergeable. Please resolve the merge conflicts.

rprichard · 2015-04-07T10:28:50Z

There seems to be a general problem where we don't output macro backtraces until after the expansion phase. The format_args! change in this PR fixes this problem for the format string, but there are other instances of the problem, including format_args! arguments:

macro_rules! format_template {
    ($tmp:tt) => (format!($tmp, a=123, b=456))
}
fn main() {
    format_template!("{a}{b}");
    format_template!("{a}");
    format_template!("{a}{b}");
    format_template!("{a}");
}

rprichard@ryan:~/mess$ rustc format-template.rs 
format-template.rs:2:42: 2:45 error: named argument never used
format-template.rs:2     ($tmp:tt) => (format!($tmp, a=123, b=456))
                                                              ^~~
format-template.rs:2:42: 2:45 error: named argument never used
format-template.rs:2     ($tmp:tt) => (format!($tmp, a=123, b=456))
                                                              ^~~
error: aborting due to 2 previous errors

rprichard · 2015-04-10T08:02:56Z

@huonw I rebased my changes, factored out a is_command_line variable with a 2-line comment above it, and squashed one of the commit's that just reordered imports from the previous commit.

bors · 2015-04-11T21:07:09Z

☔ The latest upstream changes (presumably #24328) made this pull request unmergeable. Please resolve the merge conflicts.

* In noop_fold_expr, call new_span in these cases: - ExprMethodCall's identifier - ExprField's identifier - ExprTupField's integer Calling new_span for ExprMethodCall's identifier is necessary to print an acceptable diagnostic for write!(&2, ""). We see this error: <std macros>:2:20: 2:66 error: type `&mut _` does not implement any method in scope named `write_fmt` <std macros>:2 ( & mut * $ dst ) . write_fmt ( format_args ! ( $ ( $ arg ) * ) ) ) ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ With this change, we also see a macro expansion backtrace leading to the write!(&2, "") call site. * After fully expanding a macro, we replace the expansion expression's span with the original span. Call fld.new_span to add a backtrace to this span. (Note that I'm call new_span after bt.pop(), so the macro just expanded isn't on the backtrace.) The motivating example for this change is println!("{}"). The format string literal is concat!($fmt, "arg") and is inside the libstd macro. We need to see the backtrace to find the println! call site. * Add a backtrace to the format_args! format expression span. Addresses rust-lang#23459

…lls.

…ests

It was added in 2011-08-05 and reduced to a no-op ten days later.

rprichard · 2015-04-12T02:21:09Z

@huonw I rebased my PR to account for @nikomatsakis's diagnostic improvements.

I rewrote the Suppress the macro backtrace commit. The other commits should be unchanged.

huonw · 2015-04-12T06:42:36Z

@bors r+

bors · 2015-04-12T06:42:37Z

📌 Commit ddbdf51 has been approved by huonw

bors · 2015-04-12T06:48:28Z

⌛ Testing commit ddbdf51 with merge feeb23d...

* In `noop_fold_expr`, call `new_span` in these cases: - `ExprMethodCall`'s identifier - `ExprField`'s identifier - `ExprTupField`'s integer Calling `new_span` for `ExprMethodCall`'s identifier is necessary to print an acceptable diagnostic for `write!(&2, "")`. We see this error: ``` <std macros>:2:20: 2:66 error: type `&mut _` does not implement any method in scope named `write_fmt` <std macros>:2 ( & mut * $ dst ) . write_fmt ( format_args ! ( $ ( $ arg ) * ) ) ) ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ``` With this change, we also see a macro expansion backtrace leading to the `write!(&2, "")` call site. * After fully expanding a macro, we replace the expansion expression's span with the original span. Call `fld.new_span` to add a backtrace to this span. (Note that I'm call `new_span` after `bt.pop()`, so the macro just expanded isn't on the backtrace.) The motivating example for this change is `println!("{}")`. The format string literal is `concat!($fmt, "arg")` and is inside the libstd macro. We need to see the backtrace to find the `println!` call site. * Add a backtrace to the `format_args!` format expression span. r? alexcrichton Addresses #23459

bors · 2015-04-12T09:03:02Z

☀️ Test successful - auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-64-nopt-t, auto-linux-64-opt, auto-linux-64-x-android-t, auto-mac-32-opt, auto-mac-64-nopt-t, auto-mac-64-opt, auto-win-32-nopt-t, auto-win-32-opt, auto-win-64-nopt-t, auto-win-64-opt

rust-highfive assigned pcwalton Apr 3, 2015

huonw assigned huonw and unassigned pcwalton Apr 3, 2015

rprichard mentioned this pull request Apr 5, 2015

Compiler provides incorrect error location for macros instantiations #24058

Closed

rprichard referenced this pull request in Manishearth/rust Apr 6, 2015

Fix checking of command line expansion spans (fixes rust-lang#20747)

4a7b710

huonw reviewed Apr 6, 2015
View reviewed changes

rprichard mentioned this pull request Apr 7, 2015

Provide context for macro expansions which result in unparsed tokens. #24078

Merged

rprichard force-pushed the span-fixes branch from b8de01c to 4629b51 Compare April 10, 2015 07:56

rprichard added 5 commits April 11, 2015 16:00

Expand internal-unstable to handle named field accesses and method ca…

a893c64

…lls.

Add compile-file/macro-backtrace-{invalid-internals,nested,println} t…

5a8f102

…ests

Suppress the macro backtrace for fileline_note and fileline_help.

fab3295

Remove the vestigial ExtCtxt::print_backtrace function.

ddbdf51

It was added in 2011-08-05 and reduced to a no-op ten days later.

rprichard force-pushed the span-fixes branch from 4629b51 to ddbdf51 Compare April 12, 2015 02:16

bors merged commit ddbdf51 into rust-lang:master Apr 12, 2015

bors mentioned this pull request Apr 12, 2015

Destabilize format_args! internals #24312

Merged

tamird mentioned this pull request Apr 22, 2015

writeln! and println! (and builtin macros’?) diagnostics are terrible #23459

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Propagate macro backtraces more often, improve formatting diagnostics #24003

Propagate macro backtraces more often, improve formatting diagnostics #24003

rprichard commented Apr 3, 2015

rust-highfive commented Apr 3, 2015

rprichard commented Apr 3, 2015

rprichard commented Apr 3, 2015

huonw commented Apr 3, 2015

rprichard commented Apr 3, 2015

rprichard commented Apr 4, 2015

rprichard commented Apr 5, 2015

huonw commented Apr 6, 2015

huonw Apr 6, 2015

rprichard Apr 7, 2015

huonw commented Apr 6, 2015

bors commented Apr 7, 2015

rprichard commented Apr 7, 2015

rprichard commented Apr 10, 2015

bors commented Apr 11, 2015

rprichard commented Apr 12, 2015

huonw commented Apr 12, 2015

bors commented Apr 12, 2015

bors commented Apr 12, 2015

bors commented Apr 12, 2015

Propagate macro backtraces more often, improve formatting diagnostics #24003

Propagate macro backtraces more often, improve formatting diagnostics #24003

Conversation

rprichard commented Apr 3, 2015

rust-highfive commented Apr 3, 2015

rprichard commented Apr 3, 2015

rprichard commented Apr 3, 2015

huonw commented Apr 3, 2015

rprichard commented Apr 3, 2015

rprichard commented Apr 4, 2015

rprichard commented Apr 5, 2015

huonw commented Apr 6, 2015

huonw Apr 6, 2015

Choose a reason for hiding this comment

rprichard Apr 7, 2015

Choose a reason for hiding this comment

huonw commented Apr 6, 2015

bors commented Apr 7, 2015

rprichard commented Apr 7, 2015

rprichard commented Apr 10, 2015

bors commented Apr 11, 2015

rprichard commented Apr 12, 2015

huonw commented Apr 12, 2015

bors commented Apr 12, 2015

bors commented Apr 12, 2015

bors commented Apr 12, 2015