Skip to content

[rustdoc] Add support new bang macro kinds #145458

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
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
114 changes: 80 additions & 34 deletions src/librustdoc/clean/inline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,27 @@ pub(crate) fn try_inline(
attrs: Option<(&[hir::Attribute], Option<LocalDefId>)>,
visited: &mut DefIdSet,
) -> Option<Vec<clean::Item>> {
fn try_inline_inner(
cx: &mut DocContext<'_>,
kind: clean::ItemKind,
did: DefId,
name: Symbol,
import_def_id: Option<LocalDefId>,
) -> clean::Item {
cx.inlined.insert(did.into());
let mut item = crate::clean::generate_item_with_correct_attrs(
cx,
kind,
did,
name,
import_def_id.as_slice(),
None,
);
// The visibility needs to reflect the one from the reexport and not from the "source" DefId.
item.inner.inline_stmt_id = import_def_id;
item
}

let did = res.opt_def_id()?;
if did.is_local() {
return None;
Expand Down Expand Up @@ -138,34 +159,31 @@ pub(crate) fn try_inline(
})
}
Res::Def(DefKind::Macro(kinds), did) => {
let mac = build_macro(cx, did, name, kinds);
let (mac, others) = build_macro(cx, did, name, kinds);

// FIXME: handle attributes and derives that aren't proc macros, and macros with
// multiple kinds
let type_kind = match kinds {
MacroKinds::BANG => ItemType::Macro,
MacroKinds::ATTR => ItemType::ProcAttribute,
MacroKinds::DERIVE => ItemType::ProcDerive,
_ => todo!("Handle macros with multiple kinds"),
_ if kinds.contains(MacroKinds::BANG) => ItemType::Macro,
_ => panic!("unsupported macro kind {kinds:?}"),
};
record_extern_fqn(cx, did, type_kind);
mac
let first = try_inline_inner(cx, mac, did, name, import_def_id);
if let Some(others) = others {
for mac_kind in others {
let mut mac = first.clone();
mac.inner.kind = mac_kind;
ret.push(mac);
}
}
ret.push(first);
return Some(ret);
}
_ => return None,
};

cx.inlined.insert(did.into());
let mut item = crate::clean::generate_item_with_correct_attrs(
cx,
kind,
did,
name,
import_def_id.as_slice(),
None,
);
// The visibility needs to reflect the one from the reexport and not from the "source" DefId.
item.inner.inline_stmt_id = import_def_id;
ret.push(item);
ret.push(try_inline_inner(cx, kind, did, name, import_def_id));
Some(ret)
}

Expand Down Expand Up @@ -753,24 +771,52 @@ fn build_macro(
def_id: DefId,
name: Symbol,
macro_kinds: MacroKinds,
) -> clean::ItemKind {
) -> (clean::ItemKind, Option<Vec<clean::ItemKind>>) {
match CStore::from_tcx(cx.tcx).load_macro_untracked(def_id, cx.tcx) {
// FIXME: handle attributes and derives that aren't proc macros, and macros with multiple
// kinds
LoadedMacro::MacroDef { def, .. } => match macro_kinds {
MacroKinds::BANG => clean::MacroItem(clean::Macro {
source: utils::display_macro_source(cx, name, &def),
macro_rules: def.macro_rules,
}),
MacroKinds::DERIVE => clean::ProcMacroItem(clean::ProcMacro {
kind: MacroKind::Derive,
helpers: Vec::new(),
}),
MacroKinds::ATTR => clean::ProcMacroItem(clean::ProcMacro {
kind: MacroKind::Attr,
helpers: Vec::new(),
}),
_ => todo!("Handle macros with multiple kinds"),
MacroKinds::BANG => (
clean::MacroItem(
clean::Macro {
source: utils::display_macro_source(cx, name, &def),
macro_rules: def.macro_rules,
},
None,
),
None,
),
MacroKinds::DERIVE => (
clean::ProcMacroItem(clean::ProcMacro {
kind: MacroKind::Derive,
helpers: Vec::new(),
}),
None,
),
MacroKinds::ATTR => (
clean::ProcMacroItem(clean::ProcMacro {
kind: MacroKind::Attr,
helpers: Vec::new(),
}),
None,
),
_ if macro_kinds.contains(MacroKinds::BANG) => {
let kind = clean::MacroItem(
clean::Macro {
source: utils::display_macro_source(cx, name, &def),
macro_rules: def.macro_rules,
},
Some(macro_kinds),
);
let mut ret = vec![];
for kind in macro_kinds.iter().filter(|kind| *kind != MacroKinds::BANG) {
match kind {
MacroKinds::ATTR => ret.push(clean::AttrMacroItem),
MacroKinds::DERIVE => ret.push(clean::DeriveMacroItem),
_ => panic!("unsupported macro kind {kind:?}"),
}
}
(kind, Some(ret))
}
_ => panic!("unsupported macro kind {macro_kinds:?}"),
},
LoadedMacro::ProcMacro(ext) => {
// Proc macros can only have a single kind
Expand All @@ -780,7 +826,7 @@ fn build_macro(
MacroKinds::DERIVE => MacroKind::Derive,
_ => unreachable!(),
};
clean::ProcMacroItem(clean::ProcMacro { kind, helpers: ext.helper_attrs })
(clean::ProcMacroItem(clean::ProcMacro { kind, helpers: ext.helper_attrs }), None)
}
}
}
Expand Down
61 changes: 48 additions & 13 deletions src/librustdoc/clean/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2845,19 +2845,54 @@ fn clean_maybe_renamed_item<'tcx>(
generics: clean_generics(generics, cx),
fields: variant_data.fields().iter().map(|x| clean_field(x, cx)).collect(),
}),
// FIXME: handle attributes and derives that aren't proc macros, and macros with
// multiple kinds
ItemKind::Macro(_, macro_def, MacroKinds::BANG) => MacroItem(Macro {
source: display_macro_source(cx, name, macro_def),
macro_rules: macro_def.macro_rules,
}),
ItemKind::Macro(_, _, MacroKinds::ATTR) => {
clean_proc_macro(item, &mut name, MacroKind::Attr, cx)
}
ItemKind::Macro(_, _, MacroKinds::DERIVE) => {
clean_proc_macro(item, &mut name, MacroKind::Derive, cx)
}
ItemKind::Macro(_, _, _) => todo!("Handle macros with multiple kinds"),
ItemKind::Macro(_, macro_def, kinds) => match kinds {
MacroKinds::BANG => MacroItem(
Macro {
source: display_macro_source(cx, name, macro_def),
macro_rules: macro_def.macro_rules,
},
None,
Copy link
Contributor

Choose a reason for hiding this comment

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

Why None and not just MacroKinds::BANG ?

),
MacroKinds::ATTR => clean_proc_macro(item, &mut name, MacroKind::Attr, cx),
MacroKinds::DERIVE => clean_proc_macro(item, &mut name, MacroKind::Derive, cx),
_ if kinds.contains(MacroKinds::BANG) => {
let kind = MacroItem(
Macro {
source: display_macro_source(cx, name, macro_def),
macro_rules: macro_def.macro_rules,
},
Some(kinds),
);
let mac = generate_item_with_correct_attrs(
cx,
kind,
item.owner_id.def_id.to_def_id(),
name,
import_ids,
renamed,
);

let mut ret = Vec::with_capacity(3);
for kind in kinds.iter().filter(|kind| *kind != MacroKinds::BANG) {
match kind {
MacroKinds::ATTR => {
let mut attr = mac.clone();
attr.inner.kind = AttrMacroItem;
ret.push(attr);
}
MacroKinds::DERIVE => {
let mut derive = mac.clone();
derive.inner.kind = DeriveMacroItem;
ret.push(derive);
}
_ => panic!("unsupported macro kind {kind:?}"),
}
}
ret.push(mac);
return ret;
}
_ => panic!("unsupported macro kind {kinds:?}"),
},
// proc macros can have a name set by attributes
ItemKind::Fn { ref sig, generics, body: body_id, .. } => {
clean_fn_or_proc_macro(item, sig, generics, body_id, &mut name, cx)
Expand Down
24 changes: 20 additions & 4 deletions src/librustdoc/clean/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use itertools::Either;
use rustc_abi::{ExternAbi, VariantIdx};
use rustc_data_structures::fx::{FxHashSet, FxIndexMap, FxIndexSet};
use rustc_hir::attrs::{AttributeKind, DeprecatedSince, Deprecation};
use rustc_hir::def::{CtorKind, DefKind, Res};
use rustc_hir::def::{CtorKind, DefKind, MacroKinds, Res};
use rustc_hir::def_id::{CrateNum, DefId, LOCAL_CRATE, LocalDefId};
use rustc_hir::lang_items::LangItem;
use rustc_hir::{BodyId, ConstStability, Mutability, Stability, StableSince, find_attr};
Expand Down Expand Up @@ -642,6 +642,18 @@ impl Item {
ItemType::from(self)
}

/// Generates the HTML file name based on the item kind.
pub(crate) fn html_filename(&self) -> String {
let type_ = if self.is_macro_placeholder() { ItemType::Macro } else { self.type_() };
format!("{type_}.{}.html", self.name.unwrap())
}

/// If the current item is a "fake" macro (ie, `AttrMacroItem | ItemKind::DeriveMacroItem` which
/// don't hold any data), it returns `true`.
pub(crate) fn is_macro_placeholder(&self) -> bool {
matches!(self.kind, ItemKind::AttrMacroItem | ItemKind::DeriveMacroItem)
}

pub(crate) fn is_default(&self) -> bool {
match self.kind {
ItemKind::MethodItem(_, Some(defaultness)) => {
Expand Down Expand Up @@ -924,7 +936,9 @@ pub(crate) enum ItemKind {
ForeignStaticItem(Static, hir::Safety),
/// `type`s from an extern block
ForeignTypeItem,
MacroItem(Macro),
MacroItem(Macro, Option<MacroKinds>),
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't like the use of Option here, for two reasons:

  1. MacroKinds::empty() already can represent the absence of any value
  2. what None actually represents here is MacroKinds::BANG

I think the rationale for the second is that this represents extra macro kinds, but I think the use of Option is still unnecessary, as it leaves MacroKinds::empty() as a meaningless value that should never be used, which seems like something we would want to avoid.

AttrMacroItem,
DeriveMacroItem,
Comment on lines +940 to +941
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like these should at the very least have a doc comment saying these are for non-proc macros.

ProcMacroItem(ProcMacro),
PrimitiveItem(PrimitiveType),
/// A required associated constant in a trait declaration.
Expand Down Expand Up @@ -974,7 +988,9 @@ impl ItemKind {
| ForeignFunctionItem(_, _)
| ForeignStaticItem(_, _)
| ForeignTypeItem
| MacroItem(_)
| MacroItem(..)
| AttrMacroItem
| DeriveMacroItem
| ProcMacroItem(_)
| PrimitiveItem(_)
| RequiredAssocConstItem(..)
Expand Down Expand Up @@ -1005,7 +1021,7 @@ impl ItemKind {
| ForeignFunctionItem(_, _)
| ForeignStaticItem(_, _)
| ForeignTypeItem
| MacroItem(_)
| MacroItem(..)
| ProcMacroItem(_)
| PrimitiveItem(_)
)
Expand Down
4 changes: 3 additions & 1 deletion src/librustdoc/fold.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,9 @@ pub(crate) trait DocFolder: Sized {
| ForeignFunctionItem(..)
| ForeignStaticItem(..)
| ForeignTypeItem
| MacroItem(_)
| MacroItem(..)
| AttrMacroItem
| DeriveMacroItem
| ProcMacroItem(_)
| PrimitiveItem(_)
| RequiredAssocConstItem(..)
Expand Down
2 changes: 2 additions & 0 deletions src/librustdoc/formats/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -373,6 +373,8 @@ impl DocFolder for CacheBuilder<'_, '_> {
| clean::RequiredAssocTypeItem(..)
| clean::AssocTypeItem(..)
| clean::StrippedItem(..)
| clean::AttrMacroItem
| clean::DeriveMacroItem
| clean::KeywordItem => {
// FIXME: Do these need handling?
// The person writing this comment doesn't know.
Expand Down
4 changes: 4 additions & 0 deletions src/librustdoc/formats/item_type.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,10 @@ impl<'a> From<&'a clean::Item> for ItemType {
clean::ForeignFunctionItem(..) => ItemType::Function, // no ForeignFunction
clean::ForeignStaticItem(..) => ItemType::Static, // no ForeignStatic
clean::MacroItem(..) => ItemType::Macro,
// Is this a good idea?
clean::AttrMacroItem => ItemType::ProcAttribute,
// Is this a good idea?
clean::DeriveMacroItem => ItemType::ProcDerive,
Comment on lines +98 to +101
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems fairly confusing to me, I don't see the advantage (is it so item_module groups things correctly? if that's the case, then the first commit on its own would not be correct, since it would categorize these macros as ItemType::Macro), and it already causes a bit of weirdness in html_filename which would not be needed if these were categorized as Macro instead.

if we did want to make this change, I would at least want to change the names of the item types to not imply they must be derives.

clean::PrimitiveItem(..) => ItemType::Primitive,
clean::RequiredAssocConstItem(..)
| clean::ProvidedAssocConstItem(..)
Expand Down
Loading
Loading