Skip to content

rustc should provide a suggestion when using a type/trait from a sibling module that isn't imported #21221

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

Closed
fitzgen opened this issue Jan 16, 2015 · 20 comments
Labels
A-diagnostics Area: Messages for errors, warnings, and lints E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.

Comments

@fitzgen
Copy link
Member

fitzgen commented Jan 16, 2015

Here is two examples in one, a trait and a type:

/Users/fitzgen/src/oxischeme/src/eval.rs:267:6: 267:11 error: attempt to implement a nonexistent trait `Trace`
/Users/fitzgen/src/oxischeme/src/eval.rs:267 impl Trace for Meaning {
                                                  ^~~~~
/Users/fitzgen/src/oxischeme/src/eval.rs:268:24: 268:35 error: use of undeclared type name `IterGcThing`
/Users/fitzgen/src/oxischeme/src/eval.rs:268     fn trace(&self) -> IterGcThing {
                                                                    ^~~~~~~~~~~

It would be nice UX to say something like "did you forget to import module::Type?" or something.

@fitzgen
Copy link
Member Author

fitzgen commented Jan 16, 2015

Sorry, forgot to say: Trace and IterGcThing are both defined in a sibling module.

@kmcallister kmcallister added A-diagnostics Area: Messages for errors, warnings, and lints E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. labels Jan 16, 2015
@kmcallister
Copy link
Contributor

Perhaps it should print a list of modules in the local crate that define something with that name.

@fitzgen
Copy link
Member Author

fitzgen commented Jan 16, 2015

Perhaps it should print a list of modules in the local crate that define something with that name.

That would be wonderful.

@fitzgen
Copy link
Member Author

fitzgen commented Jan 16, 2015

@kmcallister You marked this easy, can you give me an idea of what might be involved plus some pointers to the relevant code? If it is simple enough, I might be able to knock this out.

@jdm
Copy link
Contributor

jdm commented Jan 16, 2015

Sounds similar to #21008.

@yati-sagade
Copy link

If this is still open (I don't see suggestions in the beta channel), I'd like to take a shot.

@VladUreche
Copy link
Contributor

Since it doesn't seem like there's much activity around this bug, let me try to revive it. Please correct me if this is not the desired behavior:

mod mul1 {
    pub trait Mul {}
}

mod mul2 {
    pub trait Mul {}
}

#[derive(Debug)]
struct Foo;

// When we comment the next line:
use mul1::Mul;

// We get the following error for the `impl` below:
//   error: use of undeclared trait name `Mul` [E0405]
// Ideally, when this bug is fixed, we would like to see:
//   error: use of undeclared trait name `Mul` [E0405]
//   note: Possible alternatives are:
//         use mul1::Mul;        // from module mul1 in this crate <or>
//         use mul2::Mul;        // from module mul1 in this crate <or>
//         use std::ops::Mul;    // from the standard library <or>
//         use other_crate::Mul; // from external crate other_crate ...
impl Mul for Foo { }

fn main() {
    let foo = Foo;
    println!("Hello, {:?}!", foo);
}

So, I imagine, in the ideal case, rustc would look up all the traits within the following contexts:

  • modules defined by the current crate
  • the standard library
  • external crates

Right?

As for the implementation, I imagine I can start from the suggest_traits_to_import method in suggest.rs. WDYT?

@nikomatsakis
Copy link
Contributor

I feel like changing the message to "trait name Mul is not a scope" would already be an improvement. To me, "undeclared" suggests something stronger (that is, not declared anywhere).

@nikomatsakis
Copy link
Contributor

But yes if we wanted to all out and give suggestions, I think the code you pointed out would be a logical starting point, though of course it is harvesting all traits that have a method with the given name, which is somewhat more complicated.

@VladUreche
Copy link
Contributor

Thank you for the feedback @nikomatsakis -- if all else fails, I will PR an update to the error message.

Outdated:

I've been following the logical thread from the trait suggestion, but I need some help:

  • the trait/method search in suggest.rs is done during typechecking, in the librustc_typeck crate, and ultimately relies on the typechecker context, through the CrateCtxt struct.
  • the error in this issue is generated during name resolution, in the librustc_resolve crate, which, as far as I can tell runs before the type checker and doesn't have a typer context.

The resolver also has a trait_map value, but printing its contents when the error occurs produces empty vectors:

{39: [], 36: []}

The output comes from the test case above. As expected, there are two node ids, but there are no trait definitions attached.

In this context, I have three questions:

  • Is this description accurate, or have I missed something?
  • Is it possible to access all traits in the crate during name resolution?
  • Any pointers as to how I can achieve this?

Thanks!

PS: Is there any faster way to prototype changes faster than make rustc-stage1? I explored stripping the commands out of the makefile here, but ran into a dead end.

@VladUreche
Copy link
Contributor

Some progress on this: VladUreche@f4abc9d

@VladUreche
Copy link
Contributor

Okay, sloppy implementation, but I have a solution:

bug-21221.rs:22:6: 22:9 error: use of undeclared trait name `Mul`. 
Are you looking for `mul1::Mul` or `mul2::Mul` from this crate? 
Or maybe `Mul` from another crate? [E0405]
bug-21221.rs:22 impl Mul for Foo { 
                     ^~~
bug-21221.rs:22:6: 22:9 help: run `rustc --explain E0405` to see a
detailed explanation

Commit is VladUreche@9a4c9e5.

@VladUreche
Copy link
Contributor

The last commit adds recommendations for both resolution errors in the bug description and the new message is more comprehensible, IMO: master...VladUreche:issue/21221

@fitzgen @nikomatsakis are you okay with the ouput for this example?

@nikomatsakis
Copy link
Contributor

Vlad, I left some comments on the commit. I didn't read it too closely but
at first glance it seems like a reasonable approach to me. My main
hesitation is that the name resolution code is expecting somewhat major
upheaval, and having to support a more complex error message like this
would make this harder -- but not a lot harder, probably.

On Sun, Feb 14, 2016 at 9:21 PM, Vlad Ureche [email protected]
wrote:

The last commit adds recommendations for both resolution errors in the bug
description and the new message is more comprehensible, IMO: VladUreche@
e11a5ff
VladUreche@e11a5ff

@fitzgen https://github.com/fitzgen @nikomatsakis
https://github.com/nikomatsakis are you okay with the result
https://github.com/VladUreche/rust/blob/e11a5ffe4643a29ce68915d97deab80d8920833b/src/test/compile-fail/issue-21221.rs?
Btw, is that the correct way to test a multi-line error message?


Reply to this email directly or view it on GitHub
#21221 (comment).

@VladUreche
Copy link
Contributor

@nikomatsakis Thank you for the feedback! All your comments should be addressed by the latest version of this commit, VladUreche@5f1b4a4, where I had a chance to clean things up.

For the burden of maintaining the additional lookup method, it makes sense to delay the PR (#31674) if there's a major refactoring going on. On my side, I can port the code over once the dust settles, just ping me.

@nikomatsakis
Copy link
Contributor

cc @jseyfried @petrochenkov, resident resolve experts :)

@nikomatsakis
Copy link
Contributor

oops!

@VladUreche
Copy link
Contributor

Thanks @nikomatsakis!
Hi @jseyfried and @petrochenkov -- the PR for this bug is here: #31674

VladUreche added a commit to VladUreche/rust that referenced this issue Feb 19, 2016
This commit adds functionality that allows the name resolution pass
to search for entities (traits/types/enums/structs) by name, in
order to show recommendations along with the errors.

For now, only E0405 and E0412 have suggestions attached, as per the
request in bug rust-lang#21221, but it's likely other errors can also benefit
from the ability to generate suggestions.
bors added a commit that referenced this issue Feb 20, 2016
This commit adds functionality that allows the name resolution pass
to search for entities (traits/types/enums/structs) by name, in
order to show recommendations along with the errors.

For now, only E0405 and E0412 have suggestions attached, as per the
request in bug #21221, but it's likely other errors can also benefit
from the ability to generate suggestions.
@VladUreche
Copy link
Contributor

@nikomatsakis @jseyfried Should we close this bug, or do you want to keep it open for the extensions that could be added?

@jseyfried
Copy link
Contributor

I think we should close it

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 E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.
Projects
None yet
Development

No branches or pull requests

7 participants