rust-lang/rust

panic! source location information does not account for macro expansion

gnzlbg opened this issue · 2 comments

Minimal working example (playground):

macro_rules! foo {
    ($e:expr) => {
        bar!($e);
        baz!($e);
    }
}
macro_rules! bar {
    ($e:expr) => { assert!($e); }  // line 8
}
macro_rules! baz {
    ($e:expr) => { assert!(!$e); }  // line 11
}

fn main() {
    foo!(true);  // line 15
}

produces the following run-time error:

thread 'main' panicked at 'assertion failed: !true', src/main.rs:15:5
note: Run with `RUST_BACKTRACE=1` for a backtrace.

Notice how the panic! points at line 15, but the error happens at line 11.

Enabling RUST_BACKTRACE=1 does not help:

stack backtrace:
   0: std::sys::unix::backtrace::tracing::imp::unwind_backtrace
             at libstd/sys/unix/backtrace/tracing/gcc_s.rs:49
   1: std::sys_common::backtrace::print
             at libstd/sys_common/backtrace.rs:71
             at libstd/sys_common/backtrace.rs:59
   2: std::panicking::default_hook::{{closure}}
             at libstd/panicking.rs:211
   3: std::panicking::default_hook
             at libstd/panicking.rs:227
   4: std::panicking::rust_panic_with_hook
             at libstd/panicking.rs:463
   5: std::panicking::begin_panic
             at /checkout/src/libstd/panicking.rs:397
   6: playground::main
             at src/main.rs:15
   7: std::rt::lang_start::{{closure}}
             at /checkout/src/libstd/rt.rs:74
   8: std::panicking::try::do_call
             at libstd/rt.rs:59
             at libstd/panicking.rs:310
   9: __rust_maybe_catch_panic
             at libpanic_unwind/lib.rs:105
  10: std::rt::lang_start_internal
             at libstd/panicking.rs:289
             at libstd/panic.rs:374
             at libstd/rt.rs:58
  11: std::rt::lang_start
             at /checkout/src/libstd/rt.rs:74
  12: main
  13: __libc_start_main
  14: _start

It would be better for assert! and similar macros to not construct the error message from a file:line:col, but to use something better instead (like a macro-expansion API).

A much more useful error message would have been:

thread 'main' panicked at 'assertion failed: !true', 
  src/main.rs:15:5: foo!(true);
  src/main.rs:11:19: assert!(!$e); 
note: Run with `RUST_BACKTRACE=1` for a backtrace.

So this seems to be a general issue with panic! (playground):

macro_rules! foo {
    ($e:expr) => {
        bar!($e);
        baz!($e);
    }
}
macro_rules! bar {
    ($e:expr) => { if !$e { panic!(); } }
}
macro_rules! baz {
    ($e:expr) => { if $e { panic!(); }  }
}

fn main() {
    foo!(true);
}

produces:

thread 'main' panicked at 'explicit panic', src/main.rs:15:5

If I make an error in the macro (playground), I get the following backtrace with -Z external-macro-backtrace:

error[E0277]: cannot multiply `{integer}` to `{float}`
  --> src/lib.rs:11:32
   |
1  | / macro_rules! foo {
2  | |     ($e:expr) => {
3  | |         bar!($e);
4  | |         baz!($e);
   | |         --------- in this macro invocation
5  | |     }
6  | | }
   | |_- in this expansion of `foo!`
...
10 | / macro_rules! baz {
11 | |     ($e:expr) => { if $e + 0.1 * 2 { panic!(); }  }
   | |                                ^ no implementation for `{float} * {integer}`
12 | | }
   | |_- in this expansion of `baz!`
...
15 |       foo!(true);
   |       ----------- in this macro invocation
   |
   = help: the trait `std::ops::Mul<{integer}>` is not implemented for `{float}`

So the information for generating a better source location for panic!s invoked inside macro expansion is already available somewhere.


So currently core::panicking::panic takes a tuple (file_id, line, col). We could extend that to a slice &[(file_id, line, col)] so that panic can print multiple locations.

Since there are code-size and compile-time trade-off, we could initially just track macro expansion and generate more accurate panic error messages in debug builds, while on release builds we could continue to use a single source location.

This not only affects the panic message but also the location reported by gdb, which is a bit confusing. Also it makes debugging in multi-line macros (like the foo macro above) quite cumbersome, because you cannot set breakpoints properly.