From 810254b31e0135d1f2dfe9e3717c3c0f69676a86 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sat, 18 Jun 2022 15:50:37 +0200 Subject: [PATCH] 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 4c54b569762..11ab3a3f931 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 0c60278a82d..34d590fb244 100644 --- a/src/librustdoc/html/render/span_map.rs +++ b/src/librustdoc/html/render/span_map.rs @@ -88,36 +88,48 @@ fn handle_path(&mut self, path: &rustc_hir::Path<'_>) { /// 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 @@ fn visit_expr(&mut self, expr: &'tcx rustc_hir::Expr<'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); -- 2.44.0