Auto merge of #80295 - GuillaumeGomez:beautify-rework, r=petrochenkov

Rework beautify_doc_string so that it returns a Symbol instead of a String

This commit comes from #80261, the goal here is to inspect the impact on performance of this change on its own.

The idea of rewriting `beautify_doc_string` is to not go through `String` if we don't need to update the doc comment to be able to keep the original `Symbol` and also to have better performance.

r? `@jyn514`

Author: bors

Diff for: compiler/rustc_ast/src/util/comments.rs

@@ -25,9 +25,8 @@ pub struct Comment {
 
 /// Makes a doc string more presentable to users.
 /// Used by rustdoc and perhaps other tools, but not by rustc.
-pub fn beautify_doc_string(data: Symbol) -> String {
-    /// remove whitespace-only lines from the start/end of lines
-    fn vertical_trim(lines: Vec<String>) -> Vec<String> {
+pub fn beautify_doc_string(data: Symbol) -> Symbol {
+    fn get_vertical_trim(lines: &[&str]) -> Option<(usize, usize)> {
         let mut i = 0;
         let mut j = lines.len();
         // first line of all-stars should be omitted
@@ -47,55 +46,58 @@ pub fn beautify_doc_string(data: Symbol) -> String {
             j -= 1;
         }
 
-        lines[i..j].to_vec()
+        if i != 0 || j != lines.len() { Some((i, j)) } else { None }
     }
 
-    /// remove a "[ \t]*\*" block from each line, if possible
-    fn horizontal_trim(lines: Vec<String>) -> Vec<String> {
+    fn get_horizontal_trim(lines: &[&str]) -> Option<usize> {
         let mut i = usize::MAX;
-        let mut can_trim = true;
         let mut first = true;
 
-        for line in &lines {
+        for line in lines {
             for (j, c) in line.chars().enumerate() {
                 if j > i || !"* \t".contains(c) {
-                    can_trim = false;
-                    break;
+                    return None;
                 }
                 if c == '*' {
                     if first {
                         i = j;
                         first = false;
                     } else if i != j {
-                        can_trim = false;
+                        return None;
                     }
                     break;
                 }
             }
             if i >= line.len() {
-                can_trim = false;
-            }
-            if !can_trim {
-                break;
+                return None;
             }
         }
+        Some(i)
+    }
 
-        if can_trim {
-            lines.iter().map(|line| (&line[i + 1..line.len()]).to_string()).collect()
+    let data_s = data.as_str();
+    if data_s.contains('\n') {
+        let mut lines = data_s.lines().collect::<Vec<&str>>();
+        let mut changes = false;
+        let lines = if let Some((i, j)) = get_vertical_trim(&lines) {
+            changes = true;
+            // remove whitespace-only lines from the start/end of lines
+            &mut lines[i..j]
         } else {
-            lines
+            &mut lines
+        };
+        if let Some(horizontal) = get_horizontal_trim(&lines) {
+            changes = true;
+            // remove a "[ \t]*\*" block from each line, if possible
+            for line in lines.iter_mut() {
+                *line = &line[horizontal + 1..];
+            }
+        }
+        if changes {
+            return Symbol::intern(&lines.join("\n"));
         }
     }
-
-    let data = data.as_str();
-    if data.contains('\n') {
-        let lines = data.lines().map(|s| s.to_string()).collect::<Vec<String>>();
-        let lines = vertical_trim(lines);
-        let lines = horizontal_trim(lines);
-        lines.join("\n")
-    } else {
-        data.to_string()
-    }
+    data
 }
 
 /// Returns `None` if the first `col` chars of `s` contain a non-whitespace char.

Diff for: compiler/rustc_ast/src/util/comments/tests.rs

@@ -6,7 +6,7 @@ fn test_block_doc_comment_1() {
     with_default_session_globals(|| {
         let comment = "\n * Test \n **  Test\n *   Test\n";
         let stripped = beautify_doc_string(Symbol::intern(comment));
-        assert_eq!(stripped, " Test \n*  Test\n   Test");
+        assert_eq!(stripped.as_str(), " Test \n*  Test\n   Test");
     })
 }
 
@@ -15,7 +15,7 @@ fn test_block_doc_comment_2() {
     with_default_session_globals(|| {
         let comment = "\n * Test\n *  Test\n";
         let stripped = beautify_doc_string(Symbol::intern(comment));
-        assert_eq!(stripped, " Test\n  Test");
+        assert_eq!(stripped.as_str(), " Test\n  Test");
     })
 }
 
@@ -24,20 +24,20 @@ fn test_block_doc_comment_3() {
     with_default_session_globals(|| {
         let comment = "\n let a: *i32;\n *a = 5;\n";
         let stripped = beautify_doc_string(Symbol::intern(comment));
-        assert_eq!(stripped, " let a: *i32;\n *a = 5;");
+        assert_eq!(stripped.as_str(), " let a: *i32;\n *a = 5;");
     })
 }
 
 #[test]
 fn test_line_doc_comment() {
     with_default_session_globals(|| {
         let stripped = beautify_doc_string(Symbol::intern(" test"));
-        assert_eq!(stripped, " test");
+        assert_eq!(stripped.as_str(), " test");
         let stripped = beautify_doc_string(Symbol::intern("! test"));
-        assert_eq!(stripped, "! test");
+        assert_eq!(stripped.as_str(), "! test");
         let stripped = beautify_doc_string(Symbol::intern("test"));
-        assert_eq!(stripped, "test");
+        assert_eq!(stripped.as_str(), "test");
         let stripped = beautify_doc_string(Symbol::intern("!test"));
-        assert_eq!(stripped, "!test");
+        assert_eq!(stripped.as_str(), "!test");
     })
 }

Diff for: compiler/rustc_save_analysis/src/lib.rs

@@ -825,7 +825,7 @@ impl<'tcx> SaveContext<'tcx> {
         for attr in attrs {
             if let Some(val) = attr.doc_str() {
                 // FIXME: Should save-analysis beautify doc strings itself or leave it to users?
-                result.push_str(&beautify_doc_string(val));
+                result.push_str(&beautify_doc_string(val).as_str());
                 result.push('\n');
             } else if self.tcx.sess.check_name(attr, sym::doc) {
                 if let Some(meta_list) = attr.meta_item_list() {

Diff for: src/librustdoc/clean/types.rs

@@ -621,7 +621,7 @@ impl Attributes {
         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);
+                let value = beautify_doc_string(value).to_string();
                 let kind = if attr.is_doc_comment() {
                     DocFragmentKind::SugaredDoc
                 } else {