-
-
Notifications
You must be signed in to change notification settings - Fork 14.8k
Rustdoc bang attr macro #152449
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
base: main
Are you sure you want to change the base?
Rustdoc bang attr macro #152449
Changes from all commits
c2c330d
aeda6c3
751f62c
655dfe2
e7d2b4b
74a8b54
fda393f
bdf7fd2
702d85b
cbe154c
76a6846
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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}; | ||
|
|
@@ -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_())) | ||
| } | ||
| } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 This would have the additional benefit that order would not depend on bitflags iter order, and that there would be no need for a
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) => { | ||
|
|
@@ -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()) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. here we use the |
||
| } | ||
|
|
||
| /// 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( | ||
|
|
@@ -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. | ||
|
|
@@ -973,7 +1006,7 @@ impl ItemKind { | |
| | ForeignFunctionItem(_, _) | ||
| | ForeignStaticItem(_, _) | ||
| | ForeignTypeItem | ||
| | MacroItem(_) | ||
| | MacroItem(..) | ||
| | ProcMacroItem(_) | ||
| | PrimitiveItem(_) | ||
| | RequiredAssocConstItem(..) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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; | ||
|
|
@@ -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
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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" }
}
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Adding a test. :)
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| ) | ||
| } 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, ",") | ||
| ) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 { | ||
|
|
@@ -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, | ||
|
|
@@ -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", | ||
| } | ||
|
|
||

There was a problem hiding this comment.
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_macrohandle being given a non-proc macro?There was a problem hiding this comment.
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.There was a problem hiding this comment.
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.