From 3f12fa7fda96de6687cdd281affcee4a61c35b80 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 26 Nov 2021 21:20:24 +0100 Subject: [PATCH 1/6] Add support for macro in "jump to def" feature --- src/librustdoc/html/format.rs | 1 + src/librustdoc/html/highlight.rs | 126 +++++++++++++++++++++---- src/librustdoc/html/render/span_map.rs | 71 ++++++++++---- 3 files changed, 162 insertions(+), 36 deletions(-) diff --git a/src/librustdoc/html/format.rs b/src/librustdoc/html/format.rs index 5baa53d55545f..29a58810036c6 100644 --- a/src/librustdoc/html/format.rs +++ b/src/librustdoc/html/format.rs @@ -519,6 +519,7 @@ impl clean::GenericArgs { } // Possible errors when computing href link source for a `DefId` +#[derive(PartialEq, Eq)] pub(crate) enum HrefError { /// This item is known to rustdoc, but from a crate that does not have documentation generated. /// diff --git a/src/librustdoc/html/highlight.rs b/src/librustdoc/html/highlight.rs index 480728b179790..209172bb98e67 100644 --- a/src/librustdoc/html/highlight.rs +++ b/src/librustdoc/html/highlight.rs @@ -5,15 +5,19 @@ //! //! Use the `render_with_highlighting` to highlight some rust code. -use crate::clean::PrimitiveType; +use crate::clean::{ExternalLocation, PrimitiveType}; use crate::html::escape::Escape; use crate::html::render::Context; use std::collections::VecDeque; use std::fmt::{Display, Write}; +use std::iter::once; +use rustc_ast as ast; use rustc_data_structures::fx::FxHashMap; +use rustc_hir::def_id::DefId; use rustc_lexer::{LiteralKind, TokenKind}; +use rustc_metadata::creader::{CStore, LoadedMacro}; use rustc_span::edition::Edition; use rustc_span::symbol::Symbol; use rustc_span::{BytePos, Span, DUMMY_SP}; @@ -99,6 +103,7 @@ fn write_code( ) { // This replace allows to fix how the code source with DOS backline characters is displayed. let src = src.replace("\r\n", "\n"); + let mut closing_tag = ""; Classifier::new( &src, edition, @@ -108,8 +113,8 @@ fn write_code( .highlight(&mut |highlight| { match highlight { Highlight::Token { text, class } => string(out, Escape(text), class, &context_info), - Highlight::EnterSpan { class } => enter_span(out, class), - Highlight::ExitSpan => exit_span(out), + Highlight::EnterSpan { class } => closing_tag = enter_span(out, class, &context_info), + Highlight::ExitSpan => exit_span(out, &closing_tag), }; }); } @@ -129,7 +134,7 @@ enum Class { RefKeyWord, Self_(Span), Op, - Macro, + Macro(Span), MacroNonTerminal, String, Number, @@ -153,7 +158,7 @@ impl Class { Class::RefKeyWord => "kw-2", Class::Self_(_) => "self", Class::Op => "op", - Class::Macro => "macro", + Class::Macro(_) => "macro", Class::MacroNonTerminal => "macro-nonterminal", Class::String => "string", Class::Number => "number", @@ -171,8 +176,22 @@ impl Class { /// a "span" (a tuple representing `(lo, hi)` equivalent of `Span`). fn get_span(self) -> Option { match self { - Self::Ident(sp) | Self::Self_(sp) => Some(sp), - _ => None, + Self::Ident(sp) | Self::Self_(sp) | Self::Macro(sp) => Some(sp), + Self::Comment + | Self::DocComment + | Self::Attribute + | Self::KeyWord + | Self::RefKeyWord + | Self::Op + | Self::MacroNonTerminal + | Self::String + | Self::Number + | Self::Bool + | Self::Lifetime + | Self::PreludeTy + | Self::PreludeVal + | Self::QuestionMark + | Self::Decoration(_) => None, } } } @@ -611,7 +630,7 @@ impl<'a> Classifier<'a> { }, TokenKind::Ident | TokenKind::RawIdent if lookahead == Some(TokenKind::Bang) => { self.in_macro = true; - sink(Highlight::EnterSpan { class: Class::Macro }); + sink(Highlight::EnterSpan { class: Class::Macro(self.new_span(before, text)) }); sink(Highlight::Token { text, class: None }); return; } @@ -658,13 +677,18 @@ impl<'a> Classifier<'a> { /// Called when we start processing a span of text that should be highlighted. /// The `Class` argument specifies how it should be highlighted. -fn enter_span(out: &mut Buffer, klass: Class) { - write!(out, "", klass.as_html()); +fn enter_span( + out: &mut Buffer, + klass: Class, + context_info: &Option>, +) -> &'static str { + string_without_closing_tag(out, "", Some(klass), context_info) + .expect("no closing tag to close wrapper...") } /// Called at the end of a span of highlighted text. -fn exit_span(out: &mut Buffer) { - out.write_str(""); +fn exit_span(out: &mut Buffer, closing_tag: &str) { + out.write_str(closing_tag); } /// Called for a span of text. If the text should be highlighted differently @@ -689,13 +713,28 @@ fn string( klass: Option, context_info: &Option>, ) { + if let Some(closing_tag) = string_without_closing_tag(out, text, klass, context_info) { + out.write_str(closing_tag); + } +} + +fn string_without_closing_tag( + out: &mut Buffer, + text: T, + klass: Option, + context_info: &Option>, +) -> Option<&'static str> { let Some(klass) = klass - else { return write!(out, "{}", text) }; + else { + write!(out, "{}", text); + return None; + }; let Some(def_span) = klass.get_span() else { - write!(out, "{}", klass.as_html(), text); - return; + write!(out, "{}", klass.as_html(), text); + return Some(""); }; + let mut text_s = text.to_string(); if text_s.contains("::") { text_s = text_s.split("::").intersperse("::").fold(String::new(), |mut path, t| { @@ -730,8 +769,17 @@ fn string( .map(|s| format!("{}{}", context_info.root_path, s)), LinkFromSrc::External(def_id) => { format::href_with_root_path(*def_id, context, Some(context_info.root_path)) - .ok() .map(|(url, _, _)| url) + .or_else(|e| { + if e == format::HrefError::NotInExternalCache + && matches!(klass, Class::Macro(_)) + { + Ok(generate_macro_def_id_path(context_info, *def_id)) + } else { + Err(e) + } + }) + .ok() } LinkFromSrc::Primitive(prim) => format::href_with_root_path( PrimitiveType::primitive_locations(context.tcx())[prim], @@ -743,11 +791,51 @@ fn string( } }) { - write!(out, "{}", klass.as_html(), href, text_s); - return; + write!(out, "{}", klass.as_html(), href, text_s); + return Some(""); } } - write!(out, "{}", klass.as_html(), text_s); + write!(out, "{}", klass.as_html(), text_s); + Some("") +} + +/// This function is to get the external macro path because they are not in the cache used n +/// `href_with_root_path`. +fn generate_macro_def_id_path(context_info: &ContextInfo<'_, '_, '_>, def_id: DefId) -> String { + let tcx = context_info.context.shared.tcx; + let crate_name = tcx.crate_name(def_id.krate).to_string(); + let cache = &context_info.context.cache(); + + let relative = tcx.def_path(def_id).data.into_iter().filter_map(|elem| { + // extern blocks have an empty name + let s = elem.data.to_string(); + if !s.is_empty() { Some(s) } else { None } + }); + // Check to see if it is a macro 2.0 or built-in macro + let mut path = if matches!( + CStore::from_tcx(tcx).load_macro_untracked(def_id, tcx.sess), + LoadedMacro::MacroDef(def, _) + if matches!(&def.kind, ast::ItemKind::MacroDef(ast_def) + if !ast_def.macro_rules) + ) { + once(crate_name.clone()).chain(relative).collect() + } else { + vec![crate_name.clone(), relative.last().expect("relative was empty")] + }; + + let url_parts = match cache.extern_locations[&def_id.krate] { + ExternalLocation::Remote(ref s) => vec![s.trim_end_matches('/')], + ExternalLocation::Local => vec![context_info.root_path.trim_end_matches('/'), &crate_name], + ExternalLocation::Unknown => panic!("unknown crate"), + }; + + let last = path.pop().unwrap(); + let last = format!("macro.{}.html", last); + if path.is_empty() { + format!("{}/{}", url_parts.join("/"), last) + } else { + format!("{}/{}/{}", url_parts.join("/"), path.join("/"), last) + } } #[cfg(test)] diff --git a/src/librustdoc/html/render/span_map.rs b/src/librustdoc/html/render/span_map.rs index 86961dc3bf149..0c60278a82dd9 100644 --- a/src/librustdoc/html/render/span_map.rs +++ b/src/librustdoc/html/render/span_map.rs @@ -8,7 +8,8 @@ use rustc_hir::intravisit::{self, Visitor}; use rustc_hir::{ExprKind, HirId, Mod, Node}; use rustc_middle::hir::nested_filter; use rustc_middle::ty::TyCtxt; -use rustc_span::Span; +use rustc_span::hygiene::MacroKind; +use rustc_span::{BytePos, ExpnKind, Span}; use std::path::{Path, PathBuf}; @@ -63,32 +64,59 @@ struct SpanMapVisitor<'tcx> { impl<'tcx> SpanMapVisitor<'tcx> { /// This function is where we handle `hir::Path` elements and add them into the "span map". - fn handle_path(&mut self, path: &rustc_hir::Path<'_>, path_span: Option) { + fn handle_path(&mut self, path: &rustc_hir::Path<'_>) { let info = match path.res { - // FIXME: For now, we only handle `DefKind` if it's not `DefKind::TyParam` or - // `DefKind::Macro`. Would be nice to support them too alongside the other `DefKind` + // FIXME: For now, we handle `DefKind` if it's not a `DefKind::TyParam`. + // Would be nice to support them too alongside the other `DefKind` // (such as primitive types!). - Res::Def(kind, def_id) if kind != DefKind::TyParam => { - if matches!(kind, DefKind::Macro(_)) { - return; - } - Some(def_id) - } + Res::Def(kind, def_id) if kind != DefKind::TyParam => Some(def_id), Res::Local(_) => None, Res::PrimTy(p) => { // FIXME: Doesn't handle "path-like" primitives like arrays or tuples. - let span = path_span.unwrap_or(path.span); - self.matches.insert(span, LinkFromSrc::Primitive(PrimitiveType::from(p))); + self.matches.insert(path.span, LinkFromSrc::Primitive(PrimitiveType::from(p))); return; } Res::Err => return, _ => return, }; if let Some(span) = self.tcx.hir().res_span(path.res) { - self.matches - .insert(path_span.unwrap_or(path.span), LinkFromSrc::Local(clean::Span::new(span))); + self.matches.insert(path.span, LinkFromSrc::Local(clean::Span::new(span))); } else if let Some(def_id) = info { - self.matches.insert(path_span.unwrap_or(path.span), LinkFromSrc::External(def_id)); + self.matches.insert(path.span, LinkFromSrc::External(def_id)); + } + } + + /// Adds the macro call into the span map. Returns `true` if the `span` was inside a macro + /// expansion, whether or not it was added to the span map. + fn handle_macro(&mut self, span: Span) -> bool { + if span.from_expansion() { + let mut data = span.ctxt().outer_expn_data(); + let mut call_site = data.call_site; + while call_site.from_expansion() { + data = call_site.ctxt().outer_expn_data(); + call_site = data.call_site; + } + + if let ExpnKind::Macro(MacroKind::Bang, macro_name) = data.kind { + let link_from_src = if let Some(macro_def_id) = data.macro_def_id { + if macro_def_id.is_local() { + LinkFromSrc::Local(clean::Span::new(data.def_site)) + } else { + LinkFromSrc::External(macro_def_id) + } + } else { + return true; + }; + let new_span = data.call_site; + let macro_name = macro_name.as_str(); + // The "call_site" includes the whole macro with its "arguments". We only want + // the macro name. + let new_span = new_span.with_hi(new_span.lo() + BytePos(macro_name.len() as u32)); + self.matches.insert(new_span, link_from_src); + } + true + } else { + false } } } @@ -101,7 +129,10 @@ impl<'tcx> Visitor<'tcx> for SpanMapVisitor<'tcx> { } fn visit_path(&mut self, path: &'tcx rustc_hir::Path<'tcx>, _id: HirId) { - self.handle_path(path, None); + if self.handle_macro(path.span) { + return; + } + self.handle_path(path); intravisit::walk_path(self, path); } @@ -143,12 +174,18 @@ impl<'tcx> Visitor<'tcx> for SpanMapVisitor<'tcx> { ); } } + } else if self.handle_macro(expr.span) { + // We don't want to deeper into the macro. + return; } intravisit::walk_expr(self, expr); } fn visit_use(&mut self, path: &'tcx rustc_hir::Path<'tcx>, id: HirId) { - self.handle_path(path, None); + if self.handle_macro(path.span) { + return; + } + self.handle_path(path); intravisit::walk_use(self, path, id); } } From dda980dec07fb7093a153180f19f967ce55fe1f9 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sat, 18 Jun 2022 15:32:26 +0200 Subject: [PATCH 2/6] Rename ContextInfo into HrefContext --- src/librustdoc/html/highlight.rs | 50 ++++++++++++++++---------------- src/librustdoc/html/sources.rs | 2 +- 2 files changed, 26 insertions(+), 26 deletions(-) diff --git a/src/librustdoc/html/highlight.rs b/src/librustdoc/html/highlight.rs index 209172bb98e67..4c54b569762c6 100644 --- a/src/librustdoc/html/highlight.rs +++ b/src/librustdoc/html/highlight.rs @@ -26,7 +26,7 @@ use super::format::{self, Buffer}; use super::render::LinkFromSrc; /// This type is needed in case we want to render links on items to allow to go to their definition. -pub(crate) struct ContextInfo<'a, 'b, 'c> { +pub(crate) struct HrefContext<'a, 'b, 'c> { pub(crate) context: &'a Context<'b>, /// This span contains the current file we're going through. pub(crate) file_span: Span, @@ -48,7 +48,7 @@ pub(crate) fn render_with_highlighting( tooltip: Option<(Option, &str)>, edition: Edition, extra_content: Option, - context_info: Option>, + href_context: Option>, decoration_info: Option, ) { debug!("highlighting: ================\n{}\n==============", src); @@ -66,7 +66,7 @@ pub(crate) fn render_with_highlighting( } write_header(out, class, extra_content); - write_code(out, src, edition, context_info, decoration_info); + write_code(out, src, edition, href_context, decoration_info); write_footer(out, playground_button); } @@ -89,8 +89,8 @@ fn write_header(out: &mut Buffer, class: Option<&str>, extra_content: Option>, + href_context: Option>, decoration_info: Option, ) { // This replace allows to fix how the code source with DOS backline characters is displayed. @@ -107,13 +107,13 @@ fn write_code( Classifier::new( &src, edition, - context_info.as_ref().map(|c| c.file_span).unwrap_or(DUMMY_SP), + href_context.as_ref().map(|c| c.file_span).unwrap_or(DUMMY_SP), decoration_info, ) .highlight(&mut |highlight| { match highlight { - Highlight::Token { text, class } => string(out, Escape(text), class, &context_info), - Highlight::EnterSpan { class } => closing_tag = enter_span(out, class, &context_info), + Highlight::Token { text, class } => string(out, Escape(text), class, &href_context), + Highlight::EnterSpan { class } => closing_tag = enter_span(out, class, &href_context), Highlight::ExitSpan => exit_span(out, &closing_tag), }; }); @@ -680,9 +680,9 @@ impl<'a> Classifier<'a> { fn enter_span( out: &mut Buffer, klass: Class, - context_info: &Option>, + href_context: &Option>, ) -> &'static str { - string_without_closing_tag(out, "", Some(klass), context_info) + string_without_closing_tag(out, "", Some(klass), href_context) .expect("no closing tag to close wrapper...") } @@ -711,9 +711,9 @@ fn string( out: &mut Buffer, text: T, klass: Option, - context_info: &Option>, + href_context: &Option>, ) { - if let Some(closing_tag) = string_without_closing_tag(out, text, klass, context_info) { + if let Some(closing_tag) = string_without_closing_tag(out, text, klass, href_context) { out.write_str(closing_tag); } } @@ -722,7 +722,7 @@ fn string_without_closing_tag( out: &mut Buffer, text: T, klass: Option, - context_info: &Option>, + href_context: &Option>, ) -> Option<&'static str> { let Some(klass) = klass else { @@ -754,10 +754,10 @@ fn string_without_closing_tag( path }); } - if let Some(context_info) = context_info { + if let Some(href_context) = href_context { if let Some(href) = - context_info.context.shared.span_correspondance_map.get(&def_span).and_then(|href| { - let context = context_info.context; + href_context.context.shared.span_correspondance_map.get(&def_span).and_then(|href| { + let context = href_context.context; // FIXME: later on, it'd be nice to provide two links (if possible) for all items: // one to the documentation page and one to the source definition. // FIXME: currently, external items only generate a link to their documentation, @@ -766,15 +766,15 @@ fn string_without_closing_tag( match href { LinkFromSrc::Local(span) => context .href_from_span(*span, true) - .map(|s| format!("{}{}", context_info.root_path, s)), + .map(|s| format!("{}{}", href_context.root_path, s)), LinkFromSrc::External(def_id) => { - format::href_with_root_path(*def_id, context, Some(context_info.root_path)) + format::href_with_root_path(*def_id, context, Some(href_context.root_path)) .map(|(url, _, _)| url) .or_else(|e| { if e == format::HrefError::NotInExternalCache && matches!(klass, Class::Macro(_)) { - Ok(generate_macro_def_id_path(context_info, *def_id)) + Ok(generate_macro_def_id_path(href_context, *def_id)) } else { Err(e) } @@ -784,7 +784,7 @@ fn string_without_closing_tag( LinkFromSrc::Primitive(prim) => format::href_with_root_path( PrimitiveType::primitive_locations(context.tcx())[prim], context, - Some(context_info.root_path), + Some(href_context.root_path), ) .ok() .map(|(url, _, _)| url), @@ -801,10 +801,10 @@ fn string_without_closing_tag( /// This function is to get the external macro path because they are not in the cache used n /// `href_with_root_path`. -fn generate_macro_def_id_path(context_info: &ContextInfo<'_, '_, '_>, def_id: DefId) -> String { - let tcx = context_info.context.shared.tcx; +fn generate_macro_def_id_path(href_context: &HrefContext<'_, '_, '_>, def_id: DefId) -> String { + let tcx = href_context.context.shared.tcx; let crate_name = tcx.crate_name(def_id.krate).to_string(); - let cache = &context_info.context.cache(); + let cache = &href_context.context.cache(); let relative = tcx.def_path(def_id).data.into_iter().filter_map(|elem| { // extern blocks have an empty name @@ -825,7 +825,7 @@ fn generate_macro_def_id_path(context_info: &ContextInfo<'_, '_, '_>, def_id: De let url_parts = match cache.extern_locations[&def_id.krate] { ExternalLocation::Remote(ref s) => vec![s.trim_end_matches('/')], - ExternalLocation::Local => vec![context_info.root_path.trim_end_matches('/'), &crate_name], + ExternalLocation::Local => vec![href_context.root_path.trim_end_matches('/'), &crate_name], ExternalLocation::Unknown => panic!("unknown crate"), }; diff --git a/src/librustdoc/html/sources.rs b/src/librustdoc/html/sources.rs index 524c90e1f4d64..bc0a56f2b052f 100644 --- a/src/librustdoc/html/sources.rs +++ b/src/librustdoc/html/sources.rs @@ -299,7 +299,7 @@ pub(crate) fn print_src( None, edition, Some(line_numbers), - Some(highlight::ContextInfo { context, file_span, root_path }), + Some(highlight::HrefContext { context, file_span, root_path }), decoration_info, ); } From 810254b31e0135d1f2dfe9e3717c3c0f69676a86 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sat, 18 Jun 2022 15:50:37 +0200 Subject: [PATCH 3/6] Improve code readability and documentation --- src/librustdoc/html/highlight.rs | 80 +++++++++++++++++--------- src/librustdoc/html/render/span_map.rs | 68 +++++++++++++--------- 2 files changed, 94 insertions(+), 54 deletions(-) diff --git a/src/librustdoc/html/highlight.rs b/src/librustdoc/html/highlight.rs index 4c54b569762c6..11ab3a3f931c3 100644 --- a/src/librustdoc/html/highlight.rs +++ b/src/librustdoc/html/highlight.rs @@ -103,7 +103,7 @@ fn write_code( ) { // This replace allows to fix how the code source with DOS backline characters is displayed. let src = src.replace("\r\n", "\n"); - let mut closing_tag = ""; + let mut closing_tags: Vec<&'static str> = Vec::new(); Classifier::new( &src, edition, @@ -113,8 +113,12 @@ fn write_code( .highlight(&mut |highlight| { match highlight { Highlight::Token { text, class } => string(out, Escape(text), class, &href_context), - Highlight::EnterSpan { class } => closing_tag = enter_span(out, class, &href_context), - Highlight::ExitSpan => exit_span(out, &closing_tag), + Highlight::EnterSpan { class } => { + closing_tags.push(enter_span(out, class, &href_context)) + } + Highlight::ExitSpan => { + exit_span(out, closing_tags.pop().expect("ExitSpan without EnterSpan")) + } }; }); } @@ -682,8 +686,10 @@ fn enter_span( klass: Class, href_context: &Option>, ) -> &'static str { - string_without_closing_tag(out, "", Some(klass), href_context) - .expect("no closing tag to close wrapper...") + string_without_closing_tag(out, "", Some(klass), href_context).expect( + "internal error: enter_span was called with Some(klass) but did not return a \ + closing HTML tag", + ) } /// Called at the end of a span of highlighted text. @@ -718,6 +724,15 @@ fn string( } } +/// This function writes `text` into `out` with some modifications depending on `klass`: +/// +/// * If `klass` is `None`, `text` is written into `out` with no modification. +/// * If `klass` is `Some` but `klass.get_span()` is `None`, it writes the text wrapped in a +/// `` with the provided `klass`. +/// * If `klass` is `Some` and has a [`rustc_span::Span`], it then tries to generate a link (`` +/// element) by retrieving the link information from the `span_correspondance_map` that was filled +/// in `span_map.rs::collect_spans_and_sources`. If it cannot retrieve the information, then it's +/// the same as the second point (`klass` is `Some` but doesn't have a [`rustc_span::Span`]). fn string_without_closing_tag( out: &mut Buffer, text: T, @@ -799,42 +814,55 @@ fn string_without_closing_tag( Some("") } -/// This function is to get the external macro path because they are not in the cache used n +/// This function is to get the external macro path because they are not in the cache used in /// `href_with_root_path`. fn generate_macro_def_id_path(href_context: &HrefContext<'_, '_, '_>, def_id: DefId) -> String { let tcx = href_context.context.shared.tcx; let crate_name = tcx.crate_name(def_id.krate).to_string(); - let cache = &href_context.context.cache(); + let cache = href_context.context.cache(); let relative = tcx.def_path(def_id).data.into_iter().filter_map(|elem| { // extern blocks have an empty name let s = elem.data.to_string(); if !s.is_empty() { Some(s) } else { None } }); - // Check to see if it is a macro 2.0 or built-in macro - let mut path = if matches!( - CStore::from_tcx(tcx).load_macro_untracked(def_id, tcx.sess), - LoadedMacro::MacroDef(def, _) - if matches!(&def.kind, ast::ItemKind::MacroDef(ast_def) - if !ast_def.macro_rules) - ) { + // Check to see if it is a macro 2.0 or built-in macro. + // More information in . + let is_macro_2 = match CStore::from_tcx(tcx).load_macro_untracked(def_id, tcx.sess) { + LoadedMacro::MacroDef(def, _) => { + // If `ast_def.macro_rules` is `true`, then it's not a macro 2.0. + matches!(&def.kind, ast::ItemKind::MacroDef(ast_def) if !ast_def.macro_rules) + } + _ => false, + }; + + let mut path = if is_macro_2 { once(crate_name.clone()).chain(relative).collect() } else { - vec![crate_name.clone(), relative.last().expect("relative was empty")] + vec![crate_name.clone(), relative.last().unwrap()] }; + if path.len() < 2 { + // The minimum we can have is the crate name followed by the macro name. If shorter, then + // it means that that `relative` was empty, which is an error. + panic!("macro path cannot be empty!"); + } - let url_parts = match cache.extern_locations[&def_id.krate] { - ExternalLocation::Remote(ref s) => vec![s.trim_end_matches('/')], - ExternalLocation::Local => vec![href_context.root_path.trim_end_matches('/'), &crate_name], - ExternalLocation::Unknown => panic!("unknown crate"), - }; + if let Some(last) = path.last_mut() { + *last = format!("macro.{}.html", last); + } - let last = path.pop().unwrap(); - let last = format!("macro.{}.html", last); - if path.is_empty() { - format!("{}/{}", url_parts.join("/"), last) - } else { - format!("{}/{}/{}", url_parts.join("/"), path.join("/"), last) + match cache.extern_locations[&def_id.krate] { + ExternalLocation::Remote(ref s) => { + // `ExternalLocation::Remote` always end with a `/`. + format!("{}{}", s, path.join("/")) + } + ExternalLocation::Local => { + // `href_context.root_path` always end with a `/`. + format!("{}{}/{}", href_context.root_path, crate_name, path.join("/")) + } + ExternalLocation::Unknown => { + panic!("crate {} not in cache when linkifying macros", crate_name) + } } } diff --git a/src/librustdoc/html/render/span_map.rs b/src/librustdoc/html/render/span_map.rs index 0c60278a82dd9..34d590fb2448c 100644 --- a/src/librustdoc/html/render/span_map.rs +++ b/src/librustdoc/html/render/span_map.rs @@ -88,36 +88,48 @@ impl<'tcx> SpanMapVisitor<'tcx> { /// Adds the macro call into the span map. Returns `true` if the `span` was inside a macro /// expansion, whether or not it was added to the span map. + /// + /// The idea for the macro support is to check if the current `Span` comes from expansion. If + /// so, we loop until we find the macro definition by using `outer_expn_data` in a loop. + /// Finally, we get the information about the macro itself (`span` if "local", `DefId` + /// otherwise) and store it inside the span map. fn handle_macro(&mut self, span: Span) -> bool { - if span.from_expansion() { - let mut data = span.ctxt().outer_expn_data(); - let mut call_site = data.call_site; - while call_site.from_expansion() { - data = call_site.ctxt().outer_expn_data(); - call_site = data.call_site; - } + if !span.from_expansion() { + return false; + } + // So if the `span` comes from a macro expansion, we need to get the original + // macro's `DefId`. + let mut data = span.ctxt().outer_expn_data(); + let mut call_site = data.call_site; + // Macros can expand to code containing macros, which will in turn be expanded, etc. + // So the idea here is to "go up" until we're back to code that was generated from + // macro expansion so that we can get the `DefId` of the original macro that was at the + // origin of this expansion. + while call_site.from_expansion() { + data = call_site.ctxt().outer_expn_data(); + call_site = data.call_site; + } - if let ExpnKind::Macro(MacroKind::Bang, macro_name) = data.kind { - let link_from_src = if let Some(macro_def_id) = data.macro_def_id { - if macro_def_id.is_local() { - LinkFromSrc::Local(clean::Span::new(data.def_site)) - } else { - LinkFromSrc::External(macro_def_id) - } - } else { - return true; - }; - let new_span = data.call_site; - let macro_name = macro_name.as_str(); - // The "call_site" includes the whole macro with its "arguments". We only want - // the macro name. - let new_span = new_span.with_hi(new_span.lo() + BytePos(macro_name.len() as u32)); - self.matches.insert(new_span, link_from_src); + let macro_name = match data.kind { + ExpnKind::Macro(MacroKind::Bang, macro_name) => macro_name, + // Even though we don't handle this kind of macro, this `data` still comes from + // expansion so we return `true` so we don't go any deeper in this code. + _ => return true, + }; + let link_from_src = match data.macro_def_id { + Some(macro_def_id) if macro_def_id.is_local() => { + LinkFromSrc::Local(clean::Span::new(data.def_site)) } - true - } else { - false - } + Some(macro_def_id) => LinkFromSrc::External(macro_def_id), + None => return true, + }; + let new_span = data.call_site; + let macro_name = macro_name.as_str(); + // The "call_site" includes the whole macro with its "arguments". We only want + // the macro name. + let new_span = new_span.with_hi(new_span.lo() + BytePos(macro_name.len() as u32)); + self.matches.insert(new_span, link_from_src); + true } } @@ -175,7 +187,7 @@ impl<'tcx> Visitor<'tcx> for SpanMapVisitor<'tcx> { } } } else if self.handle_macro(expr.span) { - // We don't want to deeper into the macro. + // We don't want to go deeper into the macro. return; } intravisit::walk_expr(self, expr); From f4db07ed4c1ab8a0f7961efc60ec32193e971f5c Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 26 Nov 2021 21:20:45 +0100 Subject: [PATCH 4/6] Add test for macro support in "jump to def" feature --- .../check-source-code-urls-to-def-std.rs | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/src/test/rustdoc/check-source-code-urls-to-def-std.rs b/src/test/rustdoc/check-source-code-urls-to-def-std.rs index b129ceb5b7302..3396b234a77b1 100644 --- a/src/test/rustdoc/check-source-code-urls-to-def-std.rs +++ b/src/test/rustdoc/check-source-code-urls-to-def-std.rs @@ -15,3 +15,28 @@ pub fn foo(a: u32, b: &str, c: String) { let y: bool = true; babar(); } + +macro_rules! yolo { () => {}} + +fn bar(a: i32) {} + +macro_rules! bar { + ($a:ident) => { bar($a) } +} + +macro_rules! data { + ($x:expr) => { $x * 2 } +} + +pub fn another_foo() { + // This is known limitation: if the macro doesn't generate anything, the visitor + // can't find any item or anything that could tell us that it comes from expansion. + // @!has - '//a[@href="../../src/foo/check-source-code-urls-to-def-std.rs.html#19"]' 'yolo!' + yolo!(); + // @has - '//a[@href="{{channel}}/std/macro.eprintln.html"]' 'eprintln!' + eprintln!(); + // @has - '//a[@href="../../src/foo/check-source-code-urls-to-def-std.rs.html#27-29"]' 'data!' + let x = data!(4); + // @has - '//a[@href="../../src/foo/check-source-code-urls-to-def-std.rs.html#23-25"]' 'bar!' + bar!(x); +} From 987c73158e2120ef75b4b7fc46dcd88a621106d8 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Mon, 20 Jun 2022 22:32:49 +0200 Subject: [PATCH 5/6] Integrate `generate_macro_def_id_path` into `href_with_root_path` --- src/librustdoc/html/format.rs | 71 +++++++++++++++++++++++++++++++- src/librustdoc/html/highlight.rs | 69 +------------------------------ 2 files changed, 72 insertions(+), 68 deletions(-) diff --git a/src/librustdoc/html/format.rs b/src/librustdoc/html/format.rs index 29a58810036c6..67b01245ef7ad 100644 --- a/src/librustdoc/html/format.rs +++ b/src/librustdoc/html/format.rs @@ -8,14 +8,16 @@ use std::borrow::Cow; use std::cell::Cell; use std::fmt; -use std::iter; +use std::iter::{self, once}; +use rustc_ast as ast; use rustc_attr::{ConstStability, StabilityLevel}; use rustc_data_structures::captures::Captures; use rustc_data_structures::fx::FxHashSet; use rustc_hir as hir; use rustc_hir::def::DefKind; use rustc_hir::def_id::DefId; +use rustc_metadata::creader::{CStore, LoadedMacro}; use rustc_middle::ty; use rustc_middle::ty::DefIdTree; use rustc_middle::ty::TyCtxt; @@ -557,6 +559,71 @@ pub(crate) fn join_with_double_colon(syms: &[Symbol]) -> String { s } +/// This function is to get the external macro path because they are not in the cache used in +/// `href_with_root_path`. +fn generate_macro_def_id_path( + def_id: DefId, + cx: &Context<'_>, + root_path: Option<&str>, +) -> (String, ItemType, Vec) { + let tcx = cx.shared.tcx; + let crate_name = tcx.crate_name(def_id.krate).to_string(); + let cache = cx.cache(); + + let fqp: Vec = tcx + .def_path(def_id) + .data + .into_iter() + .filter_map(|elem| { + // extern blocks (and a few others things) have an empty name. + match elem.data.get_opt_name() { + Some(s) if !s.is_empty() => Some(s), + _ => None, + } + }) + .collect(); + let relative = fqp.iter().map(|elem| elem.to_string()); + // Check to see if it is a macro 2.0 or built-in macro. + // More information in . + let is_macro_2 = match CStore::from_tcx(tcx).load_macro_untracked(def_id, tcx.sess) { + LoadedMacro::MacroDef(def, _) => { + // If `ast_def.macro_rules` is `true`, then it's not a macro 2.0. + matches!(&def.kind, ast::ItemKind::MacroDef(ast_def) if !ast_def.macro_rules) + } + _ => false, + }; + + let mut path = if is_macro_2 { + once(crate_name.clone()).chain(relative).collect() + } else { + vec![crate_name.clone(), relative.last().unwrap()] + }; + if path.len() < 2 { + // The minimum we can have is the crate name followed by the macro name. If shorter, then + // it means that that `relative` was empty, which is an error. + panic!("macro path cannot be empty!"); + } + + if let Some(last) = path.last_mut() { + *last = format!("macro.{}.html", last); + } + + let url = match cache.extern_locations[&def_id.krate] { + ExternalLocation::Remote(ref s) => { + // `ExternalLocation::Remote` always end with a `/`. + format!("{}{}", s, path.join("/")) + } + ExternalLocation::Local => { + // `root_path` always end with a `/`. + format!("{}{}/{}", root_path.unwrap_or(""), crate_name, path.join("/")) + } + ExternalLocation::Unknown => { + panic!("crate {} not in cache when linkifying macros", crate_name) + } + }; + (url, ItemType::Macro, fqp) +} + pub(crate) fn href_with_root_path( did: DefId, cx: &Context<'_>, @@ -612,6 +679,8 @@ pub(crate) fn href_with_root_path( ExternalLocation::Unknown => return Err(HrefError::DocumentationNotBuilt), }, ) + } else if matches!(def_kind, DefKind::Macro(_)) { + return Ok(generate_macro_def_id_path(did, cx, root_path)); } else { return Err(HrefError::NotInExternalCache); } diff --git a/src/librustdoc/html/highlight.rs b/src/librustdoc/html/highlight.rs index 11ab3a3f931c3..d2ef89078bf6d 100644 --- a/src/librustdoc/html/highlight.rs +++ b/src/librustdoc/html/highlight.rs @@ -5,19 +5,15 @@ //! //! Use the `render_with_highlighting` to highlight some rust code. -use crate::clean::{ExternalLocation, PrimitiveType}; +use crate::clean::PrimitiveType; use crate::html::escape::Escape; use crate::html::render::Context; use std::collections::VecDeque; use std::fmt::{Display, Write}; -use std::iter::once; -use rustc_ast as ast; use rustc_data_structures::fx::FxHashMap; -use rustc_hir::def_id::DefId; use rustc_lexer::{LiteralKind, TokenKind}; -use rustc_metadata::creader::{CStore, LoadedMacro}; use rustc_span::edition::Edition; use rustc_span::symbol::Symbol; use rustc_span::{BytePos, Span, DUMMY_SP}; @@ -784,17 +780,8 @@ fn string_without_closing_tag( .map(|s| format!("{}{}", href_context.root_path, s)), LinkFromSrc::External(def_id) => { format::href_with_root_path(*def_id, context, Some(href_context.root_path)) - .map(|(url, _, _)| url) - .or_else(|e| { - if e == format::HrefError::NotInExternalCache - && matches!(klass, Class::Macro(_)) - { - Ok(generate_macro_def_id_path(href_context, *def_id)) - } else { - Err(e) - } - }) .ok() + .map(|(url, _, _)| url) } LinkFromSrc::Primitive(prim) => format::href_with_root_path( PrimitiveType::primitive_locations(context.tcx())[prim], @@ -814,57 +801,5 @@ fn string_without_closing_tag( Some("") } -/// This function is to get the external macro path because they are not in the cache used in -/// `href_with_root_path`. -fn generate_macro_def_id_path(href_context: &HrefContext<'_, '_, '_>, def_id: DefId) -> String { - let tcx = href_context.context.shared.tcx; - let crate_name = tcx.crate_name(def_id.krate).to_string(); - let cache = href_context.context.cache(); - - let relative = tcx.def_path(def_id).data.into_iter().filter_map(|elem| { - // extern blocks have an empty name - let s = elem.data.to_string(); - if !s.is_empty() { Some(s) } else { None } - }); - // Check to see if it is a macro 2.0 or built-in macro. - // More information in . - let is_macro_2 = match CStore::from_tcx(tcx).load_macro_untracked(def_id, tcx.sess) { - LoadedMacro::MacroDef(def, _) => { - // If `ast_def.macro_rules` is `true`, then it's not a macro 2.0. - matches!(&def.kind, ast::ItemKind::MacroDef(ast_def) if !ast_def.macro_rules) - } - _ => false, - }; - - let mut path = if is_macro_2 { - once(crate_name.clone()).chain(relative).collect() - } else { - vec![crate_name.clone(), relative.last().unwrap()] - }; - if path.len() < 2 { - // The minimum we can have is the crate name followed by the macro name. If shorter, then - // it means that that `relative` was empty, which is an error. - panic!("macro path cannot be empty!"); - } - - if let Some(last) = path.last_mut() { - *last = format!("macro.{}.html", last); - } - - match cache.extern_locations[&def_id.krate] { - ExternalLocation::Remote(ref s) => { - // `ExternalLocation::Remote` always end with a `/`. - format!("{}{}", s, path.join("/")) - } - ExternalLocation::Local => { - // `href_context.root_path` always end with a `/`. - format!("{}{}/{}", href_context.root_path, crate_name, path.join("/")) - } - ExternalLocation::Unknown => { - panic!("crate {} not in cache when linkifying macros", crate_name) - } - } -} - #[cfg(test)] mod tests; From beb2f364cc85b4408da1d043f875d159003558e4 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Mon, 20 Jun 2022 23:31:40 +0200 Subject: [PATCH 6/6] Fix panic by checking if `CStore` has the crate data we want before actually querying it --- compiler/rustc_metadata/src/creader.rs | 4 ++++ src/librustdoc/html/format.rs | 20 ++++++++++++++------ 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_metadata/src/creader.rs b/compiler/rustc_metadata/src/creader.rs index 947d563ae3cd1..555db5846edd0 100644 --- a/compiler/rustc_metadata/src/creader.rs +++ b/compiler/rustc_metadata/src/creader.rs @@ -133,6 +133,10 @@ impl CStore { CrateNum::new(self.metas.len() - 1) } + pub fn has_crate_data(&self, cnum: CrateNum) -> bool { + self.metas[cnum].is_some() + } + pub(crate) fn get_crate_data(&self, cnum: CrateNum) -> CrateMetadataRef<'_> { let cdata = self.metas[cnum] .as_ref() diff --git a/src/librustdoc/html/format.rs b/src/librustdoc/html/format.rs index 67b01245ef7ad..056eda089c1de 100644 --- a/src/librustdoc/html/format.rs +++ b/src/librustdoc/html/format.rs @@ -565,7 +565,7 @@ fn generate_macro_def_id_path( def_id: DefId, cx: &Context<'_>, root_path: Option<&str>, -) -> (String, ItemType, Vec) { +) -> Result<(String, ItemType, Vec), HrefError> { let tcx = cx.shared.tcx; let crate_name = tcx.crate_name(def_id.krate).to_string(); let cache = cx.cache(); @@ -583,9 +583,15 @@ fn generate_macro_def_id_path( }) .collect(); let relative = fqp.iter().map(|elem| elem.to_string()); + let cstore = CStore::from_tcx(tcx); + // We need this to prevent a `panic` when this function is used from intra doc links... + if !cstore.has_crate_data(def_id.krate) { + debug!("No data for crate {}", crate_name); + return Err(HrefError::NotInExternalCache); + } // Check to see if it is a macro 2.0 or built-in macro. // More information in . - let is_macro_2 = match CStore::from_tcx(tcx).load_macro_untracked(def_id, tcx.sess) { + let is_macro_2 = match cstore.load_macro_untracked(def_id, tcx.sess) { LoadedMacro::MacroDef(def, _) => { // If `ast_def.macro_rules` is `true`, then it's not a macro 2.0. matches!(&def.kind, ast::ItemKind::MacroDef(ast_def) if !ast_def.macro_rules) @@ -601,7 +607,8 @@ fn generate_macro_def_id_path( if path.len() < 2 { // The minimum we can have is the crate name followed by the macro name. If shorter, then // it means that that `relative` was empty, which is an error. - panic!("macro path cannot be empty!"); + debug!("macro path cannot be empty!"); + return Err(HrefError::NotInExternalCache); } if let Some(last) = path.last_mut() { @@ -618,10 +625,11 @@ fn generate_macro_def_id_path( format!("{}{}/{}", root_path.unwrap_or(""), crate_name, path.join("/")) } ExternalLocation::Unknown => { - panic!("crate {} not in cache when linkifying macros", crate_name) + debug!("crate {} not in cache when linkifying macros", crate_name); + return Err(HrefError::NotInExternalCache); } }; - (url, ItemType::Macro, fqp) + Ok((url, ItemType::Macro, fqp)) } pub(crate) fn href_with_root_path( @@ -680,7 +688,7 @@ pub(crate) fn href_with_root_path( }, ) } else if matches!(def_kind, DefKind::Macro(_)) { - return Ok(generate_macro_def_id_path(did, cx, root_path)); + return generate_macro_def_id_path(did, cx, root_path); } else { return Err(HrefError::NotInExternalCache); }