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

rustdoc: Treat declarative macros more like other item kinds #132302

Merged
merged 1 commit into from
Nov 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 5 additions & 13 deletions src/librustdoc/clean/inline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,11 +134,7 @@ pub(crate) fn try_inline(
})
}
Res::Def(DefKind::Macro(kind), did) => {
let is_doc_hidden = cx.tcx.is_doc_hidden(did)
|| attrs_without_docs
.map(|(attrs, _)| attrs)
.is_some_and(|attrs| utils::attrs_have_doc_flag(attrs.iter(), sym::hidden));
let mac = build_macro(cx, did, name, import_def_id, kind, is_doc_hidden);
let mac = build_macro(cx, did, name, kind);

let type_kind = match kind {
MacroKind::Bang => ItemType::Macro,
Expand Down Expand Up @@ -740,18 +736,14 @@ fn build_macro(
cx: &mut DocContext<'_>,
def_id: DefId,
name: Symbol,
import_def_id: Option<LocalDefId>,
macro_kind: MacroKind,
is_doc_hidden: bool,
) -> clean::ItemKind {
match CStore::from_tcx(cx.tcx).load_macro_untracked(def_id, cx.tcx) {
LoadedMacro::MacroDef { def, .. } => match macro_kind {
MacroKind::Bang => {
let vis = cx.tcx.visibility(import_def_id.map(|d| d.to_def_id()).unwrap_or(def_id));
clean::MacroItem(clean::Macro {
source: utils::display_macro_source(cx, name, &def, def_id, vis, is_doc_hidden),
})
}
MacroKind::Bang => clean::MacroItem(clean::Macro {
source: utils::display_macro_source(cx, name, &def),
macro_rules: def.macro_rules,
}),
MacroKind::Derive | MacroKind::Attr => {
clean::ProcMacroItem(clean::ProcMacro { kind: macro_kind, helpers: Vec::new() })
}
Expand Down
11 changes: 4 additions & 7 deletions src/librustdoc/clean/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2787,13 +2787,10 @@ fn clean_maybe_renamed_item<'tcx>(
fields: variant_data.fields().iter().map(|x| clean_field(x, cx)).collect(),
}),
ItemKind::Impl(impl_) => return clean_impl(impl_, item.owner_id.def_id, cx),
ItemKind::Macro(macro_def, MacroKind::Bang) => {
let ty_vis = cx.tcx.visibility(def_id);
MacroItem(Macro {
// FIXME this shouldn't be false
source: display_macro_source(cx, name, macro_def, def_id, ty_vis, false),
})
}
ItemKind::Macro(macro_def, MacroKind::Bang) => MacroItem(Macro {
source: display_macro_source(cx, name, macro_def),
macro_rules: macro_def.macro_rules,
}),
ItemKind::Macro(_, macro_kind) => clean_proc_macro(item, &mut name, macro_kind, cx),
// proc macros can have a name set by attributes
ItemKind::Fn(ref sig, generics, body_id) => {
Expand Down
2 changes: 2 additions & 0 deletions src/librustdoc/clean/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2543,6 +2543,8 @@ pub(crate) struct ImportSource {
#[derive(Clone, Debug)]
pub(crate) struct Macro {
pub(crate) source: String,
/// Whether the macro was defined via `macro_rules!` as opposed to `macro`.
pub(crate) macro_rules: bool,
}

#[derive(Clone, Debug)]
Expand Down
15 changes: 3 additions & 12 deletions src/librustdoc/clean/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ use crate::clean::{
clean_middle_ty, inline,
};
use crate::core::DocContext;
use crate::html::format::visibility_to_src_with_space;

#[cfg(test)]
mod tests;
Expand Down Expand Up @@ -599,7 +598,7 @@ pub(crate) static DOC_CHANNEL: Lazy<&'static str> =

/// Render a sequence of macro arms in a format suitable for displaying to the user
/// as part of an item declaration.
pub(super) fn render_macro_arms<'a>(
fn render_macro_arms<'a>(
tcx: TyCtxt<'_>,
matchers: impl Iterator<Item = &'a TokenTree>,
arm_delim: &str,
Expand All @@ -620,9 +619,6 @@ pub(super) fn display_macro_source(
cx: &mut DocContext<'_>,
name: Symbol,
def: &ast::MacroDef,
def_id: DefId,
vis: ty::Visibility<DefId>,
is_doc_hidden: bool,
) -> String {
// Extract the spans of all matchers. They represent the "interface" of the macro.
let matchers = def.body.tokens.chunks(4).map(|arm| &arm[0]);
Expand All @@ -632,18 +628,13 @@ pub(super) fn display_macro_source(
} else {
if matchers.len() <= 1 {
format!(
"{vis}macro {name}{matchers} {{\n ...\n}}",
vis = visibility_to_src_with_space(Some(vis), cx.tcx, def_id, is_doc_hidden),
"macro {name}{matchers} {{\n ...\n}}",
matchers = matchers
.map(|matcher| render_macro_matcher(cx.tcx, matcher))
.collect::<String>(),
)
} else {
format!(
"{vis}macro {name} {{\n{arms}}}",
vis = visibility_to_src_with_space(Some(vis), cx.tcx, def_id, is_doc_hidden),
arms = render_macro_arms(cx.tcx, matchers, ","),
)
format!("macro {name} {{\n{arms}}}", arms = render_macro_arms(cx.tcx, matchers, ","))
}
}
}
Expand Down
41 changes: 0 additions & 41 deletions src/librustdoc/html/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1615,47 +1615,6 @@ pub(crate) fn visibility_print_with_space<'a, 'tcx: 'a>(
})
}

/// This function is the same as print_with_space, except that it renders no links.
/// It's used for macros' rendered source view, which is syntax highlighted and cannot have
/// any HTML in it.
pub(crate) fn visibility_to_src_with_space<'a, 'tcx: 'a>(
visibility: Option<ty::Visibility<DefId>>,
tcx: TyCtxt<'tcx>,
item_did: DefId,
is_doc_hidden: bool,
) -> impl Display + 'a + Captures<'tcx> {
let vis: Cow<'static, str> = match visibility {
None => "".into(),
Some(ty::Visibility::Public) => "pub ".into(),
Some(ty::Visibility::Restricted(vis_did)) => {
// FIXME(camelid): This may not work correctly if `item_did` is a module.
// However, rustdoc currently never displays a module's
// visibility, so it shouldn't matter.
let parent_module = find_nearest_parent_module(tcx, item_did);

if vis_did.is_crate_root() {
"pub(crate) ".into()
} else if parent_module == Some(vis_did) {
// `pub(in foo)` where `foo` is the parent module
// is the same as no visibility modifier
"".into()
} else if parent_module.and_then(|parent| find_nearest_parent_module(tcx, parent))
== Some(vis_did)
{
"pub(super) ".into()
} else {
format!("pub(in {}) ", tcx.def_path_str(vis_did)).into()
}
}
};
display_fn(move |f| {
if is_doc_hidden {
f.write_str("#[doc(hidden)] ")?;
}
f.write_str(&vis)
})
}

pub(crate) trait PrintWithSpace {
fn print_with_space(&self) -> &str;
}
Expand Down
7 changes: 0 additions & 7 deletions src/librustdoc/html/highlight.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,13 +58,6 @@ pub(crate) fn render_example_with_highlighting(
write_footer(out, playground_button);
}

/// Highlights `src` as an item-decl, returning the HTML output.
pub(crate) fn render_item_decl_with_highlighting(src: &str, out: &mut Buffer) {
write!(out, "<pre class=\"rust item-decl\">");
write_code(out, src, None, None);
write!(out, "</pre>");
}

fn write_header(
out: &mut Buffer,
class: &str,
Expand Down
9 changes: 7 additions & 2 deletions src/librustdoc/html/render/print_item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ use crate::html::format::{
Buffer, Ending, PrintWithSpace, display_fn, join_with_double_colon, print_abi_with_space,
print_constness_with_space, print_where_clause, visibility_print_with_space,
};
use crate::html::highlight;
use crate::html::markdown::{HeadingOffset, MarkdownSummaryLine};
use crate::html::render::{document_full, document_item_info};
use crate::html::url_parts_builder::UrlPartsBuilder;
Expand Down Expand Up @@ -1745,7 +1744,13 @@ fn item_variants(
}

fn item_macro(w: &mut Buffer, cx: &mut Context<'_>, it: &clean::Item, t: &clean::Macro) {
highlight::render_item_decl_with_highlighting(&t.source, w);
wrap_item(w, |w| {
// FIXME: Also print `#[doc(hidden)]` for `macro_rules!` if it `is_doc_hidden`.
Copy link
Member Author

@fmease fmease Oct 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Preexisting, will fix separately bc. there's another bug related to this that concerns all item kinds: #132304. Context: visibility_print_with_space not only renders an item's vis but also its #[doc(hidden)] if applicable.

if !t.macro_rules {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(pub... macro_rules! was once proposed as a middle ground between decl macros 1.2 and 2.0; however, this proposal was rejected if I remember correctly)

write!(w, "{}", visibility_print_with_space(it, cx));
}
write!(w, "{}", Escape(&t.source));
});
write!(w, "{}", document(cx, it, None, HeadingOffset::H2))
}

Expand Down
2 changes: 2 additions & 0 deletions tests/rustdoc/decl_macro.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ mod a {
}
mod c {
//@ has decl_macro/a/b/c/macro.by_example_vis_named.html //pre 'pub(in a) macro by_example_vis_named($foo:expr) {'
// Regression test for <https://github.com/rust-lang/rust/issues/83000>:
//@ has - '//pre[@class="rust item-decl"]//a[@class="mod"]/@href' '../../index.html'
pub(in a) macro by_example_vis_named {
($foo:expr) => {}
}
Expand Down
28 changes: 8 additions & 20 deletions tests/rustdoc/macro_rules-matchers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,29 +3,17 @@

#![crate_name = "foo"]

//@ has 'foo/macro.todo.html'
//@ has - '//span[@class="macro"]' 'macro_rules!'
//@ hasraw - ' todo {'

//@ hasraw - '{ () =&gt; { ... }; ($('
//@ has - '//span[@class="macro-nonterminal"]' '$'
//@ has - '//span[@class="macro-nonterminal"]' 'arg'
//@ hasraw - ':tt)+'
//@ hasraw - ') =&gt; { ... }; }'
//@ has 'foo/macro.todo.html' '//pre' 'macro_rules! todo { \
// () => { ... }; \
// ($($arg:tt)+) => { ... }; \
// }'
pub use std::todo;

mod mod1 {
//@ has 'foo/macro.macro1.html'
//@ hasraw - 'macro_rules!'
//@ hasraw - 'macro1'
//@ hasraw - '{ () =&gt; { ... }; ($('
//@ has - '//span[@class="macro-nonterminal"]' '$'
//@ has - '//span[@class="macro-nonterminal"]' 'arg'
//@ hasraw - ':'
//@ hasraw - 'expr'
//@ hasraw - '),'
//@ hasraw - '+'
//@ hasraw - ') =&gt; { ... }; }'
//@ has 'foo/macro.macro1.html' '//pre' 'macro_rules! macro1 { \
// () => { ... }; \
// ($($arg:expr),+) => { ... }; \
// }'
#[macro_export]
macro_rules! macro1 {
() => {};
Expand Down
Loading