Rustdoc: Use correct def_id for doctree::Import

[aDotInTheVoid]

The default overwrites the crate root, which crashes rustdoc-json.

While investigating this, It turns out somehow, some items are being documented twice. I'm not sure how this is happening but for now, we just make sure they were the same if they have the same id.

Zulip descussion

Bless script (Once this is more pollished I'll submit it)

r? @jyn514

[jyn514]

@bors r+

I have a nagging feeling this is fixing the symptom instead of the underlying bug, since the JSON module is still getting tricked into overwriting the original item with the import, but this seems strictly better than the current ICE so I'm ok with merging it for now.

[bors]

📌 Commit c254a15 has been approved by jyn514

[bors]

⌛ Testing commit c254a15 with merge 34aaff8c6343edf57d1716b9119e3e11770da033...

[bors]

💔 Test failed - checks-actions

[jyn514]

Cargotest failed while testing xsv because proptest's test cases are randomized. cc @rust-lang/infra - I thought we got rid of randomized tests at some point? Did we miss some?

cc @BurntSushi, you may be interested in fixing the xsv bug (assuming it hasn't already been fixed on master, I think cargotest is pretty out of date): https://github.com/rust-lang-ci/rust/runs/1511131529#step:24:8300

@bors retry

[bors]

⌛ Testing commit c254a15 with merge afa995b...

[BurntSushi]

@jyn514 I think that bug (or test) should be fixed on master. Are y'all using master or the latest release? The latest release is pretty old at this point and I'm not sure when I'll get the next one out.

[jyn514]

We appear to be using a pinned commit from 2017 🤦

rust/src/tools/cargotest/main.rs

Line 39 in b5ff9c3

sha: "66956b6bfd62d6ac767a6b6499c982eae20a2c9f",

I'll see about updating that.

[aDotInTheVoid]

I have a nagging feeling this is fixing the symptom instead of the underlying bug, since the JSON module is still getting tricked into overwriting the original item with the import

Theres 2 things going on here, that caused 2 items with the same ID to be inserted, only one of which this fixes

1. Imports were given the CRATE_DEF_INDEX instead of their real ID. This is fixed
2. Items were being documented twice. This is not fixed

I've added a check that the item removed from the map is the same as what was put, so we ICE if case 1 comes back. For now, case 2 is silently ignored, but this will be changed when it is fixed.

[jyn514]

Imports were given the CRATE_DEF_INDEX instead of their real ID. This is fixed

Well, but they're being given the ID of the item, right, not the id of the import? I think that's where the duplicates come from, it's being stored for both the import and the original item. Consider

mod x {
  pub struct S;
}
pub use x::S;

Right now I think it will insert S into the map twice: even though the item is unique, the places it is accessible are not.

[bors]

☀️ Test successful - checks-actions
Approved by: jyn514
Pushing afa995b to master...

[aDotInTheVoid]

Well, but they're being given the ID of the item, right, not the id of the import?

We have id 0:7, which is the import of id 0:5, so they do have seperate IDs

[aDotInTheVoid]

@rustbot modify labels +A-rustdoc-json