Incorporate jyn's feedback

* Move call location logic from function constructor to rendering
* Fix issue with macro spans in scraping examples
* Clean up example loading logic

Documentation / newtype for DecorationInfo

Fix line number display

Serialize edition of call site, other small cleanup

Author: willcrichton

src/librustdoc/clean/inline.rs

@@ -235,7 +235,7 @@ fn build_external_function(cx: &mut DocContext<'_>, did: DefId) -> clean::Functi
         decl,
         generics,
         header: hir::FnHeader { unsafety: sig.unsafety(), abi: sig.abi(), constness, asyncness },
-        call_locations: None,
+        def_id: did,
     }
 }
 

src/librustdoc/clean/mod.rs

@@ -801,10 +801,8 @@ impl<'a> Clean<Function> for (&'a hir::FnSig<'a>, &'a hir::Generics<'a>, hir::Bo
     fn clean(&self, cx: &mut DocContext<'_>) -> Function {
         let (generics, decl) =
             enter_impl_trait(cx, |cx| (self.1.clean(cx), (&*self.0.decl, self.2).clean(cx)));
-        let mut function = Function { decl, generics, header: self.0.header, call_locations: None };
         let def_id = self.2.hir_id.owner.to_def_id();
-        function.load_call_locations(def_id, cx);
-        function
+        Function { decl, generics, header: self.0.header, def_id }
     }
 }
 
@@ -936,14 +934,13 @@ impl Clean<Item> for hir::TraitItem<'_> {
                     let (generics, decl) = enter_impl_trait(cx, |cx| {
                         (self.generics.clean(cx), (&*sig.decl, &names[..]).clean(cx))
                     });
-                    let mut t =
-                        Function { header: sig.header, decl, generics, call_locations: None };
+                    let def_id = self.def_id.to_def_id();
+                    let mut t = Function { header: sig.header, decl, generics, def_id };
                     if t.header.constness == hir::Constness::Const
                         && is_unstable_const_fn(cx.tcx, local_did).is_some()
                     {
                         t.header.constness = hir::Constness::NotConst;
                     }
-                    t.load_call_locations(self.def_id.to_def_id(), cx);
                     TyMethodItem(t)
                 }
                 hir::TraitItemKind::Type(ref bounds, ref default) => {
@@ -1062,7 +1059,7 @@ impl Clean<Item> for ty::AssocItem {
                         ty::ImplContainer(_) => Some(self.defaultness),
                         ty::TraitContainer(_) => None,
                     };
-                    let mut function = Function {
+                    let function = Function {
                         generics,
                         decl,
                         header: hir::FnHeader {
@@ -1071,12 +1068,11 @@ impl Clean<Item> for ty::AssocItem {
                             constness,
                             asyncness,
                         },
-                        call_locations: None,
+                        def_id: self.def_id,
                     };
-                    function.load_call_locations(self.def_id, cx);
                     MethodItem(function, defaultness)
                 } else {
-                    let mut function = Function {
+                    let function = Function {
                         generics,
                         decl,
                         header: hir::FnHeader {
@@ -1085,9 +1081,8 @@ impl Clean<Item> for ty::AssocItem {
                             constness: hir::Constness::NotConst,
                             asyncness: hir::IsAsync::NotAsync,
                         },
-                        call_locations: None,
+                        def_id: self.def_id,
                     };
-                    function.load_call_locations(self.def_id, cx);
                     TyMethodItem(function)
                 }
             }
@@ -2086,7 +2081,8 @@ fn clean_use_statement(
 impl Clean<Item> for (&hir::ForeignItem<'_>, Option<Symbol>) {
     fn clean(&self, cx: &mut DocContext<'_>) -> Item {
         let (item, renamed) = self;
-        cx.with_param_env(item.def_id.to_def_id(), |cx| {
+        let def_id = item.def_id.to_def_id();
+        cx.with_param_env(def_id, |cx| {
             let kind = match item.kind {
                 hir::ForeignItemKind::Fn(ref decl, ref names, ref generics) => {
                     let abi = cx.tcx.hir().get_foreign_abi(item.hir_id());
@@ -2106,7 +2102,7 @@ impl Clean<Item> for (&hir::ForeignItem<'_>, Option<Symbol>) {
                             constness: hir::Constness::NotConst,
                             asyncness: hir::IsAsync::NotAsync,
                         },
-                        call_locations: None,
+                        def_id,
                     })
                 }
                 hir::ForeignItemKind::Static(ref ty, mutability) => {

src/librustdoc/clean/types.rs

@@ -42,7 +42,6 @@ use crate::formats::cache::Cache;
 use crate::formats::item_type::ItemType;
 use crate::html::render::cache::ExternalLocation;
 use crate::html::render::Context;
-use crate::scrape_examples::{self, FnCallLocations};
 
 use self::FnRetTy::*;
 use self::ItemKind::*;
@@ -1255,19 +1254,7 @@ crate struct Function {
     crate decl: FnDecl,
     crate generics: Generics,
     crate header: hir::FnHeader,
-    crate call_locations: Option<FnCallLocations>,
-}
-
-impl Function {
-    /// If --scrape-examples is used, then this function attempts to find call locations
-    /// for `self` within `RenderOptions::call_locations` and store them in `Function::call_locations`.
-    crate fn load_call_locations(&mut self, def_id: hir::def_id::DefId, cx: &DocContext<'_>) {
-        if let Some(call_locations) = cx.render_options.call_locations.as_ref() {
-            let key = scrape_examples::def_id_call_key(cx.tcx, def_id);
-            self.call_locations = call_locations.get(&key).cloned();
-            debug!("call_locations: {:?} -- {:?}", key, self.call_locations);
-        }
-    }
+    crate def_id: DefId,
 }
 
 #[derive(Clone, PartialEq, Eq, Debug, Hash)]

src/librustdoc/config.rs

@@ -207,6 +207,7 @@ impl fmt::Debug for Options {
             .field("run_check", &self.run_check)
             .field("no_run", &self.no_run)
             .field("nocapture", &self.nocapture)
+            .field("scrape_examples", &self.scrape_examples)
             .finish()
     }
 }
@@ -285,7 +286,7 @@ crate struct RenderOptions {
     crate emit: Vec<EmitType>,
     /// If `true`, HTML source pages will generate links for items to their definition.
     crate generate_link_to_definition: bool,
-    crate call_locations: Option<AllCallLocations>,
+    crate call_locations: AllCallLocations,
 }
 
 #[derive(Copy, Clone, Debug, PartialEq, Eq)]

src/librustdoc/html/highlight.rs

@@ -31,7 +31,9 @@ crate struct ContextInfo<'a, 'b, 'c> {
     crate root_path: &'c str,
 }
 
-crate type DecorationInfo = FxHashMap<&'static str, Vec<(u32, u32)>>;
+/// Decorations are represented as a map from CSS class to vector of character ranges.
+/// Each range will be wrapped in a span with that class.
+crate struct DecorationInfo(crate FxHashMap<&'static str, Vec<(u32, u32)>>);
 
 /// Highlights `src`, returning the HTML output.
 crate fn render_with_highlighting(
@@ -273,6 +275,7 @@ struct Decorations {
 impl Decorations {
     fn new(info: DecorationInfo) -> Self {
         let (starts, ends) = info
+            .0
             .into_iter()
             .map(|(kind, ranges)| ranges.into_iter().map(move |(lo, hi)| ((lo, kind), hi)))
             .flatten()
@@ -305,6 +308,7 @@ impl<'a> Classifier<'a> {
         decoration_info: Option<DecorationInfo>,
     ) -> Classifier<'_> {
         let tokens = PeekIter::new(TokenIter { src });
+        let decorations = decoration_info.map(Decorations::new);
         Classifier {
             tokens,
             in_attribute: false,

src/librustdoc/html/render/context.rs

@@ -35,6 +35,7 @@ use crate::html::format::Buffer;
 use crate::html::markdown::{self, plain_text_summary, ErrorCodes, IdMap};
 use crate::html::static_files::PAGE;
 use crate::html::{layout, sources};
+use crate::scrape_examples::AllCallLocations;
 
 /// Major driving force in all rustdoc rendering. This contains information
 /// about where in the tree-like hierarchy rendering is occurring and controls
@@ -124,6 +125,8 @@ crate struct SharedContext<'tcx> {
     crate span_correspondance_map: FxHashMap<rustc_span::Span, LinkFromSrc>,
     /// The [`Cache`] used during rendering.
     crate cache: Cache,
+
+    crate call_locations: AllCallLocations,
 }
 
 impl SharedContext<'_> {
@@ -389,6 +392,7 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> {
             generate_redirect_map,
             show_type_layout,
             generate_link_to_definition,
+            call_locations,
             ..
         } = options;
 
@@ -480,6 +484,7 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> {
             templates,
             span_correspondance_map: matches,
             cache,
+            call_locations,
         };
 
         // Add the default themes to the `Vec` of stylepaths

src/librustdoc/html/render/mod.rs

@@ -73,8 +73,9 @@ use crate::html::format::{
     print_generic_bounds, print_where_clause, Buffer, HrefError, PrintWithSpace,
 };
 use crate::html::markdown::{HeadingOffset, Markdown, MarkdownHtml, MarkdownSummaryLine};
+use crate::html::highlight;
 use crate::html::sources;
-use crate::scrape_examples::{CallData, FnCallLocations};
+use crate::scrape_examples::CallData;
 
 /// A pair of name and its optional document.
 crate type NameDoc = (String, Option<String>);
@@ -592,9 +593,13 @@ fn document_full_inner(
         }
     }
 
-    match &*item.kind {
+    let kind = match &*item.kind {
+        clean::ItemKind::StrippedItem(box kind) | kind => kind,
+    };
+
+    match kind {
         clean::ItemKind::FunctionItem(f) | clean::ItemKind::MethodItem(f, _) => {
-            render_call_locations(w, cx, &f.call_locations, item);
+            render_call_locations(w, cx, f.def_id, item);
         }
         _ => {}
     }
@@ -2458,14 +2463,11 @@ fn collect_paths_for_type(first_ty: clean::Type, cache: &Cache) -> Vec<String> {
 const MAX_FULL_EXAMPLES: usize = 5;
 
 /// Generates the HTML for example call locations generated via the --scrape-examples flag.
-fn render_call_locations(
-    w: &mut Buffer,
-    cx: &Context<'_>,
-    call_locations: &Option<FnCallLocations>,
-    item: &clean::Item,
-) {
-    let call_locations = match call_locations.as_ref() {
-        Some(call_locations) if call_locations.len() > 0 => call_locations,
+fn render_call_locations(w: &mut Buffer, cx: &Context<'_>, def_id: DefId, item: &clean::Item) {
+    let tcx = cx.tcx();
+    let key = crate::scrape_examples::def_id_call_key(tcx, def_id);
+    let call_locations = match cx.shared.call_locations.get(&key) {
+        Some(call_locations) => call_locations,
         _ => {
             return;
         }
@@ -2483,7 +2485,6 @@ fn render_call_locations(
     );
 
     // Generate the HTML for a single example, being the title and code block
-    let tcx = cx.tcx();
     let write_example = |w: &mut Buffer, (path, call_data): (&PathBuf, &CallData)| -> bool {
         let contents = match fs::read_to_string(&path) {
             Ok(contents) => contents,
@@ -2497,40 +2498,51 @@ fn render_call_locations(
 
         // To reduce file sizes, we only want to embed the source code needed to understand the example, not
         // the entire file. So we find the smallest byte range that covers all items enclosing examples.
-        assert!(call_data.locations.len() > 0);
+        assert!(!call_data.locations.is_empty());
         let min_loc =
             call_data.locations.iter().min_by_key(|loc| loc.enclosing_item.byte_span.0).unwrap();
-        let min_byte = min_loc.enclosing_item.byte_span.0;
-        let min_line = min_loc.enclosing_item.line_span.0;
-        let max_byte =
+        let (byte_offset, _) = min_loc.enclosing_item.byte_span;
+        let (line_offset, _) = min_loc.enclosing_item.line_span;
+        let byte_ceiling =
             call_data.locations.iter().map(|loc| loc.enclosing_item.byte_span.1).max().unwrap();
 
         // The output code is limited to that byte range.
-        let contents_subset = &contents[(min_byte as usize)..(max_byte as usize)];
+        let contents_subset = &contents[(byte_offset as usize)..(byte_ceiling as usize)];
 
         // The call locations need to be updated to reflect that the size of the program has changed.
-        // Specifically, the ranges are all subtracted by `min_byte` since that's the new zero point.
+        // Specifically, the ranges are all subtracted by `byte_offset` since that's the new zero point.
         let (byte_ranges, line_ranges): (Vec<_>, Vec<_>) = call_data
             .locations
             .iter()
             .map(|loc| {
                 let (byte_lo, byte_hi) = loc.call_expr.byte_span;
                 let (line_lo, line_hi) = loc.call_expr.line_span;
-                ((byte_lo - min_byte, byte_hi - min_byte), (line_lo - min_line, line_hi - min_line))
+                (
+                    (byte_lo - byte_offset, byte_hi - byte_offset),
+                    (line_lo - line_offset, line_hi - line_offset),
+                )
             })
             .unzip();
 
-        let edition = cx.shared.edition();
+        let (init_min, init_max) = line_ranges[0];
+        let line_range = if init_min == init_max {
+            format!("line {}", init_min + line_offset + 1)
+        } else {
+            format!("lines {}-{}", init_min + line_offset + 1, init_max + line_offset + 1)
+        };
+
         write!(
             w,
-            r#"<div class="scraped-example" data-locs="{locations}">
+            r#"<div class="scraped-example" data-locs="{locations}" data-offset="{offset}">
                 <div class="scraped-example-title">
-                   {name} <a href="{root}{url}" target="_blank">[src]</a>
+                   {name} (<a href="{root}{url}" target="_blank">{line_range}</a>)
                 </div>
                 <div class="code-wrapper">"#,
             root = cx.root_path(),
             url = call_data.url,
             name = call_data.display_name,
+            line_range = line_range,
+            offset = line_offset,
             // The locations are encoded as a data attribute, so they can be read
             // later by the JS for interactions.
             locations = serde_json::to_string(&line_ranges).unwrap(),
@@ -2551,8 +2563,8 @@ fn render_call_locations(
                 _ => false,
             })?;
             Some(rustc_span::Span::with_root_ctxt(
-                file.start_pos + BytePos(min_byte),
-                file.start_pos + BytePos(max_byte),
+                file.start_pos + BytePos(byte_offset),
+                file.start_pos + BytePos(byte_ceiling),
             ))
         })()
         .unwrap_or(rustc_span::DUMMY_SP);
@@ -2566,12 +2578,12 @@ fn render_call_locations(
         sources::print_src(
             w,
             contents_subset,
-            edition,
+            call_data.edition,
             file_span,
             cx,
             &root_path,
-            Some(min_line),
-            Some(decoration_info),
+            Some(line_offset),
+            Some(highlight::DecorationInfo(decoration_info)),
         );
         write!(w, "</div></div>");
 
@@ -2590,12 +2602,14 @@ fn render_call_locations(
         };
 
         let mut locs = call_locations.into_iter().collect::<Vec<_>>();
-        locs.sort_by_key(|x| sort_criterion(x));
+        locs.sort_by_key(sort_criterion);
         locs
     };
 
-    // Write just one example that's visible by default in the method's description.
     let mut it = ordered_locations.into_iter().peekable();
+
+    // An example may fail to write if its source can't be read for some reason, so this method
+    // continues iterating until a write suceeds
     let write_and_skip_failure = |w: &mut Buffer, it: &mut Peekable<_>| {
         while let Some(example) = it.next() {
             if write_example(&mut *w, example) {
@@ -2604,6 +2618,7 @@ fn render_call_locations(
         }
     };
 
+    // Write just one example that's visible by default in the method's description.
     write_and_skip_failure(w, &mut it);
 
     // Then add the remaining examples in a hidden section.
@@ -2626,7 +2641,7 @@ fn render_call_locations(
             write_and_skip_failure(w, &mut it);
         }
 
-        // For the remaining examples, generate a <ul /> containing links to the source files.
+        // For the remaining examples, generate a <ul> containing links to the source files.
         if it.peek().is_some() {
             write!(
                 w,
@@ -2635,7 +2650,7 @@ fn render_call_locations(
             it.for_each(|(_, call_data)| {
                 write!(
                     w,
-                    r#"<li><a href="{}{}" target="_blank">{}</a></li>"#,
+                    r#"<li><a href="{root}{url}" target="_blank">{name}</a></li>"#,
                     root = cx.root_path(),
                     url = call_data.url,
                     name = call_data.display_name

src/librustdoc/html/static/css/rustdoc.css

@@ -1978,10 +1978,6 @@ details.undocumented[open] > summary::before {
 	font-family: 'Fira Sans';
 }
 
-.scraped-example-title a {
-	margin-left: 5px;
-}
-
 .scraped-example:not(.expanded) .code-wrapper pre.line-numbers,
 .scraped-example:not(.expanded) .code-wrapper .example-wrap pre.rust {
 	overflow: hidden;