Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DO NOT MERGE] Document non-exported macros with --document-private-items #79396

Closed
wants to merge 1 commit into from

Conversation

camelid
Copy link
Member

@camelid camelid commented Nov 25, 2020

Fixes #73754.

This is in no way ready to be merged since it doesn't work yet. I would
appreciate some advice if you have time!

@camelid camelid added A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Nov 25, 2020
@rust-highfive
Copy link
Collaborator

r? @matthewjasper

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 25, 2020
@camelid camelid changed the title Document non-exported macros with --document-private-items [DO NOT MERGE] Document non-exported macros with --document-private-items Nov 25, 2020
@camelid camelid added A-HIR Area: The high-level intermediate representation (HIR) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Nov 25, 2020
@camelid
Copy link
Member Author

camelid commented Nov 25, 2020

The approach I took is to track non-exported macros in hir::Crate along with exported macros. The trouble I'm having is that I need to add some info about the private macros (non-exported macros) to the HIR, but then that triggers lints etc. that are intended for public macros. Thus, CI will fail with a spectacular 870 errors (or more) when compiling core.

This will also probably need a perf run since it's tracking additional information in the HIR, which has the potential slow the compiler down somewhat.

@camelid
Copy link
Member Author

camelid commented Nov 25, 2020

For reference, this is the error that I get when I use rustdoc with this line commented-out:

walk_list!(visitor, visit_macro_def, krate.non_exported_macros);

Errors:

❯ rustdoc +stage1 --document-private-items src/test/rustdoc/non-exported-macros.rs
error: internal compiler error: compiler/rustc_middle/src/hir/map/mod.rs:897:18: hir::map::Map::span_with_body: id not in map: HirId { owner: DefId(0:5 ~ non_exported_macros[8787]::foo_macro), local_id: 0 }

thread 'rustc' panicked at 'Box<Any>', compiler/rustc_errors/src/lib.rs:958:9
stack backtrace:
   0: std::panicking::begin_panic
   1: rustc_errors::HandlerInner::bug
   2: rustc_errors::Handler::bug
   3: rustc_middle::util::bug::opt_span_bug_fmt::{{closure}}
   4: rustc_middle::ty::context::tls::with_opt::{{closure}}
   5: rustc_middle::ty::context::tls::with_opt
   6: rustc_middle::util::bug::opt_span_bug_fmt
   7: rustc_middle::util::bug::bug_fmt
   8: rustc_middle::hir::map::Map::span_with_body
   9: rustdoc::clean::types::Item::from_def_id_and_parts::{{closure}}
             at ./src/librustdoc/clean/types.rs:141:17
  10: core::option::Option<T>::map_or_else
             at ./library/core/src/option.rs:503:24
  11: rustdoc::clean::types::Item::from_def_id_and_parts
             at ./src/librustdoc/clean/types.rs:137:22
  12: <rustdoc::doctree::Macro as rustdoc::clean::Clean<rustdoc::clean::types::Item>>::clean
             at ./src/librustdoc/clean/mod.rs:2261:9
  13: <rustdoc::doctree::Module as rustdoc::clean::Clean<rustdoc::clean::types::Item>>::clean::{{closure}}
             at ./src/librustdoc/clean/mod.rs:236:49
  14: core::iter::adapters::map::map_fold::{{closure}}
             at ./library/core/src/iter/adapters/map.rs:80:28
  15: core::iter::traits::iterator::Iterator::fold
             at ./library/core/src/iter/traits/iterator.rs:2023:21
  16: <core::iter::adapters::map::Map<I,F> as core::iter::traits::iterator::Iterator>::fold
             at ./library/core/src/iter/adapters/map.rs:120:9
  17: core::iter::traits::iterator::Iterator::for_each
             at ./library/core/src/iter/traits/iterator.rs:678:9
  18: <alloc::vec::Vec<T,A> as alloc::vec::SpecExtend<T,I>>::spec_extend
             at ./library/alloc/src/vec.rs:2565:17
  19: <alloc::vec::Vec<T,A> as core::iter::traits::collect::Extend<T>>::extend
             at ./library/alloc/src/vec.rs:2251:9
  20: <rustdoc::doctree::Module as rustdoc::clean::Clean<rustdoc::clean::types::Item>>::clean
             at ./src/librustdoc/clean/mod.rs:236:9
  21: rustdoc::clean::utils::krate
             at ./src/librustdoc/clean/utils.rs:44:22
  22: rustdoc::core::run_global_ctxt::{{closure}}
             at ./src/librustdoc/core.rs:548:53
  23: rustc_data_structures::profiling::VerboseTimingGuard::run
             at ./compiler/rustc_data_structures/src/profiling.rs:570:9
  24: rustc_session::utils::<impl rustc_session::session::Session>::time
             at ./compiler/rustc_session/src/utils.rs:9:9
  25: rustdoc::core::run_global_ctxt
             at ./src/librustdoc/core.rs:548:21
  26: rustdoc::core::run_core::{{closure}}::{{closure}}::{{closure}}::{{closure}}
             at ./src/librustdoc/core.rs:458:21
  27: rustc_interface::passes::QueryContext::enter::{{closure}}
             at ./compiler/rustc_interface/src/passes.rs:725:42
  28: rustc_middle::ty::context::tls::enter_context::{{closure}}
             at ./compiler/rustc_middle/src/ty/context.rs:1739:50
  29: rustc_middle::ty::context::tls::set_tlv
             at ./compiler/rustc_middle/src/ty/context.rs:1723:9
  30: rustc_middle::ty::context::tls::enter_context
             at ./compiler/rustc_middle/src/ty/context.rs:1739:9
  31: rustc_interface::passes::QueryContext::enter
             at ./compiler/rustc_interface/src/passes.rs:725:9
  32: rustdoc::core::run_core::{{closure}}::{{closure}}::{{closure}}
             at ./src/librustdoc/core.rs:457:17
  33: rustc_data_structures::profiling::VerboseTimingGuard::run
             at ./compiler/rustc_data_structures/src/profiling.rs:570:9
  34: rustc_session::utils::<impl rustc_session::session::Session>::time
             at ./compiler/rustc_session/src/utils.rs:9:9
  35: rustdoc::core::run_core::{{closure}}::{{closure}}
             at ./src/librustdoc/core.rs:456:46
  36: rustc_interface::queries::<impl rustc_interface::interface::Compiler>::enter
             at ./compiler/rustc_interface/src/queries.rs:415:19
  37: rustdoc::core::run_core::{{closure}}
             at ./src/librustdoc/core.rs:415:9
  38: rustc_interface::interface::create_compiler_and_run::{{closure}}
             at ./compiler/rustc_interface/src/interface.rs:196:13
  39: rustc_span::with_source_map
             at ./compiler/rustc_span/src/lib.rs:764:5
  40: rustc_interface::interface::create_compiler_and_run
             at ./compiler/rustc_interface/src/interface.rs:190:5
  41: rustdoc::core::run_core
             at ./src/librustdoc/core.rs:414:5
  42: rustdoc::main_options
             at ./src/librustdoc/lib.rs:531:53
  43: rustdoc::main_args::{{closure}}
             at ./src/librustdoc/lib.rs:464:17
  44: rustc_interface::util::setup_callbacks_and_run_in_thread_pool_with_globals::{{closure}}::{{closure}}
             at ./compiler/rustc_interface/src/util.rs:152:13
  45: scoped_tls::ScopedKey<T>::set
             at /Users/user/.cargo/registry/src/github.com-1ecc6299db9ec823/scoped-tls-1.0.0/src/lib.rs:137:9
  46: rustc_span::with_session_globals
             at ./compiler/rustc_span/src/lib.rs:93:5
  47: rustc_interface::util::setup_callbacks_and_run_in_thread_pool_with_globals::{{closure}}
             at ./compiler/rustc_interface/src/util.rs:150:9
  48: rustc_interface::util::scoped_thread::{{closure}}
             at ./compiler/rustc_interface/src/util.rs:125:24
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

error: Unrecognized option: 'document-private-items'

error: aborting due to previous error

This is the file I'm trying to document:

// compile-flags: --document-private-items

fn foo_fn() {}

pub fn pub_foo_fn() {}

macro_rules! foo_macro {
    () => { };
}

#[macro_export]
macro_rules! exported_foo_macro {
    () => { };
}

// TODO: add `@has` checks

@petrochenkov
Copy link
Contributor

I'll copypaste my answer to @danielhenrymantilla , who was going to implement similar things.

Hi.
Regarding #77862 "Rustdoc: Fix macros 2.0 and built-in derives being shown at the wrong path".
I'm pretty sure that the rustdoc-based solution is a workaround and technical debt, I just didn't have time to properly suggest anything better.

For modularized macros (macro and pub macro_rules, if they get merged) their source parent module (the module into which they are placed in source code) and their name-resolution parent module (parent::my_macro) coincide , so preserving these macro items in HIR would be the most natural way to give these items to rustdoc (and perhaps save-analysis as well), in both regular and document-private-items rustdoc mode.
Rustdoc then wouldn't require any special treatment of these macros compared to other items.

macro_rules without #[macro_export] are only documented in document-private-items mode, and correct location for documenting them doesn't really exist, so documenting them in the module in which they are written in source code would be good enough.
That means preserving these macros in HIR and treating exactly like the macro items mentioned above would be the simplest solution.

#[macro_export] macro_rules are the most annoying case.
They may be defined in some module in source code, but they still need to be documented documented in the crate root by rustdoc (and also encoded as placed into the crate root in metadata).
I'm not sure what is the cleanest way to address them, perhaps moving them all into the root module during lowering to HIR would work?

@bors
Copy link
Contributor

bors commented Nov 29, 2020

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

Note that reviewers usually do not review pull requests until merge conflicts are resolved! Once you resolve the conflicts, you should change the labels applied by bors to indicate that your PR is ready for review. Post this as a comment to change the labels:

@rustbot modify labels: +S-waiting-on-review -S-waiting-on-author

@jyn514
Copy link
Member

jyn514 commented Apr 16, 2021

@camelid @petrochenkov It sounds like the current approach both doesn't work and adds technical debt that would have to be fixed later, so I'm going to close this. I'll copy @petrochenkov's advice for the proper way to do this to the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-HIR Area: The high-level intermediate representation (HIR) A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[rustdoc] Private macros are not documented with --document-private-items
6 participants