Stream: t-compiler

Topic: prelude crates


nikomatsakis (Sep 27 2018 at 18:12, on Zulip):

@eddyb is there an easy way to get a list of "prelude crates" which can be referenced without need of an extern crate?

eddyb (Sep 27 2018 at 18:12, on Zulip):

it's trivially computed in the resolve code. you might want to put it in the Session

eddyb (Sep 27 2018 at 18:12, on Zulip):

it's core, std, meta, plus all the names in --extern

eddyb (Sep 27 2018 at 18:13, on Zulip):

https://github.com/rust-lang/rust/blob/c9865b1c37f8cb8a257591e6ea0b32a5df1f3d41/src/librustc_resolve/lib.rs#L1666-L1675

nikomatsakis (Sep 27 2018 at 18:15, on Zulip):

OK, I'm asking because of @davidtwco's PR, which is special casing the std name — I think because of the exern crate std that we insert in the prelude.

nikomatsakis (Sep 27 2018 at 18:15, on Zulip):

It seems like the "general" form of this check is to look for those crates that are in the prelude

davidtwco (Sep 27 2018 at 18:17, on Zulip):

I looked for a while to see if I could find anything that did that and couldn't find anything.

eddyb (Sep 27 2018 at 18:18, on Zulip):

rustc_resolve is the only place that needs it to do its job, heh

davidtwco (Sep 27 2018 at 18:18, on Zulip):

(as in, I looked for some function or property somewhere that I could call to check if a CrateNum was from the prelude)

eddyb (Sep 27 2018 at 18:19, on Zulip):

aaah

eddyb (Sep 27 2018 at 18:19, on Zulip):

you can check if you can access a name directly, but not that a CrateNum relates to that

eddyb (Sep 27 2018 at 18:19, on Zulip):

it's kind of a tricky thing overall

eddyb (Sep 27 2018 at 18:19, on Zulip):

depends how accurate you want to get

nikomatsakis (Sep 27 2018 at 18:20, on Zulip):

also @eddyb I just filed #54618 — namely that we ought to search prelude crates for traits that offer the methods the user was trying to call

nikomatsakis (Sep 27 2018 at 18:20, on Zulip):

no idea how hard that's going to be though

eddyb (Sep 27 2018 at 18:20, on Zulip):

okay, so, rustc_resolve already has code for that too

eddyb (Sep 27 2018 at 18:21, on Zulip):

maybe we should speculatively load all of them and record the CrateNums

eddyb (Sep 27 2018 at 18:21, on Zulip):

@nikomatsakis https://github.com/rust-lang/rust/blob/c9865b1c37f8cb8a257591e6ea0b32a5df1f3d41/src/librustc_resolve/lib.rs#L4404-L4408

eddyb (Sep 27 2018 at 18:22, on Zulip):

note the maybe_ in the name - that won't produce an user error if the crate can't be loaded

davidtwco (Sep 27 2018 at 18:22, on Zulip):

I remembered the code for this from my last PR that was predominantly in rustc_resolve, but couldn't see a way to access that stuff from where ever this change took place.

nikomatsakis (Sep 27 2018 at 18:22, on Zulip):

maybe we should speculatively load all of them and record the CrateNums

well, I think we only want to do that in the case of a compilation error?

nikomatsakis (Sep 27 2018 at 18:22, on Zulip):

I just remember people were concerned about side effects

nikomatsakis (Sep 27 2018 at 18:22, on Zulip):

I think due to linking or something though

eddyb (Sep 27 2018 at 18:22, on Zulip):

so I guess cache it in Session?

eddyb (Sep 27 2018 at 18:23, on Zulip):

(and compute it on-demand)

eddyb (Sep 27 2018 at 18:23, on Zulip):

actually, it should be cheap to get the crates if they were loaded already, so, nvm, you can just do this on every error and be fine

nikomatsakis (Sep 27 2018 at 18:23, on Zulip):

man I wish the query system went back that far

eddyb (Sep 27 2018 at 18:23, on Zulip):

just put it somewhere central for convenience

nikomatsakis (Sep 27 2018 at 18:23, on Zulip):

"feels like a query"

eddyb (Sep 27 2018 at 18:23, on Zulip):

I wish I had nothing else to work on than pushing back the query system to parsing :P

eddyb (Sep 27 2018 at 18:24, on Zulip):

I have most of it planned out, I just... need to go do it

davidtwco (Sep 27 2018 at 18:28, on Zulip):

I'm not sure I follow completely. If I've got a CrateNum, and I want to call some function to check whether it is from the prelude crates - I'm not quite understanding what change I'd need to make in order to do that lookup.

nikomatsakis (Sep 27 2018 at 18:38, on Zulip):

I wish I had nothing else to work on than pushing back the query system to parsing :P

me too :)

nikomatsakis (Sep 27 2018 at 18:38, on Zulip):

@davidtwco hmm, I think that the code @eddyb pointed out would wind up giving a set of crate names

nikomatsakis (Sep 27 2018 at 18:38, on Zulip):

so I guess you'd have to check if the name of the crate with that crate num is in that set

eddyb (Sep 27 2018 at 18:38, on Zulip):

well, initially it's a set of names

eddyb (Sep 27 2018 at 18:38, on Zulip):

but if you use the last code I linked, you actually get a set of CrateNums

eddyb (Sep 27 2018 at 18:39, on Zulip):

which is better, I think?

nikomatsakis (Sep 27 2018 at 18:40, on Zulip):

seems better, yeah

eddyb (Sep 27 2018 at 18:42, on Zulip):

because it might matter which of several crates with the same name can actually be referred to in the current crate (i.e. what --extern foo=... makes it load, vs what's a dependency of other dependencies)

davidtwco (Sep 27 2018 at 18:43, on Zulip):

So, the intention is to just save those from that point into something in Session?

eddyb (Sep 27 2018 at 18:43, on Zulip):

not "from that point"

eddyb (Sep 27 2018 at 18:43, on Zulip):

you don't want rustc_resolve to have any side-effects like that

eddyb (Sep 27 2018 at 18:44, on Zulip):

instead, you want to move that code in a method on the Session, so it can be called from multiple places

davidtwco (Sep 27 2018 at 18:45, on Zulip):

Ah, I see, sounds good. Thanks.

davidtwco (Sep 27 2018 at 19:40, on Zulip):

I've changed this with the most recent commit.

davidtwco (Sep 27 2018 at 20:37, on Zulip):

@nikomatsakis does this failure look spurious to you? - not sure what the most recent commit could have done to really break that. It passed tests locally.

nikomatsakis (Sep 27 2018 at 20:45, on Zulip):

sure does

davidtwco (Sep 27 2018 at 20:46, on Zulip):

Cool, restarted that Travis job.

davidtwco (Sep 27 2018 at 21:12, on Zulip):

Huh, it happened again.

davidtwco (Sep 27 2018 at 21:13, on Zulip):

It doesn't seem to be happening locally.

nikomatsakis (Sep 27 2018 at 21:18, on Zulip):

well

nikomatsakis (Sep 27 2018 at 21:18, on Zulip):

actually

nikomatsakis (Sep 27 2018 at 21:18, on Zulip):

that same code is used in symbol name generation

nikomatsakis (Sep 27 2018 at 21:18, on Zulip):

which..is one of the things I now hate about it

nikomatsakis (Sep 27 2018 at 21:18, on Zulip):

also one of the things that is totally my fault

nikomatsakis (Sep 27 2018 at 21:18, on Zulip):

but never mind

nikomatsakis (Sep 27 2018 at 21:19, on Zulip):

so it is plausible that we have caused these errors

nikomatsakis (Sep 27 2018 at 21:19, on Zulip):

why it doesn't occur locally idk, are you building to stage2?

nikomatsakis (Sep 27 2018 at 21:19, on Zulip):

perhaps that is it?

davidtwco (Sep 27 2018 at 21:19, on Zulip):

I've started a full build locally when the first failure happened.

davidtwco (Sep 27 2018 at 21:19, on Zulip):

It has gotten past where it would normally fail on Travis.

davidtwco (Sep 27 2018 at 21:20, on Zulip):

It surprises me because the commit just moves where something is instantiated, there's no real functional difference (except the comparison for the item path changing).

davidtwco (Sep 27 2018 at 21:28, on Zulip):

Huh, hit the error locally at the very end of the compiler artifacts for stage1.

davidtwco (Sep 27 2018 at 21:30, on Zulip):

I'm really not sure what it is about the commit that would cause that.

davidtwco (Sep 27 2018 at 21:37, on Zulip):

Any ideas @nikomatsakis?

davidtwco (Sep 27 2018 at 21:41, on Zulip):

Since Travis seems to have messed up the log for the most recent retry, this is a gist of the error I see locally: https://gist.github.com/davidtwco/36b469179c0bd3851b0de4c426abc0fa

nikomatsakis (Sep 27 2018 at 21:48, on Zulip):

I'm really not sure what it is about the commit that would cause that.

ugh. I'm not really sure, but I can sort of imagine...

nikomatsakis (Sep 27 2018 at 21:49, on Zulip):

hmm

nikomatsakis (Sep 27 2018 at 21:49, on Zulip):

I feel like it's weird that we generate a symbol that takes into account where the extern crate appears in the source tree anyway

nikomatsakis (Sep 27 2018 at 21:49, on Zulip):

but I imagine that if we are altering things based on edition

nikomatsakis (Sep 27 2018 at 21:49, on Zulip):

maybe we are linking together crates that wind up with a different .. view? not sure

nikomatsakis (Sep 27 2018 at 21:50, on Zulip):

I have to run but let me give you a 'tip'

davidtwco (Sep 27 2018 at 21:50, on Zulip):

All this new commit does for the rustc_resolve module is change where we get the prelude crate names from. Other than that, it's just the trait suggestion logic that changed again in this commit.

nikomatsakis (Sep 27 2018 at 21:51, on Zulip):

I think the code is in librustc_codegen_utils/symbol_names.rs

davidtwco (Sep 27 2018 at 21:52, on Zulip):

The previous commit (which didn't seem to have that issue) did change that file, but only to add a #[derive(Debug)] to a struct.

nikomatsakis (Sep 27 2018 at 21:52, on Zulip):

@davidtwco in particular

fn def_symbol_name<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId) -> ty::SymbolName {
    let mut buffer = SymbolPathBuffer::new();
    item_path::with_forced_absolute_paths(|| {
        tcx.push_item_path(&mut buffer, def_id);
    });
    buffer.into_interned()
}
nikomatsakis (Sep 27 2018 at 21:52, on Zulip):

I bet it's some kind of interaction with that flag

davidtwco (Sep 27 2018 at 21:53, on Zulip):

Perhaps.. but I didn't really change that flag at all, I added a new one, that only changes things when that flag isn't set.

nikomatsakis (Sep 27 2018 at 21:53, on Zulip):

well, maybe you're wrong about that? :)

davidtwco (Sep 27 2018 at 21:53, on Zulip):

I might be.

davidtwco (Sep 27 2018 at 21:53, on Zulip):

I guess I did.

nikomatsakis (Sep 27 2018 at 21:53, on Zulip):

alternatively

nikomatsakis (Sep 27 2018 at 21:53, on Zulip):

sometimes we fail to set the flag in some path

nikomatsakis (Sep 27 2018 at 21:54, on Zulip):

though I'd be surprsied to see such a bug crop up now

davidtwco (Sep 27 2018 at 21:54, on Zulip):

No, I think you're right. When I stopped it from recursing to the crate root when making the item path that could be it.

nikomatsakis (Sep 27 2018 at 21:54, on Zulip):

fwiw I think @mw is overhauling the symbol name generation code... not sure what exactly they have in mind though

davidtwco (Sep 27 2018 at 21:54, on Zulip):

Must have been lucky with the previous commit to not have failed I guess.

davidtwco (Sep 27 2018 at 23:01, on Zulip):

Alright, I've got a fresh build and tests passing locally, it takes a slightly different approach. Pushed that, so here's hoping.

nikomatsakis (Sep 28 2018 at 17:14, on Zulip):

@davidtwco nice!

nikomatsakis (Sep 28 2018 at 19:32, on Zulip):

@davidtwco around?

nikomatsakis (Sep 28 2018 at 19:33, on Zulip):

left a review

davidtwco (Sep 28 2018 at 19:35, on Zulip):

@nikomatsakis ping

Last update: Nov 22 2019 at 04:45UTC