`#[track_caller]` erroneously points to macro call

[c410-f3r]

use core::num::TryFromIntError;

#[derive(Debug)]
pub struct Error {
  pub column: u32,
  pub file: &'static str,
  pub line: u32,
  pub ty: ErrorTy,
}

#[derive(Debug)]
pub enum ErrorTy {
  BadCast(TryFromIntError),
}

impl From<TryFromIntError> for Error {
  #[inline]
  #[track_caller]
  fn from(from: TryFromIntError) -> Self {
    let location = core::panic::Location::caller();
    Self {
      column: location.column(),
      file: location.file(),
      line: location.line(),
      ty: ErrorTy::BadCast(from)
    }
  }
}

macro_rules! foo {
    () => {{
        let rslt: u8 = (-1i8).try_into()?;
        rslt
    }};
}

fn main() -> Result<(), Error> {
    let _ = foo!();
    Ok(())
}

Error::line points to line 38 (let _ = foo!();) but should in fact point to line 32 (let rslt: u8 = (-1i8).try_into()?;).

[c410-f3r]

Not sure if this behavior is intended.

@eddyb Do you have something to say about it? You reviewed most of the #[track_caller] implementation

[eddyb]

Not sure if this behavior is intended.

That it is - it's modeled after what you might get from (file!(), line!()), which, because they likely show up in a macro like panic!, need to point to the user code.

Nowadays, panic! calls a #[track_caller] function and yet again the panic! macro's definition must be skipped for the user's benefit.

I'm not sure how much we could improve this without somehow encoding all the levels of macro expansion in panic::Location (cc @anp).

[c410-f3r]

Thank you very much for the information, @eddyb

This issue is of my interest so please let me know if there is anything I can help

[anp]

should in fact point to line 32

I'm not sure it should, but that might be because I think of macros as a copy/paste of code before any function calls occur. #[track_caller] is an attribute for function ABIs, which macros don't have or participate in as used in this example.

panic! macro's definition must be skipped for the user's benefit

It's been a while since I've been in this code, but do we? I'm only seeing any related attributes on the internal functions, and I'm almost positive there's no compiler knowledge of the panic!() macro for the purposes of caller location (unless something's changed since my last involvement).

[c410-f3r]

Would you guys be open to support #[track_caller] on macro declarations?

#[track_caller]
macro_rules! foo { ... }

Or #[track_caller] on macro calls?

macro_rules! foo { ... }

fn main () {
    #[track_caller]
    foo!( ... )
}

[eddyb]

panic! macro's definition must be skipped for the user's benefit

It's been a while since I've been in this code, but do we? I'm only seeing any related attributes on the internal functions, and I'm almost positive there's no compiler knowledge of the panic!() macro for the purposes of caller location (unless something's changed since my last involvement).

What I mean is that those functions receive not the location inside panic!'s definition, but where the user invoked panic!. There is explicit logic to find the first Span up the macro expansion backtrace that is not inside a macro definition, regardless of which macros are involved.

That is, panic! is a macro, so we don't show its internals. If we started showing macro internals, panic! would need some attribute to get the old (i.e. current) behavior back (probably #[track_caller] heh).

---

@c410-f3r Are you also suggesting changing the default behavior (to not skip macros)? Because without a change, #[track_caller] on macros is effectively already the default.

[c410-f3r]

@c410-f3r Are you also suggesting changing the default behavior (to not skip macros)? Because without a change, #[track_caller] on macros is effectively already the default.

Honestly and personally, I am willing to accept anything that tracks function calls inside macros.

It is a "breaking change" to modify the current behavior, right? Even though #[track_caller] is not a library API, something like "Adding #[track_caller] on macro definitions will modify the output" could be used as a workaround.

But if it is OK to skip macro calls by default, then OK :)

Whatever solution is chosen (if chosen), I can try to implement it myself so that you guys won't have more stuff do to. Just need an overview of how things should be done.

[c410-f3r]

Kind bump

[c410-f3r]

Closing due to the lack of response. If you guys want me to deal with this situation with at least a brief mentorship, then feel free to ping me.

[eddyb]

Whatever solution is chosen (if chosen)

Sorry, didn't see this - I think this would need to be discussed in a larger context, but maybe RFC is overkill? I genuinely don't know the appropriate venue for something like this (as I'm not that active myself).
^{(Btw, due to how GH notifs work, I'm more likely to see something if @eddyb is included)}

Maybe someone from @rust-lang/lang can nominate this for further discussion?
To summarize the conundrum (which I don't know whether it needs an RFC or not):

• #[track_caller] was implemented to point to the outermost macro expansion, just like file!()/line!() did - this means that e.g. the panic! macro has to do nothing for its #[track_caller] callee to see the invocation site of panic!(...) instead of macro_rules! panic
• sometimes pointing at the macro might be beneficial, but if we make that behavior the default, then all the macros like panic! (and macros everywhere in the ecosystem using panic!, which didn't have to change for the #[track_caller] transition) will require an annotation on the macro declaration (could just be the #[track_caller] attribute) to indicate the "skip the macro" behavior should be used still
• theoretically we could make the macro annotation #[track_caller(skip_macro = false)] (bikeshedding, likely not the best syntax), and have that be an opt-in into the new behavior, keeping the existing (macro-skipping) one by default and breaking nothing

[c410-f3r]

@eddyb Thanks for the explanation