Stream: t-compiler

Topic: general edition work


nikomatsakis (Sep 14 2018 at 14:48, on Zulip):

@davidtwco so I noticed this the other day https://github.com/rust-lang/rust/issues/54230 and I was thinking this would be a nice thing to do for the 2018 release to help people transition

nikomatsakis (Sep 14 2018 at 14:49, on Zulip):

I'm not sure if there's another issue about it

nikomatsakis (Sep 14 2018 at 14:49, on Zulip):

anyway, thought you might enjoy it if you are looking for something different

davidtwco (Sep 14 2018 at 14:53, on Zulip):

Sounds good, I'll assign and take a look.

nikomatsakis (Sep 14 2018 at 14:54, on Zulip):

if we don't want to be too smart, this is real easy

nikomatsakis (Sep 14 2018 at 14:54, on Zulip):

I guess that .. ideally .. we might also compare against possible crate names?

nikomatsakis (Sep 14 2018 at 14:54, on Zulip):

well, best thing seems like it would be to compare against names in the root + possible crate names, but the former might be tricky to enumerate; we probably don't have a "complete" list at the time. But we could get an "incomplete" list relatively easily

nikomatsakis (Sep 14 2018 at 14:55, on Zulip):

should probably ping @eddyb or @Vadim Petrochenkov for thoughts

nikomatsakis (Sep 14 2018 at 14:56, on Zulip):

(done)

eddyb (Sep 14 2018 at 15:22, on Zulip):

wait, what list of crates?

nikomatsakis (Sep 14 2018 at 15:23, on Zulip):

from the prelude?

eddyb (Sep 14 2018 at 15:23, on Zulip):

one thing that I'm not sure everyone's on board with: migration code should remove "extern crate" only when redundant with extern_prelude crates

nikomatsakis (Sep 14 2018 at 15:24, on Zulip):

if we see use foo::bar::baz and foo is not valid, we should try to help the user find things they might have meant, is what I am saying. I am particularly interested in the case of helping people overcome 2015 "muscle memory", but it's probably good to also help people by suggesting crates.

nikomatsakis (Sep 14 2018 at 15:24, on Zulip):

one thing that I'm not sure everyone's on board with: migration code should remove "extern crate" only when redundant with extern_prelude crates

define "redundant"?

nikomatsakis (Sep 14 2018 at 15:24, on Zulip):

I think I'm on board with that...

nikomatsakis (Sep 14 2018 at 15:24, on Zulip):

there are some weird cases where you could re-write to use crate_name; I guess

eddyb (Sep 14 2018 at 15:25, on Zulip):

I mean, you can't remove extern crate rustc

eddyb (Sep 14 2018 at 15:25, on Zulip):

but you can remove extern crate cargodep

nikomatsakis (Sep 14 2018 at 15:25, on Zulip):

yes ok

nikomatsakis (Sep 14 2018 at 15:26, on Zulip):

we're too aggressive now in other words, because we don't check that

nikomatsakis (Sep 14 2018 at 15:26, on Zulip):

(seems right)

eddyb (Sep 14 2018 at 15:27, on Zulip):

well, if we don't check that. idk

davidtwco (Sep 16 2018 at 19:07, on Zulip):

@nikomatsakis I added a comment to the issue: at some point the error has changed and it does now make a suggestion. Is this still something we want to look at?

davidtwco (Sep 17 2018 at 15:21, on Zulip):

@nikomatsakis not sure if I was ambigious in the comment: that error was changed without my doing anything - some other PR landed it. So I'm not sure how much there is to do for that issue anymore.

nikomatsakis (Sep 17 2018 at 15:23, on Zulip):

I don't think it's a very good suggestion

nikomatsakis (Sep 17 2018 at 15:23, on Zulip):

(personally)

nikomatsakis (Sep 17 2018 at 15:23, on Zulip):

but it's not wrong

nikomatsakis (Sep 17 2018 at 15:24, on Zulip):

that said

nikomatsakis (Sep 17 2018 at 15:24, on Zulip):

if we modify the test slightly...

davidtwco (Sep 17 2018 at 15:24, on Zulip):

If there are improvements you'd like to see made then I can still work on doing that. I just noticed that the error had changed while I was digging around at the weekend and wasn't sure if there was still work to be done as a result.

nikomatsakis (Sep 17 2018 at 15:25, on Zulip):

left a comment

nikomatsakis (Sep 17 2018 at 15:25, on Zulip):

yeah, the exact thing to do is a bit unclear

davidtwco (Sep 17 2018 at 15:26, on Zulip):
error[E0432]: unresolved import `foo`
  --> src/test/ui/rust-2018/local-path-suggestions.rs:18:9
   |
18 |     use foo::Bar;
   |         ^^^ Could not find `foo` in `{{root}}`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0432`.
davidtwco (Sep 17 2018 at 15:26, on Zulip):

I guess I can work on improving the above.

davidtwco (Sep 17 2018 at 15:26, on Zulip):

(that's from the example in the comment you left)

nikomatsakis (Sep 17 2018 at 15:32, on Zulip):

that's what I would expect, yes

nikomatsakis (Sep 17 2018 at 15:32, on Zulip):

the other case I would like to see improved is something like use tsd::foo

nikomatsakis (Sep 17 2018 at 15:32, on Zulip):

which would ideally suggest std

nikomatsakis (Sep 17 2018 at 15:32, on Zulip):

(or maybe it does?)

nikomatsakis (Sep 17 2018 at 15:32, on Zulip):

this applies to use foo too, potentialy

nikomatsakis (Sep 17 2018 at 15:33, on Zulip):

basically I think we should look for:

but we could start by looking for modules/items in the root with an exact match

davidtwco (Sep 17 2018 at 15:33, on Zulip):

Doing use tsd::path doesn't suggest use std::path.

nikomatsakis (Sep 17 2018 at 15:33, on Zulip):

(I'm not 100% sure how to do it, though)

davidtwco (Sep 17 2018 at 15:33, on Zulip):

Just the same error as above with the {{root}}.

nikomatsakis (Sep 17 2018 at 15:34, on Zulip):

ok

davidtwco (Sep 20 2018 at 15:13, on Zulip):

@nikomatsakis Submitted a PR for the above: #54391 - apologies for the delay on it, didn't find time to look at it as quickly as I'd hoped.

Happy to change up how it is implemented if it isn't what you had in mind - error reporting logic seems fairly scattered in the resolve crate and I wasn't sure at which point to insert the suggestions, since there were a handful of places where similar suggestions are added currently. Not sure about the resolve_self_super_hint.rs test case change either.

Last update: Nov 16 2019 at 01:45UTC