Investigate removing need_backline from rustdoc::clean::DocFragment

[jyn514]

I expected this change to work:

diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs
index 2bd90f67cf4..a7ef658e811 100644
--- a/src/librustdoc/clean/types.rs
+++ b/src/librustdoc/clean/types.rs
@@ -936,8 +936,8 @@ fn get_word_attr(mut self, word: Symbol) -> Option<ast::NestedMetaItem> {
 //
 // * Applying the computed indent to each lines in each doc fragment (a `DocFragment` can contain
 //   multiple lines in case of `#[doc = ""]`).
-// * Adding backlines between `DocFragment`s and adding an extra one if required (stored in the
-//   `need_backline` field).
+//
+// NOTE: this can contain trailing newlines. Make sure you remove the final newline.
 fn add_doc_fragment(out: &mut String, frag: &DocFragment) {
     let s = frag.doc.as_str();
     let mut iter = s.lines().peekable();
@@ -952,9 +952,7 @@ fn add_doc_fragment(out: &mut String, frag: &DocFragment) {
             out.push('\n');
         }
     }
-    if frag.need_backline {
-        out.push('\n');
-    }
+    out.push('\n');
 }
 
 /// Collapse a collection of [`DocFragment`]s into one string,
@@ -964,6 +962,9 @@ fn add_doc_fragment(out: &mut String, frag: &DocFragment) {
     for frag in doc_strings {
         add_doc_fragment(&mut acc, frag);
     }
+    if acc.ends_with('\n') {
+        acc.pop();
+    }
     acc
 }
 
@@ -1091,6 +1092,9 @@ fn update_need_backline(doc_strings: &mut Vec<DocFragment>) {
             }
             add_doc_fragment(&mut out, new_frag);
         }
+        if out.ends_with('\n') {
+            out.pop();
+        }
         if out.is_empty() { None } else { Some(out) }
     }
 
@@ -1103,6 +1107,9 @@ fn update_need_backline(doc_strings: &mut Vec<DocFragment>) {
         for new_frag in self.doc_strings.iter() {
             let out = ret.entry(new_frag.parent_module).or_default();
             add_doc_fragment(out, new_frag);
+            if out.ends_with('\n') {
+                out.pop();
+            }
         }
         ret
     }

But it causes source_span_for_markdown_range to panic:

---- [rustdoc] rustdoc/intra-doc/trait-impl.rs stdout ----

error: rustdoc failed!
------------------------------------------
stderr:
------------------------------------------
thread 'rustc' panicked at 'could not find markdown in source', src/librustdoc/passes/mod.rs:199:48

Backtrace

thread 'rustc' panicked at 'could not find markdown in source', src/librustdoc/passes/mod.rs:199:48
stack backtrace:
   0: rust_begin_unwind
             at /rustc/d3f300477b89e70dd42379ba53c0e8ff74e9c694/library/std/src/panicking.rs:498:5
   1: core::panicking::panic_fmt
             at /rustc/d3f300477b89e70dd42379ba53c0e8ff74e9c694/library/core/src/panicking.rs:107:14
   2: core::panicking::panic_display
             at /rustc/d3f300477b89e70dd42379ba53c0e8ff74e9c694/library/core/src/panicking.rs:63:5
   3: core::option::expect_failed
             at /rustc/d3f300477b89e70dd42379ba53c0e8ff74e9c694/library/core/src/option.rs:1819:5
   4: rustdoc::passes::source_span_for_markdown_range
   5: <rustdoc::passes::collect_intra_doc_links::report_diagnostic<rustdoc::passes::collect_intra_doc_links::resolution_failure::{closure#0}>::{closure#0} as core::ops::function::FnOnce<(rustc_middle::lint::LintDiagnosticBuilder,)>>::call_once::{shim:vtable#0}
   6: rustc_middle::lint::struct_lint_level::struct_lint_level_impl
   7: rustc_middle::lint::struct_lint_level::<rustdoc::passes::collect_intra_doc_links::report_diagnostic<rustdoc::passes::collect_intra_doc_links::resolution_failure::{closure#0}>::{closure#0}>
   8: <rustc_middle::ty::context::TyCtxt>::struct_span_lint_hir::<rustc_span::span_encoding::Span, rustdoc::passes::collect_intra_doc_links::report_diagnostic<rustdoc::passes::collect_intra_doc_links::resolution_failure::{closure#0}>::{closure#0}>
   9: rustdoc::passes::collect_intra_doc_links::report_diagnostic::<rustdoc::passes::collect_intra_doc_links::resolution_failure::{closure#0}>
  10: rustdoc::passes::collect_intra_doc_links::resolution_failure
  11: <rustdoc::passes::collect_intra_doc_links::LinkCollector as rustdoc::visit::DocVisitor>::visit_item
  12: <rustdoc::passes::collect_intra_doc_links::LinkCollector as rustdoc::visit::DocVisitor>::visit_item_recur
  13: <rustdoc::passes::collect_intra_doc_links::LinkCollector as rustdoc::visit::DocVisitor>::visit_item
  14: <rustdoc::passes::collect_intra_doc_links::LinkCollector as rustdoc::visit::DocVisitor>::visit_item_recur
  15: <rustdoc::passes::collect_intra_doc_links::LinkCollector as rustdoc::visit::DocVisitor>::visit_item
  16: <rustdoc::passes::collect_intra_doc_links::LinkCollector as rustdoc::visit::DocVisitor>::visit_crate
  17: rustdoc::passes::collect_intra_doc_links::collect_intra_doc_links
  18: <rustc_session::session::Session>::time::<rustdoc::clean::types::Crate, rustdoc::core::run_global_ctxt::{closure#11}>
  19: rustdoc::core::run_global_ctxt
  20: <rustc_session::session::Session>::time::<(rustdoc::clean::types::Crate, rustdoc::config::RenderOptions, rustdoc::formats::cache::Cache), rustdoc::main_options::{closure#0}::{closure#0}::{closure#0}::{closure#0}>
  21: <rustc_interface::passes::QueryContext>::enter::<rustdoc::main_options::{closure#0}::{closure#0}::{closure#0}, core::result::Result<(), rustc_errors::ErrorReported>>
  22: <rustc_interface::interface::Compiler>::enter::<rustdoc::main_options::{closure#0}::{closure#0}, core::result::Result<(), rustc_errors::ErrorReported>>
  23: rustc_span::with_source_map::<core::result::Result<(), rustc_errors::ErrorReported>, rustc_interface::interface::create_compiler_and_run<core::result::Result<(), rustc_errors::ErrorReported>, rustdoc::main_options::{closure#0}>::{closure#1}>
  24: rustc_interface::interface::create_compiler_and_run::<core::result::Result<(), rustc_errors::ErrorReported>, rustdoc::main_options::{closure#0}>
  25: rustdoc::main_options
  26: <scoped_tls::ScopedKey<rustc_span::SessionGlobals>>::set::<rustc_interface::util::setup_callbacks_and_run_in_thread_pool_with_globals<rustdoc::main_args::{closure#0}, core::result::Result<(), rustc_errors::ErrorReported>>::{closure#0}::{closure#0}, core::result::Result<(), rustc_errors::ErrorReported>>

I don't have time to look into it right now, but I wouldn't expect it to be too hard to fix and it would reduce the memory usage of rustdoc by quite a lot. I'd suggest starting by adding a bunch of assertions that the old and new behavior of collapsed_doc_value is the same and investigating the assertion failures.

[xTachyon]

@rustbot claim

[vacuus]

Since add_doc_fragment would unconditionally add a final newline, shouldn't the note say that (instead of "this can") and the 'pop's be unconditional? Better yet, couldn't the final newline just not be 'push'ed? Across the entire codebase, there's just one other invocation of 'add_doc_fragment' (currently line 1087 here); if that's intentional, a newline could be unconditionally pushed right after. it seems that that doesn't need a newline popped. Instead, I think the last function you modified (collapsed_doc_value_by_module_level) should only have a newline popped for the last DocFragment, as need_backline would only be false for the last DocFragment, as far as I can tell.
I made a commit to that effect in the PR above, though I have to resolve the merge conflict somehow.

[jyn514]

@vacuus feel free to try that out and see if it works :) like I said, I don't have time to work on it right now.