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: Correctly handle built-in compiler proc-macros as proc-macro and not macro #110279

Merged
merged 2 commits into from
Apr 14, 2023
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
26 changes: 16 additions & 10 deletions src/librustdoc/clean/inline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ pub(crate) fn try_inline(
clean::ConstantItem(build_const(cx, did))
}
Res::Def(DefKind::Macro(kind), did) => {
let mac = build_macro(cx, did, name, import_def_id);
let mac = build_macro(cx, did, name, import_def_id, kind);

let type_kind = match kind {
MacroKind::Bang => ItemType::Macro,
Expand Down Expand Up @@ -651,18 +651,24 @@ fn build_macro(
def_id: DefId,
name: Symbol,
import_def_id: Option<DefId>,
macro_kind: MacroKind,
) -> clean::ItemKind {
match CStore::from_tcx(cx.tcx).load_macro_untracked(def_id, cx.sess()) {
LoadedMacro::MacroDef(item_def, _) => {
if let ast::ItemKind::MacroDef(ref def) = item_def.kind {
let vis = cx.tcx.visibility(import_def_id.unwrap_or(def_id));
clean::MacroItem(clean::Macro {
source: utils::display_macro_source(cx, name, def, def_id, vis),
})
} else {
unreachable!()
LoadedMacro::MacroDef(item_def, _) => match macro_kind {
MacroKind::Bang => {
if let ast::ItemKind::MacroDef(ref def) = item_def.kind {
let vis = cx.tcx.visibility(import_def_id.unwrap_or(def_id));
clean::MacroItem(clean::Macro {
source: utils::display_macro_source(cx, name, def, def_id, vis),
})
} else {
unreachable!()
}
}
}
MacroKind::Derive | MacroKind::Attr => {
clean::ProcMacroItem(clean::ProcMacro { kind: macro_kind, helpers: Vec::new() })
}
},
LoadedMacro::ProcMacro(ext) => clean::ProcMacroItem(clean::ProcMacro {
kind: ext.macro_kind(),
helpers: ext.helper_attrs,
Expand Down
69 changes: 39 additions & 30 deletions src/librustdoc/clean/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -909,6 +909,38 @@ fn clean_ty_generics<'tcx>(
}
}

fn clean_proc_macro<'tcx>(
item: &hir::Item<'tcx>,
name: &mut Symbol,
kind: MacroKind,
cx: &mut DocContext<'tcx>,
) -> ItemKind {
let attrs = cx.tcx.hir().attrs(item.hir_id());
if kind == MacroKind::Derive &&
let Some(derive_name) = attrs
.lists(sym::proc_macro_derive)
.find_map(|mi| mi.ident())
{
*name = derive_name.name;
}

let mut helpers = Vec::new();
for mi in attrs.lists(sym::proc_macro_derive) {
if !mi.has_name(sym::attributes) {
continue;
}

if let Some(list) = mi.meta_item_list() {
for inner_mi in list {
if let Some(ident) = inner_mi.ident() {
helpers.push(ident.name);
}
}
}
}
ProcMacroItem(ProcMacro { kind, helpers })
}

fn clean_fn_or_proc_macro<'tcx>(
item: &hir::Item<'tcx>,
sig: &hir::FnSig<'tcx>,
Expand All @@ -930,31 +962,7 @@ fn clean_fn_or_proc_macro<'tcx>(
}
});
match macro_kind {
Some(kind) => {
if kind == MacroKind::Derive {
*name = attrs
.lists(sym::proc_macro_derive)
.find_map(|mi| mi.ident())
.expect("proc-macro derives require a name")
.name;
}

let mut helpers = Vec::new();
for mi in attrs.lists(sym::proc_macro_derive) {
if !mi.has_name(sym::attributes) {
continue;
}

if let Some(list) = mi.meta_item_list() {
for inner_mi in list {
if let Some(ident) = inner_mi.ident() {
helpers.push(ident.name);
}
}
}
}
ProcMacroItem(ProcMacro { kind, helpers })
}
Some(kind) => clean_proc_macro(item, name, kind, cx),
None => {
let mut func = clean_function(cx, sig, generics, FunctionArgs::Body(body_id));
clean_fn_decl_legacy_const_generics(&mut func, attrs);
Expand Down Expand Up @@ -2247,16 +2255,17 @@ 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),
// proc macros can have a name set by attributes
ItemKind::Fn(ref sig, generics, body_id) => {
clean_fn_or_proc_macro(item, sig, generics, body_id, &mut name, cx)
}
ItemKind::Macro(ref macro_def, _) => {
ItemKind::Macro(ref macro_def, MacroKind::Bang) => {
let ty_vis = cx.tcx.visibility(def_id);
MacroItem(Macro {
source: display_macro_source(cx, name, macro_def, def_id, ty_vis),
})
}
ItemKind::Macro(_, macro_kind) => clean_proc_macro(item, &mut name, macro_kind, cx),
Copy link
Member

@fmease fmease Apr 13, 2023

Choose a reason for hiding this comment

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

So as far as I understand we treat non-bang macro declarations (macro or macro_rules!) as proc macros because we assume that they have #[rustc_builtin_macro] on them and since currently there is no way to define attributes and derive macros via declarative macros, right?

Seems fine to me, although that fact could change in the far™ future. If that happened we'd "need" new ItemTypes anyway.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, as you can see in the test I updated. We'll see if it ever happen but I never heard of it (if you have a link to a discussion about it, I'm very interested!).

Copy link
Member

@fmease fmease Apr 13, 2023

Choose a reason for hiding this comment

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

Nothing to link to, no. I faintly remember reading discussions about it someplace sometime ago.

// proc macros can have a name set by attributes
ItemKind::Fn(ref sig, generics, body_id) => {
clean_fn_or_proc_macro(item, sig, generics, body_id, &mut name, cx)
}
ItemKind::Trait(_, _, generics, bounds, item_ids) => {
let items = item_ids
.iter()
Expand Down
15 changes: 15 additions & 0 deletions tests/rustdoc/compiler-derive-proc-macro.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// This test ensures that compiler builtin proc-macros are considered as such.

#![crate_name = "foo"]

// @has 'foo/index.html'
// Each compiler builtin proc-macro has a trait equivalent so we should have
// a trait section as well.
// @count - '//*[@id="main-content"]//*[@class="small-section-header"]' 2
// @has - '//*[@id="main-content"]//*[@class="small-section-header"]' 'Traits'
// @has - '//*[@id="main-content"]//*[@class="small-section-header"]' 'Derive Macros'

// Now checking the correct file is generated as well.
// @has 'foo/derive.Clone.html'
// @!has 'foo/macro.Clone.html'
pub use std::clone::Clone;
12 changes: 8 additions & 4 deletions tests/rustdoc/macro_pub_in_module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,22 +7,26 @@
#![crate_name = "krate"]
#![no_core]

// @has external_crate/some_module/macro.external_macro.html
// @!has external_crate/macro.external_macro.html
// @has external_crate/some_module/macro.external_macro.html
// @!has external_crate/macro.external_macro.html
extern crate external_crate;

pub mod inner {
// @has krate/inner/macro.raw_const.html
// @!has krate/macro.raw_const.html
pub macro raw_const() {}

// @has krate/inner/macro.test.html
// @has krate/inner/attr.test.html
// @!has krate/macro.test.html
GuillaumeGomez marked this conversation as resolved.
Show resolved Hide resolved
// @!has krate/inner/macro.test.html
// @!has krate/attr.test.html
#[rustc_builtin_macro]
pub macro test($item:item) {}

// @has krate/inner/macro.Clone.html
// @has krate/inner/derive.Clone.html
// @!has krate/inner/macro.Clone.html
// @!has krate/macro.Clone.html
GuillaumeGomez marked this conversation as resolved.
Show resolved Hide resolved
// @!has krate/derive.Clone.html
#[rustc_builtin_macro]
pub macro Clone($item:item) {}

Expand Down