rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
98.13k stars 12.69k forks source link

Overly verbose diagnostic when calling .as_ref() on type not implementing AsRef #89806

Closed 8051Enthusiast closed 2 years ago

8051Enthusiast commented 3 years ago

Following code seems to lead to overly long diagnostics: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&code=fn%20main()%20%7B%0A%20%20%20%200u8.as_ref()%0A%7D

fn main() {
    0u8.as_ref()
}

The current output is:

error[E0599]: no method named `as_ref` found for type `u8` in the current scope
   --> b.rs:2:9
    |
2   |     0u8.as_ref()
    |         ^^^^^^ method not found in `u8`
    |
   ::: ~/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/convert/mod.rs:159:8
    |
159 |     fn as_ref(&self) -> &T;
    |        ------
    |        |
    |        the method is available for `Box<u8>` here
    |        the method is available for `Arc<u8>` here
    |        the method is available for `Rc<u8>` here
    |        the method is available for `Box<&mut u8>` here
    |        the method is available for `Arc<&mut u8>` here
    |        the method is available for `Rc<&mut u8>` here
    |        the method is available for `Box<&u8>` here
    |        the method is available for `Arc<&u8>` here
    |        the method is available for `Rc<&u8>` here
    |
   ::: ~/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/pin.rs:584:12
    |
584 |     pub fn as_ref(&self) -> Pin<&P::Target> {
    |            ------
    |            |
    |            the method is available for `Pin<&mut u8>` here
    |            the method is available for `Pin<&u8>` here
    |
help: consider wrapping the receiver expression with the appropriate type
    |
2   |     Box::new(0u8).as_ref()
    |     ^^^^^^^^^   ^
help: consider wrapping the receiver expression with the appropriate type
    |
2   |     Arc::new(0u8).as_ref()
    |     ^^^^^^^^^   ^
help: consider wrapping the receiver expression with the appropriate type
    |
2   |     Rc::new(0u8).as_ref()
    |     ^^^^^^^^   ^
help: consider wrapping the receiver expression with the appropriate type
    |
2   |     Box::new(&mut 0u8).as_ref()
    |     ^^^^^^^^^^^^^    ^
help: consider wrapping the receiver expression with the appropriate type
    |
2   |     Pin::new(&mut 0u8).as_ref()
    |     ^^^^^^^^^^^^^    ^
help: consider wrapping the receiver expression with the appropriate type
    |
2   |     Arc::new(&mut 0u8).as_ref()
    |     ^^^^^^^^^^^^^    ^
help: consider wrapping the receiver expression with the appropriate type
    |
2   |     Rc::new(&mut 0u8).as_ref()
    |     ^^^^^^^^^^^^    ^
help: consider wrapping the receiver expression with the appropriate type
    |
2   |     Box::new(&0u8).as_ref()
    |     ^^^^^^^^^^   ^
help: consider wrapping the receiver expression with the appropriate type
    |
2   |     Pin::new(&0u8).as_ref()
    |     ^^^^^^^^^^   ^
help: consider wrapping the receiver expression with the appropriate type
    |
2   |     Arc::new(&0u8).as_ref()
    |     ^^^^^^^^^^   ^
help: consider wrapping the receiver expression with the appropriate type
    |
2   |     Rc::new(&0u8).as_ref()
    |     ^^^^^^^^^   ^

Note that this happens not just with 0u8 but with most types not implementing AsRef as far as I can see.

moxian commented 3 years ago

This regressed between 1.52.0 and 1.53.0. Bisection gives:

Regression in nightly-2021-04-09
<...>
found 8 bors merge commits in the specified range
  commit[0] 2021-04-07UTC: Auto merge of #82451 - jyn514:defaults, r=Mark-Simulacrum
  commit[1] 2021-04-08UTC: Auto merge of #83986 - Dylan-DPC:rollup-51vygcj, r=Dylan-DPC
  commit[2] 2021-04-08UTC: Auto merge of #82958 - camelid:res-docs, r=petrochenkov
  commit[3] 2021-04-08UTC: Auto merge of #83866 - jyn514:disambiguator-error, r=camelid
  commit[4] 2021-04-08UTC: Auto merge of #83981 - nagisa:nagisa/revert-cfg-wasm, r=Mark-Simulacrum
  commit[5] 2021-04-08UTC: Auto merge of #83500 - camelid:split-debuginfo-docs-cleanup, r=steveklabnik
  commit[6] 2021-04-08UTC: Auto merge of #83763 - alexcrichton:wasm-multivalue-abi, r=nagisa
  commit[7] 2021-04-08UTC: Auto merge of #84008 - Dylan-DPC:rollup-invxvg8, r=Dylan-DPC

it's very likely #83689 . cc @estebank

PatchMixolydic commented 3 years ago

Seems similar to #84769 (though this probably isn't a duplicate since that issue doesn't appear to have regressed).

estebank commented 3 years ago

We need to filter traits with blacket impls on all of these types, I guess. We can also get away with a check for "oh, we're suggesting all of these? it's likely wrong" and silence them that way (with a length check or something similarly simple).

yuvaldolev commented 3 years ago

@rustbot claim

yuvaldolev commented 3 years ago

Hi @estebank I'm new to the rustc compiler. Where should I start looking for a place to perform these checks? Thanks!

JakobDegen commented 3 years ago

These suggestions show up here as well

struct S;

impl std::convert::TryInto<i32> for S {
    type Error = ();
    fn try_into(self) -> Result<i32, Self::Error> {
        Err(())
    }
}

mod out_of_scope {
    pub trait TryInto {
        fn try_into(self) -> Result<i32, ()>;
    }

    impl TryInto for super::S {
        fn try_into(self) -> Result<i32, ()> {
            Err(())
        }
    }
}

fn err(a: S) -> i32 {
    a.try_into().unwrap()
}

Yielding an output of

error[E0599]: no method named `try_into` found for struct `S` in the current scope
   --> test.rs:25:7
    |
3   | struct S(i32);
    | -------------- method `try_into` not found for this
...
25  |     a.try_into().unwrap()
    |       ^^^^^^^^ method not found in `S`
    |
   ::: ~/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/convert/mod.rs:399:8
    |
399 |     fn try_into(self) -> Result<T, Self::Error>;
    |        --------
    |        |
    |        the method is available for `Box<S>` here
    |        the method is available for `Pin<S>` here
    |        the method is available for `Arc<S>` here
    |        the method is available for `Rc<S>` here
    |
    = help: items from traits can only be used if the trait is in scope
help: consider wrapping the receiver expression with the appropriate type
    |
25  |     Box::new(a).try_into().unwrap()
    |     +++++++++ +
help: consider wrapping the receiver expression with the appropriate type
    |
25  |     Pin::new(a).try_into().unwrap()
    |     +++++++++ +
help: consider wrapping the receiver expression with the appropriate type
    |
25  |     Arc::new(a).try_into().unwrap()
    |     +++++++++ +
help: consider wrapping the receiver expression with the appropriate type
    |
25  |     Rc::new(a).try_into().unwrap()
    |     ++++++++ +
help: the following traits are implemented but not in scope; perhaps add a `use` for one of them:
    |
3   | use crate::outside::TryInto;
    |
3   | use std::convert::TryInto;
    |

error: aborting due to previous error

For more information about this error, try `rustc --explain E0599`.

Here they would not fix the actual issue, and introduce a new one if incorporated. That being said, this is probably not a big deal since I can't reproduce this in more realistic scenarios where someone is not defining their own TryInto.

estebank commented 3 years ago

@yuvaldolev the code is being suggested here:

https://github.com/estebank/rust/blob/6b64202d5eebdaddcc246412a795ec32dac0f8f2/compiler/rustc_typeck/src/check/method/suggest.rs#L988-L1070

For the case in the comment above, we might want to delay emitting a suggestion until we've confirmed that importing a trait wouldn't be enough to fix the problem. For the original report... we might want to check whether the trait we've found doesn't have a blanket impl, because those would otherwise always suggest the wrapper types, and would be always wrong. We try to do that with a deny list for specific traits, which we can extend to include AsRef and call it a day for the purposes of this ticket, but ideally we would make it so that all blanket impls (impl<T> Foo for T where T: Bar) even those defined by non-std crates are ignored.

yuvaldolev commented 3 years ago

@estebank Thanks!

pitaj commented 2 years ago

Shouldn't this be closed now that the relevant PR is merged?