Skip to content
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
56 changes: 33 additions & 23 deletions src/librustdoc/clean/inline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,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 @@ -139,32 +160,21 @@ pub(crate) fn try_inline(
Res::Def(DefKind::Macro(kinds), did) => {
let mac = build_macro(cx.tcx, 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"),
// Then it means it's more than one type so we default to "macro".
_ => ItemType::Macro,
};
record_extern_fqn(cx, did, type_kind);
mac
ret.push(try_inline_inner(cx, mac, did, name, import_def_id));
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 @@ -777,13 +787,7 @@ fn build_macro(
macro_kinds: MacroKinds,
) -> clean::ItemKind {
match CStore::from_tcx(tcx).load_macro_untracked(tcx, def_id) {
// 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(tcx, name, &def),
macro_rules: def.macro_rules,
}),
MacroKinds::DERIVE => clean::ProcMacroItem(clean::ProcMacro {
kind: MacroKind::Derive,
helpers: Vec::new(),
Expand All @@ -792,7 +796,13 @@ fn build_macro(
kind: MacroKind::Attr,
helpers: Vec::new(),
}),
_ => todo!("Handle macros with multiple kinds"),
_ => clean::MacroItem(
clean::Macro {
source: utils::display_macro_source(tcx, name, &def),
macro_rules: def.macro_rules,
},
macro_kinds,
),
},
LoadedMacro::ProcMacro(ext) => {
// Proc macros can only have a single kind
Expand Down
24 changes: 11 additions & 13 deletions src/librustdoc/clean/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2898,19 +2898,17 @@ 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.tcx, name, macro_def),
macro_rules: macro_def.macro_rules,
}),
ItemKind::Macro(_, _, MacroKinds::ATTR) => {
clean_proc_macro(item, &mut name, MacroKind::Attr, cx.tcx)
}
ItemKind::Macro(_, _, MacroKinds::DERIVE) => {
clean_proc_macro(item, &mut name, MacroKind::Derive, cx.tcx)
}
ItemKind::Macro(_, _, _) => todo!("Handle macros with multiple kinds"),
ItemKind::Macro(_, macro_def, kinds) => match kinds {
MacroKinds::ATTR => clean_proc_macro(item, &mut name, MacroKind::Attr, cx.tcx),
MacroKinds::DERIVE => clean_proc_macro(item, &mut name, MacroKind::Derive, cx.tcx),
Comment on lines +2902 to +2903
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think this is wrong, a macro_rules! macro should be able to go into this branch, which does not seem ideal.

Do we really not have a way of just... checking if something is a proc macro or not? can clean_proc_macro handle being given a non-proc macro?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

In this case, if macro_rules! containing only an attr or a derive, we treat them the same as their proc-macro equivalent.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

is that the desired behavior? it seems somewhat odd.

_ => MacroItem(
Macro {
source: display_macro_source(cx.tcx, name, macro_def),
macro_rules: macro_def.macro_rules,
},
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
41 changes: 37 additions & 4 deletions src/librustdoc/clean/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use rustc_data_structures::fx::{FxHashSet, FxIndexMap, FxIndexSet};
use rustc_data_structures::thin_vec::ThinVec;
use rustc_hir as hir;
use rustc_hir::attrs::{AttributeKind, DeprecatedSince, Deprecation, DocAttribute};
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::{Attribute, BodyId, ConstStability, Mutability, Stability, StableSince, find_attr};
Expand Down Expand Up @@ -745,11 +745,33 @@ impl Item {
find_attr!(&self.attrs.other_attrs, NonExhaustive(..))
}

/// Returns a documentation-level item type from the item.
/// Returns a documentation-level item type from the item. In case of a `macro_rules!` which
/// contains an attr/derive kind, it will always return `ItemType::Macro`. If you want all
/// kinds, you need to use [`Item::types`].
pub(crate) fn type_(&self) -> ItemType {
ItemType::from(self)
}

/// Returns an item types. There is only one case where it can return more than one kind:
/// for `macro_rules!` items which contain an attr/derive kind.
pub(crate) fn types(&self) -> impl Iterator<Item = ItemType> {
if let ItemKind::MacroItem(_, macro_kinds) = self.kind {
Either::Right(macro_kinds.iter().map(|kind| match kind {
MacroKinds::ATTR => ItemType::BangMacroAttribute,
MacroKinds::DERIVE => ItemType::BangMacroDerive,
MacroKinds::BANG => ItemType::Macro,
_ => panic!("unsupported macro kind {kind:?}"),
}))
} else {
Either::Left(std::iter::once(self.type_()))
}
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This could easily be made an iterator by having an array of the different macro item types, filtering over that for the macro case, then just using iter::once in the non-macro case. The two types of iterators can be unified into one type either by casting them to trait objects or by using itertools::Either.

This would have the additional benefit that order would not depend on bitflags iter order, and that there would be no need for a panic! here.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Hence why I added the FIXME comment. I guess you're fine with it so doing the change right away. :)


/// Returns true if this a macro declared with the `macro` keyword or with `macro_rules!.
pub(crate) fn is_bang_macro_or_macro_rules(&self) -> bool {
matches!(self.kind, ItemKind::MacroItem(..))
}

pub(crate) fn defaultness(&self) -> Option<Defaultness> {
match self.kind {
ItemKind::MethodItem(_, defaultness) | ItemKind::RequiredMethodItem(_, defaultness) => {
Expand All @@ -759,6 +781,11 @@ impl Item {
}
}

/// Generates the HTML file name based on the item kind.
pub(crate) fn html_filename(&self) -> String {
format!("{type_}.{name}.html", type_ = self.type_(), name = self.name.unwrap())
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

here we use the type_() method, but for the search index, we use types().pop().unwrap(). Since the search index data is used to generate hrefs for search results, it seems like this mismatch could cause issues in the future.

}

/// Returns a `FnHeader` if `self` is a function item, otherwise returns `None`.
pub(crate) fn fn_header(&self, tcx: TyCtxt<'_>) -> Option<hir::FnHeader> {
fn build_fn_header(
Expand Down Expand Up @@ -918,7 +945,13 @@ pub(crate) enum ItemKind {
ForeignStaticItem(Static, hir::Safety),
/// `type`s from an extern block
ForeignTypeItem,
MacroItem(Macro),
/// A macro defined with `macro_rules` or the `macro` keyword. It can be multiple things (macro,
/// derive and attribute, potentially multiple at once). Don't forget to look into the
///`MacroKinds` values.
///
/// If a `macro_rules!` only contains a `attr`/`derive` branch, then it's not stored in this
/// variant but in the `ProcMacroItem` variant.
MacroItem(Macro, MacroKinds),
ProcMacroItem(ProcMacro),
PrimitiveItem(PrimitiveType),
/// A required associated constant in a trait declaration.
Expand Down Expand Up @@ -973,7 +1006,7 @@ impl ItemKind {
| ForeignFunctionItem(_, _)
| ForeignStaticItem(_, _)
| ForeignTypeItem
| MacroItem(_)
| MacroItem(..)
| ProcMacroItem(_)
| PrimitiveItem(_)
| RequiredAssocConstItem(..)
Expand Down
64 changes: 52 additions & 12 deletions src/librustdoc/clean/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use std::{ascii, mem};

use rustc_ast as ast;
use rustc_ast::join_path_idents;
use rustc_ast::token::{Token, TokenKind};
use rustc_ast::tokenstream::TokenTree;
use rustc_data_structures::thin_vec::{ThinVec, thin_vec};
use rustc_hir as hir;
Expand Down Expand Up @@ -585,38 +586,77 @@ pub(crate) static RUSTDOC_VERSION: Lazy<&'static str> =

/// Render a sequence of macro arms in a format suitable for displaying to the user
/// as part of an item declaration.
fn render_macro_arms<'a>(
fn render_macro_arms(
tcx: TyCtxt<'_>,
matchers: impl Iterator<Item = &'a TokenTree>,
tokens: &rustc_ast::tokenstream::TokenStream,
arm_delim: &str,
) -> String {
let mut tokens = tokens.iter();
let mut out = String::new();
for matcher in matchers {
while let Some(mut token) = tokens.next() {
// If this an attr/derive rule, it looks like `attr() () => {}`, so the token needs to be
// handled at the same time as the actual matcher.
//
// Without that, we would end up with `attr()` on one line and the matcher `()` on another.
let pre = if matches!(token, TokenTree::Token(..)) {
let pre = format!("{}() ", render_macro_matcher(tcx, token));
// Skipping the always empty `()` following the attr/derive ident.
tokens.next();
let Some(next) = tokens.next() else {
return out;
};
token = next;
pre
} else {
String::new()
};
writeln!(
out,
" {matcher} => {{ ... }}{arm_delim}",
matcher = render_macro_matcher(tcx, matcher),
" {pre}{matcher} => {{ ... }}{arm_delim}",
matcher = render_macro_matcher(tcx, token),
)
.unwrap();
// We skip the `=>`, macro "body" and the delimiter closing that "body" since we don't
// render them.
let _token = tokens.next();
// The `=>`.
debug_assert_matches!(
_token,
Some(TokenTree::Token(Token { kind: TokenKind::FatArrow, .. }, _))
);
let _token = tokens.next();
// The arm body.
debug_assert_matches!(_token, Some(TokenTree::Delimited(..)));
// The delimiter (which may be omitted on the last arm's body).
let _token = tokens.next();
debug_assert_matches!(_token, None | Some(TokenTree::Token(Token { .. }, _)));
}
out
}

pub(super) fn display_macro_source(tcx: TyCtxt<'_>, name: Symbol, def: &ast::MacroDef) -> 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]);

if def.macro_rules {
format!("macro_rules! {name} {{\n{arms}}}", arms = render_macro_arms(tcx, matchers, ";"))
format!(
"macro_rules! {name} {{\n{arms}}}",
arms = render_macro_arms(tcx, &def.body.tokens, ";")
)
} else {
if matchers.len() <= 1 {
if def.body.tokens.len() <= 4 {
format!(
"macro {name}{matchers} {{\n ...\n}}",
matchers =
matchers.map(|matcher| render_macro_matcher(tcx, matcher)).collect::<String>(),
matchers = def
.body
.tokens
.get(0)
.map(|matcher| render_macro_matcher(tcx, matcher))
.unwrap_or_default(),
Comment on lines +645 to +653
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

v2 macros can use the multi-branch form with a single branch. does this code correctly handle that? do we have a test for it?

macro foo {
    () => { "bar" }
}

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Adding a test. :)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Hum actually both this PR and nightly renders it this like:

Image

So I guess the matchers themselves don't matter.

)
} else {
format!("macro {name} {{\n{arms}}}", arms = render_macro_arms(tcx, matchers, ","))
format!(
"macro {name} {{\n{arms}}}",
arms = render_macro_arms(tcx, &def.body.tokens, ",")
)
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/fold.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ pub(crate) trait DocFolder: Sized {
| ForeignFunctionItem(..)
| ForeignStaticItem(..)
| ForeignTypeItem
| MacroItem(_)
| MacroItem(..)
| ProcMacroItem(_)
| PrimitiveItem(_)
| RequiredAssocConstItem(..)
Expand Down
9 changes: 7 additions & 2 deletions src/librustdoc/formats/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -597,8 +597,9 @@ fn add_item_to_search_index(tcx: TyCtxt<'_>, cache: &mut Cache, item: &clean::It
let aliases = item.attrs.get_doc_aliases();
let is_deprecated = item.is_deprecated(tcx);
let is_unstable = item.is_unstable();
let mut types = item.types();
let index_item = IndexItem {
ty: item.type_(),
ty: types.next().unwrap(),
defid: Some(defid),
name,
module_path: parent_path.to_vec(),
Expand All @@ -614,7 +615,11 @@ fn add_item_to_search_index(tcx: TyCtxt<'_>, cache: &mut Cache, item: &clean::It
is_deprecated,
is_unstable,
};

for type_ in types {
let mut index_item_copy = index_item.clone();
index_item_copy.ty = type_;
cache.search_index.push(index_item_copy);
}
cache.search_index.push(index_item);
}

Expand Down
13 changes: 9 additions & 4 deletions src/librustdoc/formats/item_type.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,12 @@ item_type! {
// This number is reserved for use in JavaScript
// Generic = 26,
Attribute = 27,
// The two next ones represent an attr/derive macro declared as a `macro_rules!`. We need this
// distinction because they will point to a `macro.[name].html` file and not
// `[attr|derive].[name].html` file, so the link generation needs to take it into account while
// still having the filtering working as expected.
BangMacroAttribute = 28,
BangMacroDerive = 29,
Comment on lines +108 to +109
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

These feel like misnomers, a "bang macro" generally refers to a fn-like macro, called with !(), not just anything declared with macro_rules!. also could definitely benefit from doc comments.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It's about how they're declared, not how they're used. Big +1 for the missing docs, adding that.

Copy link
Copy Markdown
Contributor

@lolbinarycat lolbinarycat Apr 24, 2026

Choose a reason for hiding this comment

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

problem is that's not what "bang macro" conventionally means, and i would rather not introduce more instances of "this term is used completely differently within rustdoc".

i would prefer we just use "DeclMaro", as both of these can be grouped under "declarative macros"

}

impl<'a> From<&'a clean::Item> for ItemType {
Expand Down Expand Up @@ -163,10 +169,9 @@ impl ItemType {
DefKind::Trait => Self::Trait,
DefKind::TyAlias => Self::TypeAlias,
DefKind::TraitAlias => Self::TraitAlias,
DefKind::Macro(MacroKinds::BANG) => ItemType::Macro,
DefKind::Macro(MacroKinds::ATTR) => ItemType::ProcAttribute,
DefKind::Macro(MacroKinds::DERIVE) => ItemType::ProcDerive,
DefKind::Macro(_) => todo!("Handle macros with multiple kinds"),
DefKind::Macro(_) => ItemType::Macro,
DefKind::ForeignTy => Self::ForeignType,
DefKind::Variant => Self::Variant,
DefKind::Field => Self::StructField,
Expand Down Expand Up @@ -221,8 +226,8 @@ impl ItemType {
ItemType::AssocConst => "associatedconstant",
ItemType::ForeignType => "foreigntype",
ItemType::Keyword => "keyword",
ItemType::ProcAttribute => "attr",
ItemType::ProcDerive => "derive",
ItemType::ProcAttribute | ItemType::BangMacroAttribute => "attr",
ItemType::ProcDerive | ItemType::BangMacroDerive => "derive",
ItemType::TraitAlias => "traitalias",
ItemType::Attribute => "attribute",
}
Expand Down
4 changes: 2 additions & 2 deletions src/librustdoc/html/markdown.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2051,7 +2051,7 @@ fn is_default_id(id: &str) -> bool {
| "crate-search"
| "crate-search-div"
// This is the list of IDs used in HTML generated in Rust (including the ones
// used in tera template files).
// used in askama template files).
| "themeStyle"
| "settings-menu"
| "help-button"
Expand Down Expand Up @@ -2090,7 +2090,7 @@ fn is_default_id(id: &str) -> bool {
| "blanket-implementations-list"
| "deref-methods"
| "layout"
| "aliased-type"
| "aliased-type",
)
}

Expand Down
Loading
Loading