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

Using an out-of-scope trait item on a boxed trait object should suggest use-ing the trait #105159

Closed
shadowfacts opened this issue Dec 2, 2022 · 4 comments · Fixed by #105164
Closed
Assignees
Labels
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.

Comments

@shadowfacts
Copy link

shadowfacts commented Dec 2, 2022

Given the following code:

use std::io::{stdin, BufReader};

fn main() {
    let reader: Box<dyn std::io::BufRead> = Box::new(BufReader::new(stdin()));
    let _ = reader.lines();
}

The current output is:

error: the `lines` method cannot be invoked on a trait object
    --> src/main.rs:5:20
     |
5    |     let _ = reader.lines();
     |                    ^^^^^
     |
    ::: /Users/shadowfacts/.rustup/toolchains/nightly-aarch64-apple-darwin/lib/rustlib/src/rust/library/std/src/io/mod.rs:2276:15
     |
2276 |         Self: Sized,
     |               ----- this has a `Sized` requirement

Ideally the output should suggest use-ing the out-of-scope trait, similar to E0599:

error[E0599]: no method named `lines` found for struct `BufReader` in the current scope
    --> src/main.rs:6:20
     |
6    |     let _ = reader.lines();
     |                    ^^^^^ method not found in `BufReader<Stdin>`
     |
    ::: /Users/shadowfacts/.rustup/toolchains/nightly-aarch64-apple-darwin/lib/rustlib/src/rust/library/std/src/io/mod.rs:2274:8
     |
2274 |     fn lines(self) -> Lines<Self>
     |        ----- the method is available for `BufReader<Stdin>` here
     |
     = help: items from traits can only be used if the trait is in scope
help: the following trait is implemented but not in scope; perhaps add a `use` for it:
     |
1    | use std::io::BufRead;
     |

Someone tried to do dynamic dispatch between two underlying BufReaders used a boxed trait object (with the type spelled Box<dyn io::BufRead>, so BufRead wasn't in scope) and call the lines method on the boxed reader, and it was unclear from the error why the method wasn't accessible:
https://mastodon.social/@mcc/109441482880682856

This is reproducible on nightly (rustc 1.67.0-nightly (c090c68 2022-12-01)).

@shadowfacts shadowfacts 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 Dec 2, 2022
@mcclure
Copy link

mcclure commented Dec 2, 2022

Note: A sign there may be a problem worth fixing here is the code I wrote is a near copypaste of a top stack overflow answer for a common problem :)

@shadowfacts
Copy link
Author

So, after some investigating, this appears to be a regression of #35976, which added essentially the same suggestion I proposed here: adding a use for the trait object's trait.

When generating the error when the lookup fails, the trait is initially included in the list of candidates, but it's filtered out by this retain (because the callee is the trait method, and so the parent of the callee is the trait, which is the candidate):

candidates.retain(|candidate| *candidate != self.tcx.parent(result.callee.def_id));

This was introduced in 88f2140 by @compiler-errors, which also removed the corresponding suggestion from the #35976 test's expected output. It's not immediately clear to me from the PR (#99146) why this change was necessary. If the retain call is removed, the only UI test that fails is the #35976 one. Reverting that change would fix this issue, but I'm not sure if there are other cases I'm missing.

(cc @estebank, with whom I was discussing this on Mastodon)

@compiler-errors
Copy link
Member

Haven't look at this yet, maybe I misinterpreted what 88f4120 was achieving, and that commit just needs a revert 🤷 -- can't recall why I did that.

@compiler-errors
Copy link
Member

Yeah, lol, I totally overlooked that the use would be load-bearing due to the way we treat trait methods on dyn Trait as inherent. I'll revert 88f2140, modify the test, etc.

@compiler-errors compiler-errors self-assigned this Dec 2, 2022
@bors bors closed this as completed in a739fc8 Dec 4, 2022
Aaron1011 pushed a commit to Aaron1011/rust that referenced this issue Jan 6, 2023
…er, r=estebank

Restore `use` suggestion for `dyn` method call requiring `Sized`

Add the suggestion back that I accidentally removed in 88f2140 because I didn't understand that suggestion was actually useful...

Fixes rust-lang#105159
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 T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants