rustdoc: `pub use` for macros is inlined too eagerly
petrochenkov opened this issue · 11 comments
pub use
for macros is "inlined" (in rustdoc sense) too eagerly and may show duplicated documentation (both inlined and non-inlined) and ignore #[doc(no_inline)]
and apparently #[doc(hidden)]
(#35896 (comment)) as well.
This can now be observed on nightly docs for the standard library - https://doc.rust-lang.org/nightly/std/.
The "Re-exports" section wasn't there previously and everything was in inlined-only form.
Looks like this affects beta as well - https://doc.rust-lang.org/beta/std/#reexports
cc @rust-lang/rustdoc
Ah, indeed. I'll take a look as soon as possible.
@GuillaumeGomez what's the status on this?
Still on my todo list.
Some initial poking:
Re-exported macros are currently pulled in during RustdocVisitor::visit_mod_contents
, and they currently don't handle doc(no_inline)
or doc(hidden)
:
rust/src/librustdoc/visit_ast.rs
Lines 219 to 252 in 1962a70
The actual export statement is handled in visit_item
, which processes through maybe_inline_local
and adds the export statement to the module for processing:
rust/src/librustdoc/visit_ast.rs
Lines 409 to 443 in 1962a70
Notably, this means that any doc(hidden)
on the export statement will hide this export rather than the macro itself.
A "quick fix" for this would be to check the import statement for doc(no_inline)
or doc(hidden)
where the import is being processed, but i'm not convinced that this is the best place to process these imports anyway. The doc comment (and name) on maybe_inline_local
indicates that only local re-exports are meant to go through that function, but i haven't yet found where cross-crate inlining properly occurs. (Or even whether this statement is incorrect! That comment was added 2 years ago, according to git blame.) The "proper" fix would be to move this processing to that location so we can re-use the "should we inline/expose this item" logic there.
While finishing that comment, i realized that cross-crate inlining happens in clean/inline.rs
. A clause would need to be added to try_inline
to catch macros that fall through there, and the import process can be moved from visit_ast::RustdocVisitor::visit_mod_contents
to a new function clean::inline::build_macro
or the like.
The problem with moving the macro handling code to try_inline
is that is doesn't handle re-exports from multiple namespaces (#34843).
rust/src/librustc_resolve/resolve_imports.rs
Lines 916 to 921 in 1962a70
If it wasn't for std::vec
we could get away with it for now.
I'm confused. Does tcx.module_exports
(called in visit_mod_contents
to grab the macro exports) do something different then? If it can find the vec!
macro that way, but still finds the module through the regular alloc::vec
module export from its export statement (when creating the module item in the std docs), how does that come into play? If tcx.module_exports
can get all the namespaces of an import and create distinct Def
s for them all, why aren't we using that?