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

Disambiguate method call for impl on dyn Trait #69161

Open
haraldh opened this issue Feb 14, 2020 · 3 comments
Open

Disambiguate method call for impl on dyn Trait #69161

haraldh opened this issue Feb 14, 2020 · 3 comments
Labels
A-traits Area: Trait system C-bug Category: This is a bug. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@haraldh
Copy link
Contributor

haraldh commented Feb 14, 2020

pub trait Error: Debug + Display {
    fn source(&self) -> Option<&(dyn Error + 'static)> {
        None
    }

    fn chain(&self) -> Chain<'_> where Self: Sized + 'static {
        Chain {
            current: Some(self),
        }
    }
}

impl dyn Error + 'static {
    fn chain(&self) -> Chain<'_> {
        Chain {
            current: Some(self),
        }
    }
}

Seems like the compiler tries the trait method for an dyn Error + 'static even though it does not work, because of !Sized and does not consider taking the impl dyn Error + 'static.

Playground: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=a6845b426451f1d7a5105a9bd3a00499

error:

error[E0034]: multiple applicable items in scope
  --> src/main.rs:82:28
   |
82 |     let mut iter = dyn_err.chain();
   |                    --------^^^^^--
   |                    |       |
   |                    |       multiple `chain` found
   |                    help: disambiguate the method call for candidate #2: `Error::chain(&dyn_err)`
   |
note: candidate #1 is defined in an impl for the type `(dyn Error + 'static)`
  --> src/main.rs:16:5
   |
16 |     fn chain(&self) -> Chain<'_> {
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
note: candidate #2 is defined in the trait `Error`
  --> src/main.rs:8:5
   |
8  |     fn chain(&self) -> Chain<'_> where Self: Sized + 'static {
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@haraldh
Copy link
Contributor Author

haraldh commented Feb 14, 2020

Introducing a new trait gets the job done, though and the compiler will pick the right thing.
https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=682e14eddb0270a6787ab0259966f2d3

@jonas-schievink jonas-schievink added C-feature-request Category: A feature request, i.e: not implemented / a PR. T-lang Relevant to the language team, which will review and decide on the PR/issue. A-traits Area: Trait system labels Feb 14, 2020
haraldh added a commit to haraldh/rust-1 that referenced this issue Feb 18, 2020
Because `chain()` was only implemented for `dyn Error`, users had
to cast errors to a `&(dyn Error)` to use the `chain()` method.

```rust
    let mut iter = (&my_error as &(dyn Error)).chain();
    // or
    let mut iter = <dyn Error>::chain(&my_error);
    // or
    let mut iter = Error::chain(&my_error);
```

What I would liked to have is

```rust
pub trait Error: Debug + Display {
    fn source(&self) -> Option<&(dyn Error + 'static)> {
        None
    }

    fn chain(&self) -> Chain<'_> where Self: Sized + 'static {
        Chain {
            current: Some(self),
        }
    }
}

impl dyn Error + 'static {
    fn chain(&self) -> Chain<'_> {
        Chain {
            current: Some(self),
        }
    }
}
```

This doesn't work, because of:
rust-lang#69161

This patch introduces an ErrorChain trait, which accomplishes the
job and is implemented for `Error` and `dyn Error + 'static`.
@JelteF
Copy link
Contributor

JelteF commented Mar 21, 2020

I've been doing a bit of digging in the type checking code and I've found the core problem of this bug:

This piece of code that checks for illegal_sized_bound, should be checked somewhere in the consider_candidates (or one of the functions that it calls, such as consider_probe). It should be done before the Ambiguity error is returned.

I'm writing this down for two reasons:

  1. If someone with more experience or time to work on this wants to pick this up, then this might be useful to them. It also shows that the issue doesn't seem very complex to fix.
  2. If no-one does, then this way I don't have to figure it out next time I want to continue working on this.

@haraldh
Copy link
Contributor Author

haraldh commented Jun 25, 2020

Any chance anybody looking into this?

@estebank estebank added A-impl-trait Area: impl Trait. Universally / existentially quantified anonymous types with static dispatch. and removed A-impl-trait Area: impl Trait. Universally / existentially quantified anonymous types with static dispatch. labels Jul 3, 2020
haraldh added a commit to haraldh/rust-1 that referenced this issue Feb 3, 2021
To produce an error iterator `std::error::Chain` one had to call
`<dyn Error>::chain()`, which was not very ergonomic, because you have to
manually cast to a trait object, if you didn't already have one with
the type erased.

```
    let mut iter = (&my_error as &(dyn Error)).chain();
    // or
    let mut iter = <dyn Error>::chain(&my_error);
    // or
    let mut iter = Error::chain(&my_error);
```

The `chain()` method can't be implemented on the Error trait, because of
rust-lang#69161

`Chain::new()` replaces `<dyn Error>::chain()` as a good alternative
without confusing users, why they can't use `my_error.chain()` directly.

The `Error::sources()` method doesn't have this problem, so implement
it more efficiently, than one could achieve this with `Chain::new().skip(1)`.

Related: rust-lang#58520
@Dylan-DPC Dylan-DPC added C-bug Category: This is a bug. and removed C-feature-request Category: A feature request, i.e: not implemented / a PR. labels Dec 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-traits Area: Trait system C-bug Category: This is a bug. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

5 participants