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.

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

Add #[track_caller] support to ? #77474

Closed
yaahc opened this issue Oct 3, 2020 · 6 comments
Closed

Add #[track_caller] support to ? #77474

yaahc opened this issue Oct 3, 2020 · 6 comments
Labels
A-error-handling Area: Error handling C-feature-request Category: A feature request, i.e: not implemented / a PR. F-track_caller `#![feature(track_caller)]` PG-error-handling Project group: Error handling (https://github.com/rust-lang/project-error-handling)

Comments

@yaahc
Copy link
Member

yaahc commented Oct 3, 2020

Right now if you construct an error via its Into implementation as provided by the blanket implementation in std it gives the following location.

Location:
   /home/jlusby/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/convert/mod.rs:545

We should add #[track_caller] to this trait and possibily some of the Try trait implementations so that errors that attempt to track their construction point can correctly point to user code.

cc @anp

@jyn514 jyn514 added A-error-handling Area: Error handling C-feature-request Category: A feature request, i.e: not implemented / a PR. F-track_caller `#![feature(track_caller)]` PG-error-handling Project group: Error handling (https://github.com/rust-lang/project-error-handling) labels Oct 3, 2020
@BGR360
Copy link
Contributor

BGR360 commented Aug 27, 2021

For reference this idea was mentioned also in #87401 (comment)

@c410-f3r
Copy link
Contributor

c410-f3r commented Mar 20, 2022

Is there anything preventing this addition? If not, then I can create a PR

@BGR360
Copy link
Contributor

BGR360 commented Mar 21, 2022

@c410-f3r Since my last comment in August, there has been further discussion about this in various places. IIRC, @yaahc resolved to add #[track_caller] to something, I think it was Into? That change might already be in. I'll go hunting in my notification history to see if I can find what I'm talking about.

@BGR360
Copy link
Contributor

BGR360 commented Mar 21, 2022

It was to from_residual and it has been merged: #91752

Unless it has been since removed again, I think that means this issue can be closed?

@c410-f3r
Copy link
Contributor

Thanks @BGR360

Unfortunately, such change did not prevent an error I am currently facing that also points stuff to ... library/core/src/convert/mod.rs.

Capture d’écran de 2022-03-21 19-05-04

But this can be reported in another issue with an appropriated reproducible snippet

@yaahc
Copy link
Member Author

yaahc commented Mar 21, 2022

I was worried it would still be broken, I remember in the past it losing the location when it went through Into and I thought I remembered ? desugaring to use Into but it looks like it uses From, so it seems to work fine for my small snippet using eyre.

use thiserror::Error;

fn main() {
    let err = inner().unwrap_err();
    println!("Error: {:?}", err);
}

#[derive(Debug, Error)]
#[error("my error")]
struct MyError;

fn inner() -> eyre::Result<()> {
    Err(MyError)?
}

It might be that original issue I was referring to was just about explicitly calling into in which case you can still reproduce the issue and I get:

Location:
    /rustc/9d1b2106e23b1abd32fce1f17267604a5102f57a/library/core/src/convert/mod.rs:543:9

Which might be worth fixing, I'm not really sure. It would be a one line change and we'd need to perf test it but I'm not confident it would pass or be something we'd want to merge.

👍 @c410-f3r for opening a new issue for this specific issue. I'm gonna go ahead and close this one for now.

@yaahc yaahc closed this as completed Mar 21, 2022
bors added a commit to rust-lang-ci/rust that referenced this issue Jan 10, 2024
…to, r=<try>

Add `#[track_caller]` to the "From implies Into" impl

This pr implements what was mentioned in rust-lang#77474 (comment)

This follows from my URLO https://users.rust-lang.org/t/104497

```rust
#![allow(warnings)]
fn main() {
    // Gives a good location
    let _: Result<(), Loc> = dbg!(Err::<(), _>(()).map_err(|e| e.into()));

    // still doesn't work, gives location of `FnOnce::call_once()`
    let _: Result<(), Loc> = dbg!(Err::<(), _>(()).map_err(Into::into));
}

#[derive(Debug)]
pub struct Loc {
    pub l: &'static std::panic::Location<'static>,
}

impl From<()> for Loc {
    #[track_caller]
    fn from(_: ()) -> Self {
        Loc {
            l: std::panic::Location::caller(),
        }
    }
}
```
bors added a commit to rust-lang-ci/rust that referenced this issue Jan 21, 2024
…to, r=Nilstrieb

Add `#[track_caller]` to the "From implies Into" impl

This pr implements what was mentioned in rust-lang#77474 (comment)

This follows from my URLO https://users.rust-lang.org/t/104497

```rust
#![allow(warnings)]
fn main() {
    // Gives a good location
    let _: Result<(), Loc> = dbg!(Err::<(), _>(()).map_err(|e| e.into()));

    // still doesn't work, gives location of `FnOnce::call_once()`
    let _: Result<(), Loc> = dbg!(Err::<(), _>(()).map_err(Into::into));
}

#[derive(Debug)]
pub struct Loc {
    pub l: &'static std::panic::Location<'static>,
}

impl From<()> for Loc {
    #[track_caller]
    fn from(_: ()) -> Self {
        Loc {
            l: std::panic::Location::caller(),
        }
    }
}
```
github-actions bot pushed a commit to rust-lang/miri that referenced this issue Jan 23, 2024
…strieb

Add `#[track_caller]` to the "From implies Into" impl

This pr implements what was mentioned in rust-lang/rust#77474 (comment)

This follows from my URLO https://users.rust-lang.org/t/104497

```rust
#![allow(warnings)]
fn main() {
    // Gives a good location
    let _: Result<(), Loc> = dbg!(Err::<(), _>(()).map_err(|e| e.into()));

    // still doesn't work, gives location of `FnOnce::call_once()`
    let _: Result<(), Loc> = dbg!(Err::<(), _>(()).map_err(Into::into));
}

#[derive(Debug)]
pub struct Loc {
    pub l: &'static std::panic::Location<'static>,
}

impl From<()> for Loc {
    #[track_caller]
    fn from(_: ()) -> Self {
        Loc {
            l: std::panic::Location::caller(),
        }
    }
}
```
lnicola pushed a commit to lnicola/rust-analyzer that referenced this issue Apr 7, 2024
…strieb

Add `#[track_caller]` to the "From implies Into" impl

This pr implements what was mentioned in rust-lang/rust#77474 (comment)

This follows from my URLO https://users.rust-lang.org/t/104497

```rust
#![allow(warnings)]
fn main() {
    // Gives a good location
    let _: Result<(), Loc> = dbg!(Err::<(), _>(()).map_err(|e| e.into()));

    // still doesn't work, gives location of `FnOnce::call_once()`
    let _: Result<(), Loc> = dbg!(Err::<(), _>(()).map_err(Into::into));
}

#[derive(Debug)]
pub struct Loc {
    pub l: &'static std::panic::Location<'static>,
}

impl From<()> for Loc {
    #[track_caller]
    fn from(_: ()) -> Self {
        Loc {
            l: std::panic::Location::caller(),
        }
    }
}
```
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this issue Apr 27, 2024
…strieb

Add `#[track_caller]` to the "From implies Into" impl

This pr implements what was mentioned in rust-lang/rust#77474 (comment)

This follows from my URLO https://users.rust-lang.org/t/104497

```rust
#![allow(warnings)]
fn main() {
    // Gives a good location
    let _: Result<(), Loc> = dbg!(Err::<(), _>(()).map_err(|e| e.into()));

    // still doesn't work, gives location of `FnOnce::call_once()`
    let _: Result<(), Loc> = dbg!(Err::<(), _>(()).map_err(Into::into));
}

#[derive(Debug)]
pub struct Loc {
    pub l: &'static std::panic::Location<'static>,
}

impl From<()> for Loc {
    #[track_caller]
    fn from(_: ()) -> Self {
        Loc {
            l: std::panic::Location::caller(),
        }
    }
}
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-error-handling Area: Error handling C-feature-request Category: A feature request, i.e: not implemented / a PR. F-track_caller `#![feature(track_caller)]` PG-error-handling Project group: Error handling (https://github.com/rust-lang/project-error-handling)
Projects
None yet
Development

No branches or pull requests

4 participants