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

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

Closed
8051Enthusiast opened this issue Oct 12, 2021 · 9 comments
Assignees
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` D-invalid-suggestion Diagnostics: A structured suggestion resulting in incorrect code. D-verbose Diagnostics: Too much output caused by a single piece of incorrect code. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@8051Enthusiast
Copy link

8051Enthusiast commented Oct 12, 2021

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.

@8051Enthusiast 8051Enthusiast added A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 12, 2021
@moxian
Copy link
Contributor

moxian commented Oct 12, 2021

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
Copy link
Contributor

PatchMixolydic commented Oct 12, 2021

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

@estebank
Copy link
Contributor

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).

@estebank estebank added D-verbose Diagnostics: Too much output caused by a single piece of incorrect code. A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` D-invalid-suggestion Diagnostics: A structured suggestion resulting in incorrect code. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. labels Oct 12, 2021
@yuvaldolev
Copy link
Contributor

@rustbot claim

@yuvaldolev
Copy link
Contributor

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

@JakobDegen
Copy link
Contributor

JakobDegen commented Oct 23, 2021

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
Copy link
Contributor

estebank commented Oct 24, 2021

@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
Copy link
Contributor

@estebank Thanks!

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Oct 30, 2021
…agnostic, r=estebank

Skipping verbose diagnostic suggestions when calling .as_ref() on type not implementing AsRef

Addresses rust-lang#89806

Skipping suggestions when calling `.as_ref()` for types that do not implement the `AsRef` trait.

r? `@estebank`
@pitaj
Copy link
Contributor

pitaj commented Dec 11, 2021

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

@jyn514 jyn514 closed this as completed Dec 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` D-invalid-suggestion Diagnostics: A structured suggestion resulting in incorrect code. D-verbose Diagnostics: Too much output caused by a single piece of incorrect code. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

8 participants