show macro backtrace with -Z flag by durka · Pull Request #45545 · rust-lang/rust

durka · 2017-10-26T04:59:07Z

Fixes#39413 by adding a facility to restore the "old school" macro expansion backtraces (previously removed in 6186538).

The restored functionality is accessed through the flag -Z external-macro-backtrace. Errors showing the truncated backtraces will suggest this flag.

Example

Code:

Details

a/src/lib.rs

#[macro_export]macro_rules! a {() => { a!(@)};(@) => { a!(@@)};(@@) => {
syntax error;}}

b/src/main.rs

#[macro_use]externcrate a;macro_rules! b {() => { b!(@)};(@) => { b!(@@)};(@@) => {
syntax error;}}fnmain(){a!();b!();}

Running without env var (note: first error is from remote macro, second from local macro):

Details

$ cargo +custom run
Compiling b v0.1.0
error: expected one of `!`, `.`, `::`, `;`, `?`, `{`, `}`, or an operator, found `error`
--> src/main.rs:12:5
|
12 | a!();
| ^^^^^
| |
| expected one of 8 possible tokens here
| unexpected token
|
= note: this error originates in a macro outside of the current crate (run with RUST_MACRO_BACKTRACE=1 for more info)
error: expected one of `!`, `.`, `::`, `;`, `?`, `{`, `}`, or an operator, found `error`
--> src/main.rs:7:16
|
7 | syntax error;
| -^^^^^ unexpected token
| |
| expected one of 8 possible tokens here
...
13 | b!();
| ----- in this macro invocation
error: aborting due to 2 previous errors
error: Could not compile `b`.
To learn more, run the command again with --verbose.

The output is the same as today, except for an addition to the note which aids discoverability of the new environment variable.

Running with env var:

Details

$ RUST_MACRO_BACKTRACE=1 cargo +custom run
Compiling b v0.1.0
error: expected one of `!`, `.`, `::`, `;`, `?`, `{`, `}`, or an operator, found `error`
--> <a macros>:1:72
|
1 | ( ) => { a ! ( @ ) } ; ( @ ) => { a ! ( @ @ ) } ; ( @ @ ) => { syntax error ;
| -^^^^^ unexpected token
| |
| expected one of 8 possible tokens here
src/main.rs:12:5: 12:10 note: in this expansion of a! (defined in <a macros>)
<a macros>:1:11: 1:20 note: in this expansion of a! (defined in <a macros>)
<a macros>:1:36: 1:47 note: in this expansion of a! (defined in <a macros>)
error: expected one of `!`, `.`, `::`, `;`, `?`, `{`, `}`, or an operator, found `error`
--> src/main.rs:7:16
|
7 | syntax error;
| -^^^^^ unexpected token
| |
| expected one of 8 possible tokens here
src/main.rs:12:5: 12:10 note: in this expansion of a! (defined in <a macros>)
<a macros>:1:11: 1:20 note: in this expansion of a! (defined in <a macros>)
<a macros>:1:36: 1:47 note: in this expansion of a! (defined in <a macros>)
error: expected one of `!`, `.`, `::`, `;`, `?`, `{`, `}`, or an operator, found `error`
--> src/main.rs:7:16
|
7 | syntax error;
| -^^^^^ unexpected token
| |
| expected one of 8 possible tokens here
src/main.rs:13:5: 13:10 note: in this expansion of b! (defined in src/main.rs)
src/main.rs:4:13: 4:18 note: in this expansion of b! (defined in src/main.rs)
src/main.rs:5:14: 5:20 note: in this expansion of b! (defined in src/main.rs)
error: aborting due to 2 previous errors
error: Could not compile `b`.
To learn more, run the command again with --verbose.

The output is hard to read, but better than nothing (and it's exactly what we used to have before the infamous fix_multispans_in_std_macros).

Wishlist:

Save the actual source of macros in crate metadata, not just AST, so the output can be improved
- Hopefully this would allow line numbers in the trace as well
Show the actual macro invocations in the traces

r? @nrc

oli-obk · 2017-10-26T08:21:02Z

Can you add a ui test?

durka · 2017-10-26T16:13:24Z

Sure, though I expect the output to change as this facility is improved and modernized. In particular, the error for the non-local macro is emitted twice (once from <a macros> with the AST-ified formatting, and once from src/main.rs) and I wouldn't mind if one of those was "fixed" to show the top-level macro invocation.

durka · 2017-10-27T04:52:10Z

Added test.

kennytm · 2017-10-29T08:20:57Z

I'm not convinced that this should be an environment variable instead of a -Z option (a -Z option can be feature-gated, the env var cannot).

durka · 2017-10-30T18:17:30Z

Well, we could just restrict the environment variable to only have an effect on nightly. (In general I think restricting useful tools for no reason is not helpful, especially since debugging macro expansion across compiler versions is a common occurrence, though I might feel differently if this were introducing new functionality instead of just restoring output that was removed.)

That being said, if -Z is the way to get this landed then I'll do it. Initially it seemed to me like a large amount of plumbing to add. I guess the way to go is:

Add a debugging option to the session
??? <--- this is the plumbing I can't figure out, how is the error emitter supposed to find out about the session options?
Add a macro_backtraces: bool flag to EmitterWriter and its constructor which is called here (but there is no Session at that point!)

shepmaster · 2017-11-03T14:11:57Z

ping @rust-lang/compiler — we haven't heard from @nrc in over 6 days, would you mind assigning a new reviewer?

shepmaster · 2017-11-11T01:02:57Z

Ok, @rust-lang/compiler — I'll roll a die for you:

r? @petrochenkov

petrochenkov · 2017-11-11T22:03:08Z

@durka

??? <--- this is the plumbing I can't figure out, how is the error emitter supposed to find out about the session options?

See #45205 for an example.
An option is added into src/librustc/session/config.rs and then used through session.opts.debugging_opts.my_option.

durka · 2017-11-15T05:50:53Z

@petrochenkov

So, the emitter seems to have no reference to any session or context. That's the problem. And there isn't a session to pass in when creating the EmitterWriter because run_compiler doesn't return one if it fails (see source links in my previous comment).

durka · 2017-11-15T05:57:36Z

Actually I think that call to EmitterWriter::new that I pinpointed is the wrong one, but I'm not sure where I should be looking. Searching for other places they are constructed, some have a session available and some do not.

petrochenkov · 2017-11-18T19:21:14Z

@durka
Unfortunately, I'm no more familiar with that infrastructure than you.

durka · 2017-11-19T22:34:19Z

I found a way to do it, but it required making a DiagnosticBuilder field public and a small update to rustfmt (ping @nrc durka/rustfmt@626ef4f). Please review.

nrc · 2017-11-20T02:22:47Z

 }
 }
+
+ fn render_macro_backtrace_old_school(&self,


old_school isn't very helpful here - what distinguishes it from a normal backtrace? expanded or complete or something might be better

Yeah, I just used the name from the commit I'm partially reverting. I think it looks a bit "old school" (it doesn't have fancy span notes or anything).

@durka would you be against using the full span machinery for them or would you think it'd be too verbose?

The more spans the better, probably. I'm afraid I'm not too familiar with the new span hardware. What would it look like?

Some changes will need to be made to the macro expansion machinery to avoid bogus spans (<a macros>:1:36: 1:47 note: in this expansion of a! (defined in <a macros>)), but I think it could be made to look something along the lines of:
error: expected one of `!`, `.`, `::`, `;`, `?`, `{`, `}`, or an operator, found `error` --> a/src/lib.rs:6:16 3 | () => { a!(@) }; | ----- in this expansion of `a!` (#2) 4 | (@) => { a!(@@) }; | ------ in this expansion of `a!` (#3) 5 | (@@) => { 6 | syntax error; | ^^^^^ expected one of 8 possible tokens here ::: src/main.rs | 12 | a!(); | ----- in this expansion of `a!` (#1) error: expected one of `!`, `.`, `::`, `;`, `?`, `{`, `}`, or an operator, found `error` --> src/main.rs:7:16 | 4 | () => { b!(@) }; | ----- in this expansion of `b!` (#2) 5 | (@) => { b!(@@) }; | ------ in this expansion of `b!` (#3) 6 | (@@) => { 7 | syntax error; | ^^^^^ expected one of 8 possible tokens here 13 | b!(); | ----- in this expansion of `b!` (#1) error: aborting due to 2 previous errors error: Could not compile `b`.

Well that mockup looks really good to me! :)

@durka suggestion implemented in #46605. The output is not perfect as it is pointing at the pretty printed expanded macro code instead of the actual code, but it is close enough :)

nrc · 2017-11-20T02:23:41Z

 "run all passes except translation; no output"),
 treat_err_as_bug: bool = (false, parse_bool, [TRACKED],
 "treat all errors that occur as bugs"),
+ macro_backtrace: bool = (false, parse_bool, [UNTRACKED],


could you use full_macro_backtrace or something? The name is a bit confusing as is because we get a macro backtrace in any case.

Sure. Or maybe external_macro_backtrace since you do get a full one for local macros?

sure, works for me

nrc · 2017-11-20T02:25:52Z

+ Handler::with_emitter(can_emit_warnings, treat_err_as_bug, macro_backtrace, emitter)
 }

 pub fn with_emitter(can_emit_warnings: bool,


Could you add another method rather than adding another bool flag to this one? That would be more self-documenting (bools tell the reader nothing), less susceptible to bugs (since it is easy enough to confuse the two bools here), and avoids a breaking change.

Do you mean a new method like Handler::with_backtracing_emitter(...)? We also thought of always initializing the flag to false and providing a setter. But that's kind of a weird mixture of constructor flags and setters which seems like it ought to be refactored into a settings struct anyway.

Maybe with_emitter_with_settings and take a settings struct to allow adding more flags in the future which are similarly detailed.

I'm calling it with_[tty_]emitter_and_flags and this should reduce the size of the patch / avoid breaking rustfmt. But we should still refactor users of with_emitter/with_tty_emitter and remove those at some point!

durka · 2017-11-20T18:06:58Z

Addressed comments.

nrc · 2017-11-21T01:00:01Z

@bors: r+

bors · 2017-11-21T01:00:02Z

📌 Commit b34a7ff has been approved by nrc

bors · 2017-11-21T06:42:21Z

⌛ Testing commit b34a7ff with merge bac7c53...

@nrc

show macro backtrace with -Z flag Fixes#39413 by adding a facility to restore the "old school" macro expansion backtraces (previously removed in 6186538). The restored functionality is accessed through the flag `-Z external-macro-backtrace`. Errors showing the truncated backtraces will suggest this flag. ### Example Code: <details> `a/src/lib.rs` ```rust #[macro_export] macro_rules! a { () => { a!(@) }; (@) => { a!(@@) }; (@@) => { syntax error; } } ``` `b/src/main.rs` ```rust #[macro_use] extern crate a; macro_rules! b { () => { b!(@) }; (@) => { b!(@@) }; (@@) => { syntax error; } } fn main() { a!(); b!(); } ``` </details> Running without env var (note: first error is from remote macro, second from local macro): <details> ``` $ cargo +custom run Compiling b v0.1.0 error: expected one of `!`, `.`, `::`, `;`, `?`, `{`, `}`, or an operator, found `error` --> src/main.rs:12:5 | 12 | a!(); | ^^^^^ | | | expected one of 8 possible tokens here | unexpected token | = note: this error originates in a macro outside of the current crate (run with RUST_MACRO_BACKTRACE=1 for more info) error: expected one of `!`, `.`, `::`, `;`, `?`, `{`, `}`, or an operator, found `error` --> src/main.rs:7:16 | 7 | syntax error; | -^^^^^ unexpected token | | | expected one of 8 possible tokens here ... 13 | b!(); | ----- in this macro invocation error: aborting due to 2 previous errors error: Could not compile `b`. To learn more, run the command again with --verbose. ``` </details> The output is the same as today, except for an addition to the note which aids discoverability of the new environment variable. Running _with_ env var: <details> ``` $ RUST_MACRO_BACKTRACE=1 cargo +custom run Compiling b v0.1.0 error: expected one of `!`, `.`, `::`, `;`, `?`, `{`, `}`, or an operator, found `error` --> <a macros>:1:72 | 1 | ( ) => { a ! ( @ ) } ; ( @ ) => { a ! ( @ @ ) } ; ( @ @ ) => { syntax error ; | -^^^^^ unexpected token | | | expected one of 8 possible tokens here src/main.rs:12:5: 12:10 note: in this expansion of a! (defined in <a macros>) <a macros>:1:11: 1:20 note: in this expansion of a! (defined in <a macros>) <a macros>:1:36: 1:47 note: in this expansion of a! (defined in <a macros>) error: expected one of `!`, `.`, `::`, `;`, `?`, `{`, `}`, or an operator, found `error` --> src/main.rs:7:16 | 7 | syntax error; | -^^^^^ unexpected token | | | expected one of 8 possible tokens here src/main.rs:12:5: 12:10 note: in this expansion of a! (defined in <a macros>) <a macros>:1:11: 1:20 note: in this expansion of a! (defined in <a macros>) <a macros>:1:36: 1:47 note: in this expansion of a! (defined in <a macros>) error: expected one of `!`, `.`, `::`, `;`, `?`, `{`, `}`, or an operator, found `error` --> src/main.rs:7:16 | 7 | syntax error; | -^^^^^ unexpected token | | | expected one of 8 possible tokens here src/main.rs:13:5: 13:10 note: in this expansion of b! (defined in src/main.rs) src/main.rs:4:13: 4:18 note: in this expansion of b! (defined in src/main.rs) src/main.rs:5:14: 5:20 note: in this expansion of b! (defined in src/main.rs) error: aborting due to 2 previous errors error: Could not compile `b`. To learn more, run the command again with --verbose. ``` </details> The output is hard to read, but better than nothing (and it's exactly what we used to have before the infamous `fix_multispans_in_std_macros`). Wishlist: - Save the actual source of macros in crate metadata, not just AST, so the output can be improved - Hopefully this would allow line numbers in the trace as well - Show the actual macro invocations in the traces r? @nrc

bors · 2017-11-21T09:13:28Z

☀️ Test successful - status-appveyor, status-travis
Approved by: nrc
Pushing bac7c53 to master...

kennytm · 2017-11-21T17:27:28Z

@durka That (run with -Z external-macro-backtrace for more info) message is going to show up in beta and stable channel?

durka · 2017-11-21T17:53:07Z

Argh. The gating causes one problem after another... Good point. Should I add "on nightly" to the message?

…

On Nov 21, 2017 12:28, "kennytm" ***@***.***> wrote: @durka <https://github.com/durka> That (run with -Z external-macro-backtrace for more info) message is going to show up in beta and stable channel? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#45545 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAC3n6sJw8jf4YaQCIWw_bQvN_NITwHeks5s4wgigaJpZM4QHAvq> .

kennytm · 2017-11-21T18:30:01Z

@durka Perhaps add a channel check (use UnstableFeatures::from_environment().is_nightly_build() or plumb through sess.opts.unstable_features) and don't emit it at all when in beta/stable. But then we need to figure out what to do with the UI tests after entering beta...

durka · 2017-11-21T19:08:03Z

It must be discoverable, that's the point of having a message. We've got users asking why there is nothing available on stable.

durka · 2017-11-21T19:12:44Z

The message for const fn also says "in Nightly you can do XYZ" so there's precedent.

kennytm · 2017-11-21T19:13:55Z

I'll file an issue for this. (Edit: #46167)

RReverser · 2017-11-28T20:17:59Z

Just wanted to say thanks for this - upgraded to nightly and it immediately helped me to debug an annoying error inside of nom macro. Hopefully this becomes stable someday.

cramertj · 2018-03-19T05:39:17Z

Is there a tracking issue for stabilizing this flag?

durka · 2018-03-19T14:29:11Z

Let's make one.

durka · 2018-03-19T14:29:30Z

In fact, should there be a tracking issue for every-Z flag?

nikomatsakis · 2018-03-19T20:33:14Z

@durka I don't think so. Many of them have no particular path to stabilization.

Aaron1011 · 2019-09-18T22:38:47Z

Was a tracking issue ever filed?

rust-highfive assigned nrc Oct 26, 2017

kennytm added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 26, 2017

rust-highfive assigned petrochenkov and unassigned nrc Nov 11, 2017

kennytm added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 12, 2017

durka changed the title ~~show macro backtrace with env var~~[WIP] show macro backtrace with env varNov 19, 2017

durka added 4 commits November 19, 2017 22:22

show macro backtrace with env var
c73bcf0

add UI test
bcd1fed

update UI tests
bec62c2

use -Z flag instead of env var
7a5a1f9

durka force-pushed the macro-backtrace branch from 3ffaaea to 7242fdcCompare November 19, 2017 22:33

durka changed the title ~~[WIP] show macro backtrace with env var~~[WIP] show macro backtrace with -Z flagNov 20, 2017

break rustfmt
28f9ffb

durka force-pushed the macro-backtrace branch from 7242fdc to 28f9ffbCompare November 20, 2017 01:01

durka changed the title ~~[WIP] show macro backtrace with -Z flag~~show macro backtrace with -Z flagNov 20, 2017

tidy 😡
30a6614

nrc reviewed Nov 20, 2017
View reviewed changes

address review comments
b34a7ff

kennytm added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 20, 2017

bors merged commit b34a7ff into rust-lang:masterNov 21, 2017

This was referenced Nov 21, 2017
Check //~ERROR comments in ui tests #46116
Merged
Make GlobalCtxt implement Sync #45912
Closed

kennytm mentioned this pull request Nov 21, 2017
Add test for #44953 #45635
Merged

kennytm mentioned this pull request Nov 21, 2017
The -Z external-macro-backtrace message will appear in the stable channel #46167
Closed

This was referenced Dec 7, 2017
a type error in a nested macro leads to a confusing compiler error message #45921
Closed
Use spans for -Z external-macro-backtrace #46605
Merged

dhardy mentioned this pull request Sep 18, 2018
Make it easy to use external-macro-backtracerust-lang/cargo#6049
Closed

Aaron1011 mentioned this pull request Feb 13, 2021
Tracking Issue for -Z macro-backtrace#82068
Open

Uh oh!

Uh oh!

Conversation

durka commented Oct 26, 2017• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Example

Uh oh!

oli-obk commented Oct 26, 2017

Uh oh!

durka commented Oct 26, 2017

Uh oh!

durka commented Oct 27, 2017

Uh oh!

kennytm commented Oct 29, 2017

Uh oh!

durka commented Oct 30, 2017• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

shepmaster commented Nov 3, 2017

Uh oh!

shepmaster commented Nov 11, 2017

Uh oh!

petrochenkov commented Nov 11, 2017

Uh oh!

durka commented Nov 15, 2017• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

durka commented Nov 15, 2017

Uh oh!

petrochenkov commented Nov 18, 2017

Uh oh!

durka commented Nov 19, 2017• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

durkaNov 20, 2017• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

durkaNov 20, 2017• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

durka commented Nov 20, 2017

Uh oh!

nrc commented Nov 21, 2017

Uh oh!

bors commented Nov 21, 2017

Uh oh!

bors commented Nov 21, 2017

Uh oh!

bors commented Nov 21, 2017

Uh oh!

kennytm commented Nov 21, 2017

Uh oh!

durka commented Nov 21, 2017 via email

Uh oh!

durka commented Oct 26, 2017•
edited
Loading

durka commented Oct 30, 2017•
edited
Loading

durka commented Nov 15, 2017•
edited
Loading

durka commented Nov 19, 2017•
edited
Loading

durkaNov 20, 2017•
edited
Loading

durkaNov 20, 2017•
edited
Loading

kennytm commented Nov 21, 2017•
edited
Loading