Stream: rustdoc

Topic: pass fake DefId's to rustc APIs


view this post on Zulip Stu (Apr 29 2021 at 18:31):

Hey,
I'm currently working on #83183 and I set up the enum, and also removed the old hack.
One of the main sources for the 280 errors that need to be fixed are

expected struct `rustc_span::def_id::DefId`, found enum `types::DefId`

Should I just put an expect_real there so that it panics when trying to pass a fake DefId?

Also, some more mentoring instructions would probably be helpful :) cc @Joshua Nelson
Currently, I'm doing most of this without knowing much about the rustdoc codebase

view this post on Zulip Joshua Nelson (Apr 29 2021 at 18:32):

oh wow that's amazing!

view this post on Zulip Joshua Nelson (Apr 29 2021 at 18:32):

Should I just put an expect_real there so that it panics when trying to pass a fake DefId?

ideally we would separate things into "known real" and to "possibly fake" fields

view this post on Zulip Joshua Nelson (Apr 29 2021 at 18:32):

can you give a couple examples of errors?

view this post on Zulip Joshua Nelson (Apr 29 2021 at 18:34):

Blindly translating things to .expect_real is probably ok for now though

view this post on Zulip Stu (Apr 29 2021 at 18:36):

// src/librustdoc/passes/collect_trait_impls.rs:182:21

self.impls.extend(get_auto_trait_and_blanket_impls(
    self.cx,
    self.cx.tcx.type_of(i.def_id.expect_real()),
    i.def_id,
));

is one example. It's hard to find "real" ones because most of the errors can be easily fixed by importing clean::DefId instead of rustc_hir::DefId

view this post on Zulip Stu (Apr 29 2021 at 18:37):

Joshua Nelson said:

Blindly translating things to .expect_real is probably ok for now though

Alright, I will start with that.

view this post on Zulip Joshua Nelson (Apr 29 2021 at 18:41):

That one looks correct to call expect_real() on, no fake ids at all are generated before get_auto_and_blanket_impls()

view this post on Zulip Joshua Nelson (Apr 29 2021 at 18:41):

Hmm I wonder if it makes sense to have only things in that impls field use fake ids

view this post on Zulip Joshua Nelson (Apr 29 2021 at 18:42):

And parameterize Item on the id or something

view this post on Zulip Joshua Nelson (Apr 29 2021 at 18:44):

Also, some more mentoring instructions would probably be helpful :) cc @Joshua Nelson

I'll be honest, most of the work here is designing what the API should look like

view this post on Zulip Joshua Nelson (Apr 29 2021 at 18:56):

I'm going to assign you on the issue so other people don't try to work on it at the same time, no pressure though - even if you only get halfway that's still useful :)

view this post on Zulip Stu (Apr 29 2021 at 18:57):

Ah yes. forgot that. Thanks

view this post on Zulip Stu (Apr 29 2021 at 19:14):

I opened #84707 to track the progress :+1:

view this post on Zulip Joshua Nelson (Apr 29 2021 at 19:16):

@Stu looks like you based it off an old branch - you might want to rebase that before fixing everything

view this post on Zulip Stu (Apr 29 2021 at 19:16):

ah yea, I forgot to update my rust fork

view this post on Zulip Joshua Nelson (Apr 29 2021 at 19:17):

oh wow yeah that's from january haha

view this post on Zulip Stu (Apr 29 2021 at 19:19):

oh wow. whoops. That will be fun to rebase

view this post on Zulip Stu (Apr 29 2021 at 19:40):

ah nice. ICE while running ./x.py check. Incremental compilation didn't like the rebase I guess

view this post on Zulip Joshua Nelson (Apr 29 2021 at 19:42):

lol nice, was it an "unstable predicates" crash?

view this post on Zulip Joshua Nelson (Apr 29 2021 at 19:42):

you can fix that with rm -r build/$TARGET/stage0-rustc/release/incremental

view this post on Zulip Stu (Apr 29 2021 at 19:42):

Yeah, found unstable fingerprints for predicates_of

I just ran clean :sweat_smile:

view this post on Zulip Joshua Nelson (Apr 29 2021 at 19:42):

oh oof

view this post on Zulip Joshua Nelson (Apr 29 2021 at 19:43):

btw if you wait to make the rustc changes for a later commit, you can use download-rustc instead which takes way less time

view this post on Zulip Stu (Apr 29 2021 at 19:46):

oh thats awesome, but I get the same NixOS error you opened an issue about

view this post on Zulip Joshua Nelson (Apr 29 2021 at 19:46):

you can fix that by copy/pasting the command and replacing ci-rustc with stage0

view this post on Zulip Joshua Nelson (Apr 30 2021 at 14:25):

Stu said:

oh thats awesome, but I get the same NixOS error you opened an issue about

@Stu fyi: https://github.com/rust-lang/rust/pull/84750

view this post on Zulip Joshua Nelson (Apr 30 2021 at 14:26):

you can cherry-pick that to have cargo fixed automatically (although you shouldn't run into this again until the beta bump or you clear your build cache)

view this post on Zulip Stu (Apr 30 2021 at 16:00):

finally, first module done :sweat_smile:

view this post on Zulip Stu (Apr 30 2021 at 16:12):

only 150 errors left :tada:

view this post on Zulip Joshua Nelson (Apr 30 2021 at 16:13):

oh boy

view this post on Zulip Stu (May 01 2021 at 09:47):

Finally, everything compiles :tada:

view this post on Zulip Stu (May 01 2021 at 09:55):

Test Result

Whoops

view this post on Zulip Stu (May 01 2021 at 12:05):

Test suite passes :tada:

Should be ready for review

view this post on Zulip Joshua Nelson (May 01 2021 at 14:35):

That's incredible

view this post on Zulip Joshua Nelson (May 01 2021 at 14:35):

Wow

view this post on Zulip Stu (May 01 2021 at 15:23):

holy shit I'm hitting this so often

view this post on Zulip Joshua Nelson (May 01 2021 at 16:05):

@Stu the fix was approved if you want to cherry pick the commit, git can handle it


Last updated: Oct 11 2021 at 22:34 UTC