Stream: t-compiler/help

Topic: panic in tcx.parent(def_id): bug in my code or rustc_middle?


Joshua Nelson (Jun 07 2020 at 14:13, on Zulip):

While working on https://github.com/rust-lang/rust/issues/65983, I changed rustdoc around to pass DefIds to resolve_str_path_error instead of LocalDefIds. Now rustdoc is panicking on any file, including empty files. Is this a bug in my code or in rustc_middle? The panic comes from tcx.parent(def_id):

        use rustc_middle::ty::DefIdTree;
        let parent_node = self.cx.tcx.parent(item.def_id);

Backtrace (the relevant bits):

  13: core::panicking::panic_bounds_check
             at src/libcore/panicking.rs:73
  14: <usize as core::slice::SliceIndex<[T]>>::index
             at /home/joshua/src/rust/src/libcore/slice/mod.rs:2872
  15: core::slice::<impl core::ops::index::Index<I> for [T]>::index
             at /home/joshua/src/rust/src/libcore/slice/mod.rs:2732
  16: <alloc::vec::Vec<T> as core::ops::index::Index<I>>::index
             at /home/joshua/src/rust/src/liballoc/vec.rs:1947
  17: <rustc_index::vec::IndexVec<I,T> as core::ops::index::Index<I>>::index
             at /home/joshua/src/rust/src/librustc_index/vec.rs:716
  18: rustc_hir::definitions::DefPathTable::def_key
             at /home/joshua/src/rust/src/librustc_hir/definitions.rs:53
  19: rustc_metadata::rmeta::decoder::<impl rustc_metadata::creader::CrateMetadataRef>::def_key
             at src/librustc_metadata/rmeta/decoder.rs:1411
  20: rustc_metadata::rmeta::decoder::cstore_impl::<impl rustc_middle::middle::cstore::CrateStore for rustc_metadata::creader::CStore>::def_key
             at src/librustc_metadata/rmeta/decoder/cstore_impl.rs:484
  21: rustc_middle::ty::context::TyCtxt::def_key
             at src/librustc_middle/ty/context.rs:1188
  22: <rustc_middle::ty::context::TyCtxt as rustc_middle::ty::DefIdTree>::parent
             at src/librustc_middle/ty/mod.rs:358
  23: <rustdoc::passes::collect_intra_doc_links::LinkCollector as rustdoc::fold::DocFolder>::fold_item
             at src/librustdoc/passes/collect_intra_doc_links.rs:482
Joshua Nelson (Jun 07 2020 at 14:14, on Zulip):

oh and the message was thread 'rustc' panicked at 'index out of bounds: the len is 38899 but the index is 38905', /home/joshua/src/rust/src/librustc_hir/definitions.rs:53:9

simulacrum (Jun 07 2020 at 14:17, on Zulip):

it might be because rustdoc allocates non-real defids?

Joshua Nelson (Jun 07 2020 at 14:19, on Zulip):

What do you mean?

simulacrum (Jun 07 2020 at 14:20, on Zulip):

IIRC last I looked, rustdoc sometimes has a DefId which is not "real" in the sense that it's not present in the tcx maps or so

simulacrum (Jun 07 2020 at 14:20, on Zulip):

see e.g. https://github.com/Mark-Simulacrum/rust/blob/6f015768c28a6e4cf163967cb62b70c645791858/src/librustdoc/clean/types.rs#L88-L90 which has special handling

marmeladema (Jun 07 2020 at 14:31, on Zulip):

So rustdoc does a weird pass where it removes some layer of the AST

marmeladema (Jun 07 2020 at 14:31, on Zulip):

which invalidates the DefId tree

marmeladema (Jun 07 2020 at 14:32, on Zulip):

See https://github.com/rust-lang/rust/pull/72088 and https://github.com/rust-lang/rust/issues/71820 for a similar issue

marmeladema (Jun 07 2020 at 14:38, on Zulip):

And I don't really know how to help you ... I am also stuck. I think the ultimate goal is to remove ReplaceBodyWithLoop entirely but I don't really understand what is it needed for right now and by what it should be replaced.

Joshua Nelson (Jun 07 2020 at 14:59, on Zulip):

Oh that's so unfortunate ... @simulacrum do you know if that MAX_DEF_ID snippet is a function somewhere? If so I could just call def_id.is_fake() and then ignore fake ids

simulacrum (Jun 07 2020 at 14:59, on Zulip):

hm I guess probably no given that it's not being used here?

Joshua Nelson (Jun 07 2020 at 15:00, on Zulip):

ok, I think I'll make a PR for that

simulacrum (Jun 07 2020 at 15:00, on Zulip):

but to be honest I'm not familiar with that piece of rustdocs code

Joshua Nelson (Jun 07 2020 at 15:56, on Zulip):

@marmeladema looking at the comments in https://github.com/Mark-Simulacrum/rust/blob/6f015768c28a6e4cf163967cb62b70c645791858/src/librustdoc/core.rs#L103, it looks like there are two issues: 1. ids which were originally valid, but were invalidated by ReplaceBodyWithLoop, and 2. ids which were always fake and were never valid. The first looks really hard to deal with but also less common, so for now I'm only dealing with the second.

marmeladema (Jun 07 2020 at 16:07, on Zulip):

Lucky you :p

Joshua Nelson (Jun 07 2020 at 16:11, on Zulip):

Oh don't worry, I'm sure the first case will show up before I'm through :laughing:

Joshua Nelson (Jun 07 2020 at 16:11, on Zulip):

I'm working on https://github.com/rust-lang/rust/issues/65983 so I'm touching basically every part of the frontend

marmeladema (Jun 07 2020 at 16:16, on Zulip):

Oh your touching the export_map right now? Me too :octopus:

marmeladema (Jun 07 2020 at 16:38, on Zulip):

@Joshua Nelson fyi https://github.com/rust-lang/rust/pull/73090
Dunno if that impacts what you're currently working on

Joshua Nelson (Jun 07 2020 at 16:53, on Zulip):

Thanks for the link. I'm mostly working on the rustdoc side for now. I didn't realize ExportMap only had local items, that makes sense since they're all from the same crate though.

Joshua Nelson (Jun 07 2020 at 17:01, on Zulip):

Yay if I don't call .parent() for fake IDs it works :tada:

Joshua Nelson (Jun 07 2020 at 17:04, on Zulip):

omg the links work too image.png

Joshua Nelson (Jun 07 2020 at 17:04, on Zulip):

It links to the external URL though, not to the dependency :thinking:

Joshua Nelson (Jun 07 2020 at 17:04, on Zulip):

let me comment on the issue, we've gotten a little far from the panic

Joshua Nelson (Jun 07 2020 at 21:12, on Zulip):

@marmeladema I ran into the first case :laughing:

(-bash@build-server) ~/.../src/test ▶️ rustdoc +stage1 rustdoc/macro-in-closure.rs
thread 'rustc' panicked at 'called `Option::unwrap()` on a `None` value', /home/joshua/src/rust/src/librustc_hir/definitions.rs:358:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
(-bash@build-server) ~/.../src/test ▶️ cat !$
cat rustdoc/macro-in-closure.rs
// Regression issue for rustdoc ICE encountered in PR #65252.

#![feature(decl_macro)]

fn main() {
    || {
        macro m() {}
    };
}
marmeladema (Jun 07 2020 at 21:16, on Zulip):

Yes thats exactly the same test that is failing in the issue I mentioned. But as a matter of fact, @ecstatic-morse has started working on this :) we might have a solution sooner than expected

Last update: Jan 22 2021 at 13:30UTC