Revert "Don't load all extern crates unconditionally"

[GuillaumeGomez]

Fixes #84738.

This reverts #83738.

For the "smart" load of external crates, we need to be able to access their items in order to check their doc comments, which seems, if not impossible, quite complicated using only the AST.

For some context, I first tried to extend the IntraLinkCrateLoader visitor by adding visit_foreign_item. Unfortunately, it never enters into this call, so definitely not the right place...

I then added visit_use_tree to then check all the imports outside with something like this:

let mut loader = crate::passes::collect_intra_doc_links::IntraLinkCrateLoader::new(resolver);
    ast::visit::walk_crate(&mut loader, krate);

    let mut items = Vec::new();
    for import in &loader.imports_to_check {
        if let Some(item) = krate.items.iter().find(|i| i.id == *import) {
            items.push(item);
        }
    }
    for item in items {
        ast::visit::walk_item(&mut item);
        for attr in &item.attrs {
            loader.check_attribute(attr);
        }
    }

This was, of course, a failure. We find the items without problems, but we still can't go into the external crate to check its items' attributes.

Finally, @jyn514 suggested to look into the CrateLoader, but it only seems to provide metadata (I went through CStore and CrateMetadata).

I think we are too limited here (with AST only) to be able to determine the crates we actually need to import, but it's very likely that I missed something. Maybe @petrochenkov or @Aaron1011 have an idea?

So until we find a way to make it work completely, we need to revert it to fix the ICE. Once merged, we'll need to re-open #68427.

r? @jyn514

[jyn514]

Please remember to re-open #68427 if this gets merged.

[jyn514]

I will not have time to review this for a while.

r? @petrochenkov

[GuillaumeGomez]

Please remember to re-open #68427 if this gets merged.

It's in the first message already. ;)

[petrochenkov]

I'd prefer to actually fix #84738 instead of reverting the PR and re-introducing other bugs.

Waiting on minimized reproduction for #84738 - https://rust-lang.zulipchat.com/#narrow/stream/266220-rustdoc/topic/crate.20loading.20during.20link.20resolution/near/240747900

[TheBlueMatt]

Does https://github.com/rust-bitcoin/rust-lightning/blob/main/lightning-background-processor/src/lib.rs qualify as a minimized reproduction? Its only a few method crate and tickles the bug. The version on crates.io hits it.

[petrochenkov]

(I've returned from vacation and will look at this next weekend.)

[GuillaumeGomez]

\o/

[bors]

☔ The latest upstream changes (presumably #86489) made this pull request unmergeable. Please resolve the merge conflicts.

[jyn514]

r=me, this has been causing lots of trouble and we can always do a more proper fix later. @GuillaumeGomez can you fix the conflict real quick?

[GuillaumeGomez]

On it!

[jyn514]

@bors r+

[bors]

📌 Commit 5f0c54d has been approved by jyn514

[jyn514]

@rust-lang/rustdoc are you ok with backporting this? For context, this fixes a regression from 1.52 to 1.53 (an ICE on code when generating intra doc links). It does reintroduce the bug from #68427, but that's a very narrow edge case I don't expect most people to run into.

[Manishearth]

I'm okay with the backport

[GuillaumeGomez]

I am as well. I think you can set the label. :)

[GuillaumeGomez]

I erroneously pushed on this branch from another PR so I need to re-approve it.

@bors: r=jyn514

[bors]

📌 Commit 5f0c54d has been approved by jyn514

[camelid]

It looks like #68427 is still closed?