Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit a646603

Browse files
authoredNov 14, 2024
Unrolled build for rust-lang#132302
Rollup merge of rust-lang#132302 - fmease:rustdoc-better-vis-for-macro-decl, r=notriddle rustdoc: Treat declarative macros more like other item kinds Apparently at some time in the past we were unable to generate an href for the module path inside the visibility of decl macros 2.0 (`pub(in ...)`). As a result of this, a whole separate function was introduced specifically for printing the visibility of decl macros that didn't attempt to generate any links. The description of PR rust-lang#84074 states: > This fixes the overly-complex invariant mentioned in rust-lang#83237 (comment), where the macro source can't have any links in it only because the cache hasn't been populated yet. I can no longer reproduce the original issue. Reusing the existing visibility rendering logic *seems* to work just fine (I couldn't come up with any counterexamples, though I invite you to prove me wrong). * Fixes rust-lang#83000 * Fixes the visibility showing up "twice" in rustdoc-JSON output: Once as the `visibility` field, once baked into the source[^1] * Fixes `#[doc(hidden)]` not getting rendered on doc(hidden) decl macros 2.0 under `--document-hiden-items` (for decl macros 1.2 the issue remains; I will address this separately when fixing rust-lang#132304). --- <details><summary>Outdated Section</summary> NOTE: The current version of this PR is committing a UI crime, I'd like to receive feedback on that. Maybe you have a satisfactory solution for how to remedy it. Namely, as you know we have two different ways of / modes for highlighting code with color: 1. Only highlighting links / item paths and avoiding to highlight tokens by kind like keywords (to reduce visual noise and maybe also artifact size). Used for item declarations(\*). 2. Highlighting tokens by kind. Used for code blocks written by the user. (\*): With the notable exception being macro declarations! Well, since this PR reuses the same function for rendering the item visibility (which only makes sense), we have a clash of modes: We now use both ways of highlighting code for decl macros: №1 for the visibility, №2 for the rest. This awkward. See for yourself: * On master: ![Screenshot 2024-10-29 at 03-37-48 by_example_vis_named in decl_macro a b c - Rust](https://github.com/user-attachments/assets/22f0ab6e-9ba9-4c4e-8fb0-0741c91d360b) * On this branch: ![Screenshot 2024-10-29 at 03-36-41 by_example_vis_named in decl_macro a b c - Rust](https://github.com/user-attachments/assets/b11d81a3-3e2e-43cb-a5b8-6773a3048732) </details> Furthermore, we now no longer syntax-highlight declarative macros (be it `macro_rules!` or `macro`) since that was inconsistent with the way we render all other item kinds. See (collapsed) *Outdated Section* above. See also rust-lang#132302 (comment). | On master | On this branch | |---|---| | ![Screenshot 2024-11-13 at 16-12-46 by_example_vis_named in decl_macro a b c - Rust](https://github.com/user-attachments/assets/cb3aeb42-a56d-4ced-80d9-f2694f369af1) | ![Screenshot 2024-11-13 at 16-13-22 by_example_vis_named in decl_macro a b c - Rust](https://github.com/user-attachments/assets/b73bee50-1b85-4862-afba-5ad471443ccc) | [^1]: E.g., `"visibility":{"restricted":{"parent":1,"path":"::a"}},/*OMITTED*/,"inner":{"macro":"pub(in a) macro by_example_vis_named($foo:expr) {\n ...\n}"}`
2 parents 8adb4b3 + 9016711 commit a646603

File tree

9 files changed

+31
-102
lines changed

9 files changed

+31
-102
lines changed
 

‎src/librustdoc/clean/inline.rs

+5-13
Original file line numberDiff line numberDiff line change
@@ -134,11 +134,7 @@ pub(crate) fn try_inline(
134134
})
135135
}
136136
Res::Def(DefKind::Macro(kind), did) => {
137-
let is_doc_hidden = cx.tcx.is_doc_hidden(did)
138-
|| attrs_without_docs
139-
.map(|(attrs, _)| attrs)
140-
.is_some_and(|attrs| utils::attrs_have_doc_flag(attrs.iter(), sym::hidden));
141-
let mac = build_macro(cx, did, name, import_def_id, kind, is_doc_hidden);
137+
let mac = build_macro(cx, did, name, kind);
142138

143139
let type_kind = match kind {
144140
MacroKind::Bang => ItemType::Macro,
@@ -740,18 +736,14 @@ fn build_macro(
740736
cx: &mut DocContext<'_>,
741737
def_id: DefId,
742738
name: Symbol,
743-
import_def_id: Option<LocalDefId>,
744739
macro_kind: MacroKind,
745-
is_doc_hidden: bool,
746740
) -> clean::ItemKind {
747741
match CStore::from_tcx(cx.tcx).load_macro_untracked(def_id, cx.tcx) {
748742
LoadedMacro::MacroDef { def, .. } => match macro_kind {
749-
MacroKind::Bang => {
750-
let vis = cx.tcx.visibility(import_def_id.map(|d| d.to_def_id()).unwrap_or(def_id));
751-
clean::MacroItem(clean::Macro {
752-
source: utils::display_macro_source(cx, name, &def, def_id, vis, is_doc_hidden),
753-
})
754-
}
743+
MacroKind::Bang => clean::MacroItem(clean::Macro {
744+
source: utils::display_macro_source(cx, name, &def),
745+
macro_rules: def.macro_rules,
746+
}),
755747
MacroKind::Derive | MacroKind::Attr => {
756748
clean::ProcMacroItem(clean::ProcMacro { kind: macro_kind, helpers: Vec::new() })
757749
}

‎src/librustdoc/clean/mod.rs

+4-7
Original file line numberDiff line numberDiff line change
@@ -2787,13 +2787,10 @@ fn clean_maybe_renamed_item<'tcx>(
27872787
fields: variant_data.fields().iter().map(|x| clean_field(x, cx)).collect(),
27882788
}),
27892789
ItemKind::Impl(impl_) => return clean_impl(impl_, item.owner_id.def_id, cx),
2790-
ItemKind::Macro(macro_def, MacroKind::Bang) => {
2791-
let ty_vis = cx.tcx.visibility(def_id);
2792-
MacroItem(Macro {
2793-
// FIXME this shouldn't be false
2794-
source: display_macro_source(cx, name, macro_def, def_id, ty_vis, false),
2795-
})
2796-
}
2790+
ItemKind::Macro(macro_def, MacroKind::Bang) => MacroItem(Macro {
2791+
source: display_macro_source(cx, name, macro_def),
2792+
macro_rules: macro_def.macro_rules,
2793+
}),
27972794
ItemKind::Macro(_, macro_kind) => clean_proc_macro(item, &mut name, macro_kind, cx),
27982795
// proc macros can have a name set by attributes
27992796
ItemKind::Fn(ref sig, generics, body_id) => {

‎src/librustdoc/clean/types.rs

+2
Original file line numberDiff line numberDiff line change
@@ -2543,6 +2543,8 @@ pub(crate) struct ImportSource {
25432543
#[derive(Clone, Debug)]
25442544
pub(crate) struct Macro {
25452545
pub(crate) source: String,
2546+
/// Whether the macro was defined via `macro_rules!` as opposed to `macro`.
2547+
pub(crate) macro_rules: bool,
25462548
}
25472549

25482550
#[derive(Clone, Debug)]

‎src/librustdoc/clean/utils.rs

+3-12
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ use crate::clean::{
2424
clean_middle_ty, inline,
2525
};
2626
use crate::core::DocContext;
27-
use crate::html::format::visibility_to_src_with_space;
2827

2928
#[cfg(test)]
3029
mod tests;
@@ -593,7 +592,7 @@ pub(crate) static DOC_CHANNEL: Lazy<&'static str> =
593592

594593
/// Render a sequence of macro arms in a format suitable for displaying to the user
595594
/// as part of an item declaration.
596-
pub(super) fn render_macro_arms<'a>(
595+
fn render_macro_arms<'a>(
597596
tcx: TyCtxt<'_>,
598597
matchers: impl Iterator<Item = &'a TokenTree>,
599598
arm_delim: &str,
@@ -614,9 +613,6 @@ pub(super) fn display_macro_source(
614613
cx: &mut DocContext<'_>,
615614
name: Symbol,
616615
def: &ast::MacroDef,
617-
def_id: DefId,
618-
vis: ty::Visibility<DefId>,
619-
is_doc_hidden: bool,
620616
) -> String {
621617
// Extract the spans of all matchers. They represent the "interface" of the macro.
622618
let matchers = def.body.tokens.chunks(4).map(|arm| &arm[0]);
@@ -626,18 +622,13 @@ pub(super) fn display_macro_source(
626622
} else {
627623
if matchers.len() <= 1 {
628624
format!(
629-
"{vis}macro {name}{matchers} {{\n ...\n}}",
630-
vis = visibility_to_src_with_space(Some(vis), cx.tcx, def_id, is_doc_hidden),
625+
"macro {name}{matchers} {{\n ...\n}}",
631626
matchers = matchers
632627
.map(|matcher| render_macro_matcher(cx.tcx, matcher))
633628
.collect::<String>(),
634629
)
635630
} else {
636-
format!(
637-
"{vis}macro {name} {{\n{arms}}}",
638-
vis = visibility_to_src_with_space(Some(vis), cx.tcx, def_id, is_doc_hidden),
639-
arms = render_macro_arms(cx.tcx, matchers, ","),
640-
)
631+
format!("macro {name} {{\n{arms}}}", arms = render_macro_arms(cx.tcx, matchers, ","))
641632
}
642633
}
643634
}

‎src/librustdoc/html/format.rs

-41
Original file line numberDiff line numberDiff line change
@@ -1615,47 +1615,6 @@ pub(crate) fn visibility_print_with_space<'a, 'tcx: 'a>(
16151615
})
16161616
}
16171617

1618-
/// This function is the same as print_with_space, except that it renders no links.
1619-
/// It's used for macros' rendered source view, which is syntax highlighted and cannot have
1620-
/// any HTML in it.
1621-
pub(crate) fn visibility_to_src_with_space<'a, 'tcx: 'a>(
1622-
visibility: Option<ty::Visibility<DefId>>,
1623-
tcx: TyCtxt<'tcx>,
1624-
item_did: DefId,
1625-
is_doc_hidden: bool,
1626-
) -> impl Display + 'a + Captures<'tcx> {
1627-
let vis: Cow<'static, str> = match visibility {
1628-
None => "".into(),
1629-
Some(ty::Visibility::Public) => "pub ".into(),
1630-
Some(ty::Visibility::Restricted(vis_did)) => {
1631-
// FIXME(camelid): This may not work correctly if `item_did` is a module.
1632-
// However, rustdoc currently never displays a module's
1633-
// visibility, so it shouldn't matter.
1634-
let parent_module = find_nearest_parent_module(tcx, item_did);
1635-
1636-
if vis_did.is_crate_root() {
1637-
"pub(crate) ".into()
1638-
} else if parent_module == Some(vis_did) {
1639-
// `pub(in foo)` where `foo` is the parent module
1640-
// is the same as no visibility modifier
1641-
"".into()
1642-
} else if parent_module.and_then(|parent| find_nearest_parent_module(tcx, parent))
1643-
== Some(vis_did)
1644-
{
1645-
"pub(super) ".into()
1646-
} else {
1647-
format!("pub(in {}) ", tcx.def_path_str(vis_did)).into()
1648-
}
1649-
}
1650-
};
1651-
display_fn(move |f| {
1652-
if is_doc_hidden {
1653-
f.write_str("#[doc(hidden)] ")?;
1654-
}
1655-
f.write_str(&vis)
1656-
})
1657-
}
1658-
16591618
pub(crate) trait PrintWithSpace {
16601619
fn print_with_space(&self) -> &str;
16611620
}

‎src/librustdoc/html/highlight.rs

-7
Original file line numberDiff line numberDiff line change
@@ -58,13 +58,6 @@ pub(crate) fn render_example_with_highlighting(
5858
write_footer(out, playground_button);
5959
}
6060

61-
/// Highlights `src` as an item-decl, returning the HTML output.
62-
pub(crate) fn render_item_decl_with_highlighting(src: &str, out: &mut Buffer) {
63-
write!(out, "<pre class=\"rust item-decl\">");
64-
write_code(out, src, None, None);
65-
write!(out, "</pre>");
66-
}
67-
6861
fn write_header(
6962
out: &mut Buffer,
7063
class: &str,

‎src/librustdoc/html/render/print_item.rs

+7-2
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@ use crate::html::format::{
3535
Buffer, Ending, PrintWithSpace, display_fn, join_with_double_colon, print_abi_with_space,
3636
print_constness_with_space, print_where_clause, visibility_print_with_space,
3737
};
38-
use crate::html::highlight;
3938
use crate::html::markdown::{HeadingOffset, MarkdownSummaryLine};
4039
use crate::html::render::{document_full, document_item_info};
4140
use crate::html::url_parts_builder::UrlPartsBuilder;
@@ -1745,7 +1744,13 @@ fn item_variants(
17451744
}
17461745

17471746
fn item_macro(w: &mut Buffer, cx: &mut Context<'_>, it: &clean::Item, t: &clean::Macro) {
1748-
highlight::render_item_decl_with_highlighting(&t.source, w);
1747+
wrap_item(w, |w| {
1748+
// FIXME: Also print `#[doc(hidden)]` for `macro_rules!` if it `is_doc_hidden`.
1749+
if !t.macro_rules {
1750+
write!(w, "{}", visibility_print_with_space(it, cx));
1751+
}
1752+
write!(w, "{}", Escape(&t.source));
1753+
});
17491754
write!(w, "{}", document(cx, it, None, HeadingOffset::H2))
17501755
}
17511756

‎tests/rustdoc/decl_macro.rs

+2
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,8 @@ mod a {
4848
}
4949
mod c {
5050
//@ has decl_macro/a/b/c/macro.by_example_vis_named.html //pre 'pub(in a) macro by_example_vis_named($foo:expr) {'
51+
// Regression test for <https://github.com/rust-lang/rust/issues/83000>:
52+
//@ has - '//pre[@class="rust item-decl"]//a[@class="mod"]/@href' '../../index.html'
5153
pub(in a) macro by_example_vis_named {
5254
($foo:expr) => {}
5355
}

‎tests/rustdoc/macro_rules-matchers.rs

+8-20
Original file line numberDiff line numberDiff line change
@@ -3,29 +3,17 @@
33

44
#![crate_name = "foo"]
55

6-
//@ has 'foo/macro.todo.html'
7-
//@ has - '//span[@class="macro"]' 'macro_rules!'
8-
//@ hasraw - ' todo {'
9-
10-
//@ hasraw - '{ () =&gt; { ... }; ($('
11-
//@ has - '//span[@class="macro-nonterminal"]' '$'
12-
//@ has - '//span[@class="macro-nonterminal"]' 'arg'
13-
//@ hasraw - ':tt)+'
14-
//@ hasraw - ') =&gt; { ... }; }'
6+
//@ has 'foo/macro.todo.html' '//pre' 'macro_rules! todo { \
7+
// () => { ... }; \
8+
// ($($arg:tt)+) => { ... }; \
9+
// }'
1510
pub use std::todo;
1611

1712
mod mod1 {
18-
//@ has 'foo/macro.macro1.html'
19-
//@ hasraw - 'macro_rules!'
20-
//@ hasraw - 'macro1'
21-
//@ hasraw - '{ () =&gt; { ... }; ($('
22-
//@ has - '//span[@class="macro-nonterminal"]' '$'
23-
//@ has - '//span[@class="macro-nonterminal"]' 'arg'
24-
//@ hasraw - ':'
25-
//@ hasraw - 'expr'
26-
//@ hasraw - '),'
27-
//@ hasraw - '+'
28-
//@ hasraw - ') =&gt; { ... }; }'
13+
//@ has 'foo/macro.macro1.html' '//pre' 'macro_rules! macro1 { \
14+
// () => { ... }; \
15+
// ($($arg:expr),+) => { ... }; \
16+
// }'
2917
#[macro_export]
3018
macro_rules! macro1 {
3119
() => {};

0 commit comments

Comments
 (0)
Failed to load comments.