Support intra-doc links on cross-crate reexports

[Manishearth]

Previously: #60883, #59833

This is the last major issue blocking the stabilization of intra doc links (#43466)

Currently, if you use intra-doc links on some item:

// crate "foo"

/// This is not [Baz]
pub struct Bar;

pub struct Baz;

the docs will not turn up on any crate reexporting it

pub use foo::Bar;

The docs will have a bare [Baz] instead of the appropriate intra-doc link, because we resolve intra-doc links locally. (More examples in #60883, #59833)

---

This is tricky to fix; we throw away module resolution information before packaging up crate metadata, so it's not possible to "unseal" a crate and rerun the resolution within it. One solution would be to pre-detect intra doc links and store resolutions in metadata, but this would be expensive since it involves parsing everything as markdown unconditionally.

A better solution is to save resolution information in the metadata, as some kind of tree keyed to the module.

@eddyb's comment outlines the correct approach to use here

What you actually want to persist cross-crate is the scope information that lets you resolve names in a certain (module) scope, after the fact. It'd also allow not doing all the the resolver hacks.

And there's a very good reason to want it in the compiler, at least same-crate: we could (finally) tell if a type is in scope somewhere so we can actually print Vec<_> instead of std::vec::Vec<_> etc.

Basically, instead of throwing away resolver metadata, we should serialize it into some form and store it in the metadata, so that it can be reloaded as necessary.

[kinnison]

I'm interested in helping on this, but right now I'm not confident in the requisite parts of the compiler. I will take a poke around, but please don't let that stop someone who actually knows what they're doing getting it done :D

[eddyb]

At this point I really wish rustdoc always took a set of crates' sources and never relied on metadata from rlibs.
It would make so many things easier, including this.

[kinnison]

Okay, I've spent a bit of time trying to find my way around the libraries which comprise the compiler, looking for things associated with the metadata and/or name resolution. I'm starting to get a feel for things, but I'm still not certain I know quite what I'm trying to achieve. Since this is E-hard I wasn't expecting to work it out right away fortunately, and I'm prepared to keep learning, but it'd be incredibly helpful if someone were prepared to help point me toward the right places to be learning about. Once I feel like I've done enough basic learning, I'll probably post a rough outline of what work I think needs doing to complete this (kinda as an expansion of Manish's original description) and then ask for review of that.

@Manishearth or @eddyb I'm guessing would be best to ask for that review, so this is me pre-asking to see if the above approach would work for either of you?

[Manishearth]

This would work for me, I think @eddyb is far more familiar with some of the stuff here than I am.

librustc_resolve contains a resolution tree that effectively needs serializing, but ideally a reduced form of it.

[eddyb]

Probably the best way to do this would be in two steps:

1. come up with some data structure to place in librustc
   • it only needs to handle being from the same crate, to begin with
   • librustc_resolve would export this, and librustdoc would use it to resolve (some?) links, potentially removing the need for "late Resolver usage" in librustdoc
   • @petrochenkov is most likely to know how and what to exfiltrate from librustc_resolve, I can only guess what parts are relevant
2. once you have that, you can make librustc_metadata encode/decode it
   • the actual serialization is based on #[derive]s so all you really need to do is add the plumbing around that
   • this is the part I can actually help with

[Manishearth]

come up with some data structure to place in librustc

Basically needs to be a map of module DefId to all contents (imported and otherwise) by name.

Something like HashMap<DefId, ModuleNames>, struct ModuleName {types: HashMap<Ident, DefId>, values: .., macros: ..}

idk how heavy that would be, or how much work that would be to compute

[kinnison]

That all sounds super-useful. I guess I need to start learning what a DefId is etc. properly :D

[eddyb]

@Manishearth Hmm, sounds like the way ExportMap works, except not just for exports. I thought rustdoc would need something more fine-grained than mods, but if not that's great.

EDIT: just realized after sending my comment, that since only exports from modules are documented, only module-level imports would be relevant.
Overall this seems pretty easy, I wonder what's the worst part. Glob imports?

[Manishearth]

@eddyb precisely.

Also, currently we don't do inner imports anyway, the following will not work (except for modules):

fn foo() {
   //! [baz]
   use bar::baz;
}

It's debatable if we want that to work, and in that case we can use local resolution though i'd prefer if we only used one way of resolving.

Maybe we can just make ExportMap contain all the module level items, with a pub/priv tag?

Of course this doesn't 100% solve your other problem of being able to spew scoped diagnostics, but it's a step.

[kinnison]

I have been thinking quite hard about this and I wonder if it might be sufficient to detect any [...] sequence in a docstring (should be cheap to do) and then do the path resolution work inside the compiler rather than deferring to when rustdoc begins? It would then be able to append (or prepend) reference resolution lines to the doc block, thereby not needing any changes to the serialisation or resolver etc.

The argument @Manishearth made in the original posting that full parsing of the markdown would be expensive wouldn't have to happen for every docstring, only ones which contain the [ character, and we could blithely create reference resolution lines for any [...] in the text without consideration of markdown if we wanted to be even faster.

I appreciate this really wouldn't help @eddyb but it strikes me as a potentially much simpler pathway to completion of this task.

cc @GuillaumeGomez

[Manishearth]

that ... could work!

i would prefer we do the harder thing, but this would let us ship.

However, note that this means we can no longer emit warnings for broken links since we're not sure which things are meant to be intra doc links without preparsing markdown

[kinnison]

Oh good point about the lint/warning. Hmm. I guess that we really do need that independent lookaside unless we start doing daft things around embedded nulls in the docs to separate input from autogenerated resolutions, and that would be HORRIBLE. I'll continue to think about possibly simpler solutions while also trying to continue to learn enough to do the "right" thing instead.

[Manishearth]

We should see if we can close this out, this is basically the one thing blocking us from stabilizing intra-doc-links, and folks have been asking for this feature for years.

There are roughly three ways to implement this:

• librustc_resolve emits a module-imports map -- a structure mapping each module to a list of its imports. this gets stored in the metadata, we consume this in rustdoc
• the rustc parser actually parses markdown in doc comments, stashes away all the unresolved paths, and librustc_resolve fixes them up with def ids, which get stored in
• rustdoc itself has its own form of metadata, and when it does intra-doc-link resolution it stashes the metadata somewhere

The last option is the one that appeals to me most because it has zero perf impact, but I don't know how best to do it architecturally (perhaps just a json file in doc/ somewhere? idk how best to invalidate it). The first option is probably the easiest.

@rust-lang/compiler @rust-lang/rustdoc do y'all have opinions on how best to do this? Do you think adding some form of metadata to rustdoc itself is doable?

[kinnison]

For anyone reading this, I ended up unable to continue with this work, so the job is still entirely open for anyone to take.

[nikomatsakis]

Huh, interesting. This feels related to our long-standing efforts to extact name resolution into a shareable code based that both rust-analyzer and rustc can make use of. Not that we've made much progress on that.

[Manishearth]

@jyn514 this is still not the actual issue at hand. it's part of it, but the actual issue involves multiple crates, and the documentation being resolved needs to come from the dependency crate. This is not about documentation on reexports itself.

Here are some test cases that should work:

//crate a

pub struct Bar;

/// Link to [Bar]
pub struct Foo;

// crate b
pub use a::Foo;

and the more complicated case of

// crate a

pub mod bar {
   pub struct Bar;
}

pub mod foo {
  use crate::bar;
  /// link to [bar::Bar]
  pub struct Foo;
}

// crate b

pub use a::foo::Foo;

Part of the issue is indeed fixing things so that reexports within a crate resolve correctly, but that's not all.