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.

Sign up for GitHub

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

Jump to bottom

Should Result::from_residual() be #[track_caller]? #89261

Closed
mcy opened this issue Sep 25, 2021 · 2 comments
Closed

Should Result::from_residual() be #[track_caller]? #89261

mcy opened this issue Sep 25, 2021 · 2 comments
Labels
F-track_caller `#![feature(track_caller)]` T-libs Relevant to the library team, which will review and decide on the PR/issue.

Comments

@mcy
Copy link
Contributor

mcy commented Sep 25, 2021

I tried this code:

use std::panic::Location;

#[derive(Debug)]
pub struct Traced<E> {
    inner: E,
    location: &'static Location<'static>,
}

impl<E> From<E> for Traced<E> {
    #[track_caller]
    fn from(inner: E) -> Self {
        Self { inner, location: Location::caller(), }
    }
}

fn square(x: u16) -> Result<u16, Traced<()>> {
    Ok(x.checked_mul(x).ok_or(())?)
}

pub fn main() {
    let x = 2000;
    println!("{:?}", square(x));
}

I expected to see this happen: Prints Err(..., location: <body of square()>).

Instead, this happened: Prints Err(..., location: <the guts of libcore>).

Godbolt: https://godbolt.org/z/s5YaT4Kxn

There is a philosophical question about whether this should work at all. After all, people going in via into() would get a location in the guts of convert.rs. I'm definitely curious about what people think... I feel like I'm abusing #[track_caller] somewhat here, so maybe we don't want to sanction this usage?

There's definitely a Pandora's box of stuff here if we do want it to work properly: for example, we would want Into to be #[track_caller] if the corresponding From is, too, but not otherwise!
@mcy mcy added the C-bug Category: This is a bug. label Sep 25, 2021
@the8472
Copy link
Member

the8472 commented Sep 26, 2021

See #88302 and #87401

@estebank estebank added F-track_caller `#![feature(track_caller)]` T-libs Relevant to the library team, which will review and decide on the PR/issue. and removed C-bug Category: This is a bug. labels Sep 29, 2021
@yaahc
Copy link
Member

yaahc commented Feb 8, 2022

fixed by #91752

@yaahc yaahc closed this as completed Feb 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
F-track_caller `#![feature(track_caller)]` T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@the8472 @estebank @yaahc @mcy