End of rework of Attributes struct

src/librustdoc/clean/types.rs

@@ -122,7 +122,7 @@ impl Item {
 
     /// Finds the `doc` attribute as a NameValue and returns the corresponding
     /// value found.
-    crate fn doc_value(&self) -> Option<&str> {
+    crate fn doc_value(&self) -> Option<String> {
         self.attrs.doc_value()
     }
 
@@ -469,11 +469,13 @@ crate struct DocFragment {
     /// This allows distinguishing between the original documentation and a pub re-export.
     /// If it is `None`, the item was not re-exported.
     crate parent_module: Option<DefId>,
-    crate doc: String,
+    crate doc: Symbol,
     crate kind: DocFragmentKind,
+    crate need_backline: bool,
+    crate indent: usize,
 }
 
-#[derive(Clone, PartialEq, Eq, Debug, Hash)]
+#[derive(Clone, Copy, PartialEq, Eq, Debug, Hash)]
 crate enum DocFragmentKind {
     /// A doc fragment created from a `///` or `//!` doc comment.
     SugaredDoc,
@@ -484,16 +486,42 @@ crate enum DocFragmentKind {
     Include { filename: Symbol },
 }
 
+fn add_doc_fragment(out: &mut String, frag: &DocFragment) {
+    let s = frag.doc.as_str();
+    let mut iter = s.lines().peekable();
+    while let Some(line) = iter.next() {
+        if line.chars().any(|c| !c.is_whitespace()) {
+            assert!(line.len() >= frag.indent);
+            out.push_str(&line[frag.indent..]);
+        } else {
+            out.push_str(line);
+        }
+        if iter.peek().is_some() {
+            out.push('\n');
+        }
+    }
+    if frag.need_backline {
+        out.push('\n');
+    }
+}
+
 impl<'a> FromIterator<&'a DocFragment> for String {
     fn from_iter<T>(iter: T) -> Self
     where
         T: IntoIterator<Item = &'a DocFragment>,
     {
+        let mut prev_kind: Option<DocFragmentKind> = None;
         iter.into_iter().fold(String::new(), |mut acc, frag| {
-            if !acc.is_empty() {
+            if !acc.is_empty()
+                && prev_kind
+                    .take()
+                    .map(|p| matches!(p, DocFragmentKind::Include { .. }) && p != frag.kind)
+                    .unwrap_or(false)
+            {
                 acc.push('\n');
             }
-            acc.push_str(&frag.doc);
+            add_doc_fragment(&mut acc, &frag);
+            prev_kind = Some(frag.kind);
             acc
         })
     }
@@ -565,7 +593,7 @@ impl Attributes {
     /// Reads a `MetaItem` from within an attribute, looks for whether it is a
     /// `#[doc(include="file")]`, and returns the filename and contents of the file as loaded from
     /// its expansion.
-    crate fn extract_include(mi: &ast::MetaItem) -> Option<(Symbol, String)> {
+    crate fn extract_include(mi: &ast::MetaItem) -> Option<(Symbol, Symbol)> {
         mi.meta_item_list().and_then(|list| {
             for meta in list {
                 if meta.has_name(sym::include) {
@@ -574,7 +602,7 @@ impl Attributes {
                     // look for that instead
                     return meta.meta_item_list().and_then(|list| {
                         let mut filename: Option<Symbol> = None;
-                        let mut contents: Option<String> = None;
+                        let mut contents: Option<Symbol> = None;
 
                         for it in list {
                             if it.has_name(sym::file) {
@@ -583,7 +611,7 @@ impl Attributes {
                                 }
                             } else if it.has_name(sym::contents) {
                                 if let Some(docs) = it.value_str() {
-                                    contents = Some(docs.to_string());
+                                    contents = Some(docs);
                                 }
                             }
                         }
@@ -622,15 +650,30 @@ impl Attributes {
         attrs: &[ast::Attribute],
         additional_attrs: Option<(&[ast::Attribute], DefId)>,
     ) -> Attributes {
-        let mut doc_strings = vec![];
+        let mut doc_strings: Vec<DocFragment> = vec![];
         let mut sp = None;
         let mut cfg = Cfg::True;
         let mut doc_line = 0;
 
+        fn update_need_backline(doc_strings: &mut Vec<DocFragment>, frag: &DocFragment) {
+            if let Some(prev) = doc_strings.last_mut() {
+                if matches!(prev.kind, DocFragmentKind::Include { .. })
+                    || prev.kind != frag.kind
+                    || prev.parent_module != frag.parent_module
+                {
+                    // add a newline for extra padding between segments
+                    prev.need_backline = prev.kind == DocFragmentKind::SugaredDoc
+                        || prev.kind == DocFragmentKind::RawDoc
+                } else {
+                    prev.need_backline = true;
+                }
+            }
+        }
+
         let clean_attr = |(attr, parent_module): (&ast::Attribute, _)| {
             if let Some(value) = attr.doc_str() {
                 trace!("got doc_str={:?}", value);
-                let value = beautify_doc_string(value).to_string();
+                let value = beautify_doc_string(value);
                 let kind = if attr.is_doc_comment() {
                     DocFragmentKind::SugaredDoc
                 } else {
@@ -638,14 +681,20 @@ impl Attributes {
                 };
 
                 let line = doc_line;
-                doc_line += value.lines().count();
+                doc_line += value.as_str().lines().count();
-                doc_strings.push(DocFragment {
+                let frag = DocFragment {
                     line,
                     span: attr.span,
                     doc: value,
                     kind,
                     parent_module,
-                });
+                    need_backline: false,
+                    indent: 0,
+                };
+
+                update_need_backline(&mut doc_strings, &frag);
+
+                doc_strings.push(frag);
 
                 if sp.is_none() {
                     sp = Some(attr.span);
@@ -663,14 +712,18 @@ impl Attributes {
                         } else if let Some((filename, contents)) = Attributes::extract_include(&mi)
                         {
                             let line = doc_line;
-                            doc_line += contents.lines().count();
+                            doc_line += contents.as_str().lines().count();
-                            doc_strings.push(DocFragment {
+                            let frag = DocFragment {
                                 line,
                                 span: attr.span,
                                 doc: contents,
                                 kind: DocFragmentKind::Include { filename },
                                 parent_module,
-                            });
+                                need_backline: false,
+                                indent: 0,
+                            };
+                            update_need_backline(&mut doc_strings, &frag);
+                            doc_strings.push(frag);
                         }
                     }
                 }
@@ -721,14 +774,39 @@ impl Attributes {
 
     /// Finds the `doc` attribute as a NameValue and returns the corresponding
     /// value found.
-    crate fn doc_value(&self) -> Option<&str> {
+    crate fn doc_value(&self) -> Option<String> {
-        self.doc_strings.first().map(|s| s.doc.as_str())
+        let mut iter = self.doc_strings.iter();
+
+        let ori = iter.next()?;
+        let mut out = String::new();
+        add_doc_fragment(&mut out, &ori);
+        while let Some(new_frag) = iter.next() {
+            if matches!(ori.kind, DocFragmentKind::Include { .. })
+                || new_frag.kind != ori.kind
+                || new_frag.parent_module != ori.parent_module
+            {
+                break;
+            }
+            add_doc_fragment(&mut out, &new_frag);
+        }
+        if out.is_empty() { None } else { Some(out) }
+    }
+
+    crate fn collapsed_doc_value_by_module_level(&self) -> FxHashMap<Option<DefId>, String> {
+        let mut ret = FxHashMap::default();
+
+        for new_frag in self.doc_strings.iter() {
+            let out = ret.entry(new_frag.parent_module).or_insert_with(|| String::new());
+            add_doc_fragment(out, &new_frag);
+        }
+        ret
     }
 
     /// Finds all `doc` attributes as NameValues and returns their corresponding values, joined
     /// with newlines.
     crate fn collapsed_doc_value(&self) -> Option<String> {
-        if !self.doc_strings.is_empty() { Some(self.doc_strings.iter().collect()) } else { None }
+        let s: String = self.doc_strings.iter().collect();
+        if s.is_empty() { None } else { Some(s) }
     }
 
     /// Gets links as a vector

src/librustdoc/core.rs

@@ -525,7 +525,7 @@ crate fn run_global_ctxt(
     let mut krate = tcx.sess.time("clean_crate", || clean::krate(&mut ctxt));
 
     if let Some(ref m) = krate.module {
-        if let None | Some("") = m.doc_value() {
+        if m.doc_value().map(|d| d.is_empty()).unwrap_or(true) {
             let help = "The following guide may be of use:\n\
                 https://doc.rust-lang.org/nightly/rustdoc/how-to-write-documentation.html";
             tcx.struct_lint_node(

src/librustdoc/doctest.rs

@@ -987,7 +987,6 @@ impl<'a, 'hir, 'tcx> HirCollector<'a, 'hir, 'tcx> {
             self.collector.names.push(name);
         }
 
-        attrs.collapse_doc_comments();
         attrs.unindent_doc_comments();
         // The collapse-docs pass won't combine sugared/raw doc attributes, or included files with
         // anything else, this will combine them for us.

src/librustdoc/formats/cache.rs

@@ -314,9 +314,10 @@ impl DocFolder for Cache {
                             ty: item.type_(),
                             name: s.to_string(),
                             path: path.join("::"),
-                            desc: item
+                            desc: item.doc_value().map_or_else(
-                                .doc_value()
+                                || String::new(),
-                                .map_or_else(|| String::new(), short_markdown_summary),
+                                |x| short_markdown_summary(&x.as_str()),
+                            ),
                             parent,
                             parent_idx: None,
                             search_type: get_index_search_type(&item),

src/librustdoc/html/render/cache.rs

@@ -78,7 +78,7 @@ crate fn build_index(krate: &clean::Crate, cache: &mut Cache) -> String {
                 ty: item.type_(),
                 name: item.name.unwrap().to_string(),
                 path: fqp[..fqp.len() - 1].join("::"),
-                desc: item.doc_value().map_or_else(|| String::new(), short_markdown_summary),
+                desc: item.doc_value().map_or_else(String::new, |s| short_markdown_summary(&s)),
                 parent: Some(did),
                 parent_idx: None,
                 search_type: get_index_search_type(&item),
@@ -127,7 +127,7 @@ crate fn build_index(krate: &clean::Crate, cache: &mut Cache) -> String {
     let crate_doc = krate
         .module
         .as_ref()
-        .map(|module| module.doc_value().map_or_else(|| String::new(), short_markdown_summary))
+        .map(|module| module.doc_value().map_or_else(String::new, |s| short_markdown_summary(&s)))
         .unwrap_or_default();
 
     #[derive(Serialize)]

src/librustdoc/html/render/mod.rs

@@ -30,7 +30,6 @@ crate mod cache;
 #[cfg(test)]
 mod tests;
 
-use std::borrow::Cow;
 use std::cell::{Cell, RefCell};
 use std::cmp::Ordering;
 use std::collections::{BTreeMap, VecDeque};
@@ -198,11 +197,11 @@ impl SharedContext<'_> {
 
     /// Based on whether the `collapse-docs` pass was run, return either the `doc_value` or the
     /// `collapsed_doc_value` of the given item.
-    crate fn maybe_collapsed_doc_value<'a>(&self, item: &'a clean::Item) -> Option<Cow<'a, str>> {
+    crate fn maybe_collapsed_doc_value<'a>(&self, item: &'a clean::Item) -> Option<String> {
         if self.collapsed {
-            item.collapsed_doc_value().map(|s| s.into())
+            item.collapsed_doc_value().map(|s| s.to_string())
         } else {
-            item.doc_value().map(|s| s.into())
+            item.doc_value().map(|s| s.to_string())
         }
     }
 }
@@ -1622,7 +1621,7 @@ impl Context<'_> {
             let short = short.to_string();
             map.entry(short).or_default().push((
                 myname,
-                Some(item.doc_value().map_or_else(|| String::new(), plain_text_summary)),
+                Some(item.doc_value().map_or_else(String::new, |s| plain_text_summary(&s))),
             ));
         }
 
@@ -1880,7 +1879,7 @@ fn document_short(
         return;
     }
     if let Some(s) = item.doc_value() {
-        let mut summary_html = MarkdownSummaryLine(s, &item.links()).into_string();
+        let mut summary_html = MarkdownSummaryLine(&s, &item.links()).into_string();
 
         if s.contains('\n') {
             let link = format!(r#" <a href="{}">Read more</a>"#, naive_assoc_href(item, link));
@@ -2197,7 +2196,7 @@ fn item_module(w: &mut Buffer, cx: &Context<'_>, item: &clean::Item, items: &[cl
                 let stab = myitem.stability_class(cx.tcx());
                 let add = if stab.is_some() { " " } else { "" };
 
-                let doc_value = myitem.doc_value().unwrap_or("");
+                let doc_value = myitem.doc_value().unwrap_or_else(String::new);
                 write!(
                     w,
                     "<tr class=\"{stab}{add}module-item\">\
@@ -2207,7 +2206,7 @@ fn item_module(w: &mut Buffer, cx: &Context<'_>, item: &clean::Item, items: &[cl
                      </tr>",
                     name = *myitem.name.as_ref().unwrap(),
                     stab_tags = extra_info_tags(myitem, item, cx.tcx()),
-                    docs = MarkdownSummaryLine(doc_value, &myitem.links()).into_string(),
+                    docs = MarkdownSummaryLine(&doc_value, &myitem.links()).into_string(),
                     class = myitem.type_(),
                     add = add,
                     stab = stab.unwrap_or_else(String::new),

src/librustdoc/passes/calculate_doc_coverage.rs

@@ -238,7 +238,7 @@ impl<'a, 'b> fold::DocFolder for CoverageCalculator<'a, 'b> {
                     &i.attrs
                         .doc_strings
                         .iter()
-                        .map(|d| d.doc.as_str())
+                        .map(|d| d.doc.to_string())
                         .collect::<Vec<_>>()
                         .join("\n"),
                     &mut tests,

src/librustdoc/passes/collapse_docs.rs

@@ -1,72 +1,14 @@
-use crate::clean::{self, DocFragment, DocFragmentKind, Item};
+use crate::clean;
 use crate::core::DocContext;
-use crate::fold;
-use crate::fold::DocFolder;
 use crate::passes::Pass;
 
-use std::mem::take;
-
 crate const COLLAPSE_DOCS: Pass = Pass {
     name: "collapse-docs",
     run: collapse_docs,
     description: "concatenates all document attributes into one document attribute",
 };
 
-crate fn collapse_docs(krate: clean::Crate, _: &DocContext<'_>) -> clean::Crate {
+crate fn collapse_docs(mut krate: clean::Crate, _: &DocContext<'_>) -> clean::Crate {
-    let mut krate = Collapser.fold_crate(krate);
     krate.collapsed = true;
     krate
 }
-
-struct Collapser;
-
-impl fold::DocFolder for Collapser {
-    fn fold_item(&mut self, mut i: Item) -> Option<Item> {
-        i.attrs.collapse_doc_comments();
-        Some(self.fold_item_recur(i))
-    }
-}
-
-fn collapse(doc_strings: &mut Vec<DocFragment>) {
-    let mut docs = vec![];
-    let mut last_frag: Option<DocFragment> = None;
-
-    for frag in take(doc_strings) {
-        if let Some(mut curr_frag) = last_frag.take() {
-            let curr_kind = &curr_frag.kind;
-            let new_kind = &frag.kind;
-
-            if matches!(*curr_kind, DocFragmentKind::Include { .. })
-                || curr_kind != new_kind
-                || curr_frag.parent_module != frag.parent_module
-            {
-                if *curr_kind == DocFragmentKind::SugaredDoc
-                    || *curr_kind == DocFragmentKind::RawDoc
-                {
-                    // add a newline for extra padding between segments
-                    curr_frag.doc.push('\n');
-                }
-                docs.push(curr_frag);
-                last_frag = Some(frag);
-            } else {
-                curr_frag.doc.push('\n');
-                curr_frag.doc.push_str(&frag.doc);
-                curr_frag.span = curr_frag.span.to(frag.span);
-                last_frag = Some(curr_frag);
-            }
-        } else {
-            last_frag = Some(frag);
-        }
-    }
-
-    if let Some(frag) = last_frag.take() {
-        docs.push(frag);
-    }
-    *doc_strings = docs;
-}
-
-impl clean::Attributes {
-    crate fn collapse_doc_comments(&mut self) {
-        collapse(&mut self.doc_strings);
-    }
-}

src/librustdoc/passes/collect_intra_doc_links.rs

@@ -891,37 +891,20 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> {
         // In the presence of re-exports, this is not the same as the module of the item.
         // Rather than merging all documentation into one, resolve it one attribute at a time
         // so we know which module it came from.
-        let mut attrs = item.attrs.doc_strings.iter().peekable();
+        for (parent_module, doc) in item.attrs.collapsed_doc_value_by_module_level() {
-        while let Some(attr) = attrs.next() {
+            debug!("combined_docs={}", doc);
-            // `collapse_docs` does not have the behavior we want:
-            // we want `///` and `#[doc]` to count as the same attribute,
-            // but currently it will treat them as separate.
-            // As a workaround, combine all attributes with the same parent module into the same attribute.
-            let mut combined_docs = attr.doc.clone();
-            loop {
-                match attrs.peek() {
-                    Some(next) if next.parent_module == attr.parent_module => {
-                        combined_docs.push('\n');
-                        combined_docs.push_str(&attrs.next().unwrap().doc);
-                    }
-                    _ => break,
-                }
-            }
-            debug!("combined_docs={}", combined_docs);
 
-            let (krate, parent_node) = if let Some(id) = attr.parent_module {
+            let (krate, parent_node) = if let Some(id) = parent_module {
-                trace!("docs {:?} came from {:?}", attr.doc, id);
                 (id.krate, Some(id))
             } else {
-                trace!("no parent found for {:?}", attr.doc);
                 (item.def_id.krate, parent_node)
             };
             // NOTE: if there are links that start in one crate and end in another, this will not resolve them.
             // This is a degenerate case and it's not supported by rustdoc.
-            for (ori_link, link_range) in markdown_links(&combined_docs) {
+            for (ori_link, link_range) in markdown_links(&doc) {
                 let link = self.resolve_link(
                     &item,
-                    &combined_docs,
+                    &doc,
                     &self_name,
                     parent_node,
                     krate,

src/librustdoc/passes/unindent_comments.rs

@@ -68,7 +68,7 @@ fn unindent_fragments(docs: &mut Vec<DocFragment>) {
     let min_indent = match docs
         .iter()
         .map(|fragment| {
-            fragment.doc.lines().fold(usize::MAX, |min_indent, line| {
+            fragment.doc.as_str().lines().fold(usize::MAX, |min_indent, line| {
                 if line.chars().all(|c| c.is_whitespace()) {
                     min_indent
                 } else {
@@ -87,7 +87,7 @@ fn unindent_fragments(docs: &mut Vec<DocFragment>) {
     };
 
     for fragment in docs {
-        if fragment.doc.lines().count() == 0 {
+        if fragment.doc.as_str().lines().count() == 0 {
             continue;
         }
 
@@ -97,18 +97,6 @@ fn unindent_fragments(docs: &mut Vec<DocFragment>) {
             min_indent
         };
 
-        fragment.doc = fragment
+        fragment.indent = min_indent;
-            .doc
-            .lines()
-            .map(|line| {
-                if line.chars().all(|c| c.is_whitespace()) {
-                    line.to_string()
-                } else {
-                    assert!(line.len() >= min_indent);
-                    line[min_indent..].to_string()
-                }
-            })
-            .collect::<Vec<_>>()
-            .join("\n");
     }
 }

src/librustdoc/passes/unindent_comments/tests.rs

@@ -1,21 +1,40 @@
 use super::*;
 use rustc_span::source_map::DUMMY_SP;
+use rustc_span::symbol::Symbol;
+use rustc_span::with_default_session_globals;
 
 fn create_doc_fragment(s: &str) -> Vec<DocFragment> {
     vec![DocFragment {
         line: 0,
         span: DUMMY_SP,
         parent_module: None,
-        doc: s.to_string(),
+        doc: Symbol::intern(s),
         kind: DocFragmentKind::SugaredDoc,
+        need_backline: false,
+        indent: 0,
     }]
 }
 
 #[track_caller]
 fn run_test(input: &str, expected: &str) {
-    let mut s = create_doc_fragment(input);
+    with_default_session_globals(|| {
-    unindent_fragments(&mut s);
+        let mut s = create_doc_fragment(input);
-    assert_eq!(s[0].doc, expected);
+        unindent_fragments(&mut s);
+        assert_eq!(
+            &s[0]
+                .doc
+                .as_str()
+                .lines()
+                .map(|l| if l.len() > s[0].indent {
+                    l[s[0].indent..].to_string()
+                } else {
+                    String::new()
+                })
+                .collect::<Vec<_>>()
+                .join("\n"),
+            expected
+        );
+    });
 }
 
 #[test]