Auto merge of rust-lang#79762 - Swatinem:remap-doctest-coverage, r=Swatinem

Remap instrument-coverage line numbers in doctests

This uses the `SourceMap::doctest_offset_line` method to re-map line
numbers from doctests. Remapping columns is not yet done, and rustdoc
still does not output the correct filename when running doctests in a
workspace.

Part of rust-lang#79417 although I dont consider that fixed until both filenames
and columns are mapped correctly.

r? `@richkadel`

I might jump on zulip the comming days. Still need to figure out how to properly write tests for this, and deal with other doctest issues in the meantime.

Author: bors

compiler/rustc_mir/src/transform/coverage/mod.rs

@@ -30,6 +30,7 @@ use rustc_middle::mir::{
 };
 use rustc_middle::ty::TyCtxt;
 use rustc_span::def_id::DefId;
+use rustc_span::source_map::SourceMap;
 use rustc_span::{CharPos, Pos, SourceFile, Span, Symbol};
 
 /// A simple error message wrapper for `coverage::Error`s.
@@ -311,7 +312,7 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> {
                 self.mir_body,
                 counter_kind,
                 self.bcb_leader_bb(bcb),
-                Some(make_code_region(file_name, &self.source_file, span, body_span)),
+                Some(make_code_region(source_map, file_name, &self.source_file, span, body_span)),
             );
         }
     }
@@ -489,6 +490,7 @@ fn inject_intermediate_expression(mir_body: &mut mir::Body<'tcx>, expression: Co
 
 /// Convert the Span into its file name, start line and column, and end line and column
 fn make_code_region(
+    source_map: &SourceMap,
     file_name: Symbol,
     source_file: &Lrc<SourceFile>,
     span: Span,
@@ -508,6 +510,8 @@ fn make_code_region(
     } else {
         source_file.lookup_file_pos(span.hi())
     };
+    let start_line = source_map.doctest_offset_line(&source_file.name, start_line);
+    let end_line = source_map.doctest_offset_line(&source_file.name, end_line);
     CodeRegion {
         file_name,
         start_line: start_line as u32,

compiler/rustc_span/src/lib.rs

@@ -182,7 +182,7 @@ impl std::fmt::Display for FileName {
         use FileName::*;
         match *self {
             Real(RealFileName::Named(ref path)) => write!(fmt, "{}", path.display()),
-            // FIXME: might be nice to display both compoments of Devirtualized.
+            // FIXME: might be nice to display both components of Devirtualized.
             // But for now (to backport fix for issue #70924), best to not
             // perturb diagnostics so its obvious test suite still works.
             Real(RealFileName::Devirtualized { ref local_path, virtual_name: _ }) => {

src/librustdoc/doctest.rs

@@ -247,9 +247,10 @@ fn run_test(
     edition: Edition,
     outdir: DirState,
     path: PathBuf,
+    test_id: &str,
 ) -> Result<(), TestFailure> {
     let (test, line_offset, supports_color) =
-        make_test(test, Some(cratename), as_test_harness, opts, edition);
+        make_test(test, Some(cratename), as_test_harness, opts, edition, Some(test_id));
 
     let output_file = outdir.path().join("rust_out");
 
@@ -387,6 +388,7 @@ crate fn make_test(
     dont_insert_main: bool,
     opts: &TestOptions,
     edition: Edition,
+    test_id: Option<&str>,
 ) -> (String, usize, bool) {
     let (crate_attrs, everything_else, crates) = partition_source(s);
     let everything_else = everything_else.trim();
@@ -542,16 +544,41 @@ crate fn make_test(
         prog.push_str(everything_else);
     } else {
         let returns_result = everything_else.trim_end().ends_with("(())");
+        // Give each doctest main function a unique name.
+        // This is for example needed for the tooling around `-Z instrument-coverage`.
+        let inner_fn_name = if let Some(test_id) = test_id {
+            format!("_doctest_main_{}", test_id)
+        } else {
+            "_inner".into()
+        };
+        let inner_attr = if test_id.is_some() { "#[allow(non_snake_case)] " } else { "" };
         let (main_pre, main_post) = if returns_result {
             (
-                "fn main() { fn _inner() -> Result<(), impl core::fmt::Debug> {",
-                "}\n_inner().unwrap() }",
+                format!(
+                    "fn main() {{ {}fn {}() -> Result<(), impl core::fmt::Debug> {{\n",
+                    inner_attr, inner_fn_name
+                ),
+                format!("\n}}; {}().unwrap() }}", inner_fn_name),
+            )
+        } else if test_id.is_some() {
+            (
+                format!("fn main() {{ {}fn {}() {{\n", inner_attr, inner_fn_name),
+                format!("\n}}; {}() }}", inner_fn_name),
             )
         } else {
-            ("fn main() {\n", "\n}")
+            ("fn main() {\n".into(), "\n}".into())
         };
-        prog.extend([main_pre, everything_else, main_post].iter().cloned());
+        // Note on newlines: We insert a line/newline *before*, and *after*
+        // the doctest and adjust the `line_offset` accordingly.
+        // In the case of `-Z instrument-coverage`, this means that the generated
+        // inner `main` function spans from the doctest opening codeblock to the
+        // closing one. For example
+        // /// ``` <- start of the inner main
+        // /// <- code under doctest
+        // /// ``` <- end of the inner main
         line_offset += 1;
+
+        prog.extend([&main_pre, everything_else, &main_post].iter().cloned());
     }
 
     debug!("final doctest:\n{}", prog);
@@ -749,28 +776,24 @@ impl Tester for Collector {
             _ => PathBuf::from(r"doctest.rs"),
         };
 
+        // For example `module/file.rs` would become `module_file_rs`
+        let file = filename
+            .to_string()
+            .chars()
+            .map(|c| if c.is_ascii_alphanumeric() { c } else { '_' })
+            .collect::<String>();
+        let test_id = format!(
+            "{file}_{line}_{number}",
+            file = file,
+            line = line,
+            number = {
+                // Increases the current test number, if this file already
+                // exists or it creates a new entry with a test number of 0.
+                self.visited_tests.entry((file.clone(), line)).and_modify(|v| *v += 1).or_insert(0)
+            },
+        );
         let outdir = if let Some(mut path) = options.persist_doctests.clone() {
-            // For example `module/file.rs` would become `module_file_rs`
-            let folder_name = filename
-                .to_string()
-                .chars()
-                .map(|c| if c == '\\' || c == '/' || c == '.' { '_' } else { c })
-                .collect::<String>();
-
-            path.push(format!(
-                "{krate}_{file}_{line}_{number}",
-                krate = cratename,
-                file = folder_name,
-                line = line,
-                number = {
-                    // Increases the current test number, if this file already
-                    // exists or it creates a new entry with a test number of 0.
-                    self.visited_tests
-                        .entry((folder_name.clone(), line))
-                        .and_modify(|v| *v += 1)
-                        .or_insert(0)
-                },
-            ));
+            path.push(&test_id);
 
             std::fs::create_dir_all(&path)
                 .expect("Couldn't create directory for doctest executables");
@@ -817,6 +840,7 @@ impl Tester for Collector {
                     edition,
                     outdir,
                     path,
+                    &test_id,
                 );
 
                 if let Err(err) = res {

src/librustdoc/doctest/tests.rs

@@ -11,7 +11,7 @@ fn main() {
 assert_eq!(2+2, 4);
 }"
     .to_string();
-    let (output, len, _) = make_test(input, None, false, &opts, DEFAULT_EDITION);
+    let (output, len, _) = make_test(input, None, false, &opts, DEFAULT_EDITION, None);
     assert_eq!((output, len), (expected, 2));
 }
 
@@ -26,7 +26,7 @@ fn main() {
 assert_eq!(2+2, 4);
 }"
     .to_string();
-    let (output, len, _) = make_test(input, Some("asdf"), false, &opts, DEFAULT_EDITION);
+    let (output, len, _) = make_test(input, Some("asdf"), false, &opts, DEFAULT_EDITION, None);
     assert_eq!((output, len), (expected, 2));
 }
 
@@ -44,7 +44,7 @@ use asdf::qwop;
 assert_eq!(2+2, 4);
 }"
     .to_string();
-    let (output, len, _) = make_test(input, Some("asdf"), false, &opts, DEFAULT_EDITION);
+    let (output, len, _) = make_test(input, Some("asdf"), false, &opts, DEFAULT_EDITION, None);
     assert_eq!((output, len), (expected, 3));
 }
 
@@ -61,7 +61,7 @@ use asdf::qwop;
 assert_eq!(2+2, 4);
 }"
     .to_string();
-    let (output, len, _) = make_test(input, Some("asdf"), false, &opts, DEFAULT_EDITION);
+    let (output, len, _) = make_test(input, Some("asdf"), false, &opts, DEFAULT_EDITION, None);
     assert_eq!((output, len), (expected, 2));
 }
 
@@ -79,7 +79,7 @@ use std::*;
 assert_eq!(2+2, 4);
 }"
     .to_string();
-    let (output, len, _) = make_test(input, Some("std"), false, &opts, DEFAULT_EDITION);
+    let (output, len, _) = make_test(input, Some("std"), false, &opts, DEFAULT_EDITION, None);
     assert_eq!((output, len), (expected, 2));
 }
 
@@ -98,7 +98,7 @@ use asdf::qwop;
 assert_eq!(2+2, 4);
 }"
     .to_string();
-    let (output, len, _) = make_test(input, Some("asdf"), false, &opts, DEFAULT_EDITION);
+    let (output, len, _) = make_test(input, Some("asdf"), false, &opts, DEFAULT_EDITION, None);
     assert_eq!((output, len), (expected, 2));
 }
 
@@ -115,7 +115,7 @@ use asdf::qwop;
 assert_eq!(2+2, 4);
 }"
     .to_string();
-    let (output, len, _) = make_test(input, Some("asdf"), false, &opts, DEFAULT_EDITION);
+    let (output, len, _) = make_test(input, Some("asdf"), false, &opts, DEFAULT_EDITION, None);
     assert_eq!((output, len), (expected, 2));
 }
 
@@ -134,7 +134,7 @@ use asdf::qwop;
 assert_eq!(2+2, 4);
 }"
     .to_string();
-    let (output, len, _) = make_test(input, Some("asdf"), false, &opts, DEFAULT_EDITION);
+    let (output, len, _) = make_test(input, Some("asdf"), false, &opts, DEFAULT_EDITION, None);
     assert_eq!((output, len), (expected, 3));
 
     // Adding more will also bump the returned line offset.
@@ -147,7 +147,7 @@ use asdf::qwop;
 assert_eq!(2+2, 4);
 }"
     .to_string();
-    let (output, len, _) = make_test(input, Some("asdf"), false, &opts, DEFAULT_EDITION);
+    let (output, len, _) = make_test(input, Some("asdf"), false, &opts, DEFAULT_EDITION, None);
     assert_eq!((output, len), (expected, 4));
 }
 
@@ -164,7 +164,7 @@ fn main() {
 assert_eq!(2+2, 4);
 }"
     .to_string();
-    let (output, len, _) = make_test(input, None, false, &opts, DEFAULT_EDITION);
+    let (output, len, _) = make_test(input, None, false, &opts, DEFAULT_EDITION, None);
     assert_eq!((output, len), (expected, 2));
 }
 
@@ -180,7 +180,7 @@ fn main() {
     assert_eq!(2+2, 4);
 }"
     .to_string();
-    let (output, len, _) = make_test(input, None, false, &opts, DEFAULT_EDITION);
+    let (output, len, _) = make_test(input, None, false, &opts, DEFAULT_EDITION, None);
     assert_eq!((output, len), (expected, 1));
 }
 
@@ -196,7 +196,7 @@ fn main() {
 assert_eq!(2+2, 4);
 }"
     .to_string();
-    let (output, len, _) = make_test(input, None, false, &opts, DEFAULT_EDITION);
+    let (output, len, _) = make_test(input, None, false, &opts, DEFAULT_EDITION, None);
     assert_eq!((output, len), (expected, 2));
 }
 
@@ -210,7 +210,7 @@ assert_eq!(2+2, 4);";
 //Ceci n'est pas une `fn main`
 assert_eq!(2+2, 4);"
         .to_string();
-    let (output, len, _) = make_test(input, None, true, &opts, DEFAULT_EDITION);
+    let (output, len, _) = make_test(input, None, true, &opts, DEFAULT_EDITION, None);
     assert_eq!((output, len), (expected, 1));
 }
 
@@ -224,7 +224,7 @@ fn make_test_display_warnings() {
 assert_eq!(2+2, 4);
 }"
     .to_string();
-    let (output, len, _) = make_test(input, None, false, &opts, DEFAULT_EDITION);
+    let (output, len, _) = make_test(input, None, false, &opts, DEFAULT_EDITION, None);
     assert_eq!((output, len), (expected, 1));
 }
 
@@ -242,7 +242,7 @@ assert_eq!(2+2, 4);
 }"
     .to_string();
 
-    let (output, len, _) = make_test(input, None, false, &opts, DEFAULT_EDITION);
+    let (output, len, _) = make_test(input, None, false, &opts, DEFAULT_EDITION, None);
     assert_eq!((output, len), (expected, 2));
 
     let input = "extern crate hella_qwop;
@@ -256,7 +256,7 @@ assert_eq!(asdf::foo, 4);
 }"
     .to_string();
 
-    let (output, len, _) = make_test(input, Some("asdf"), false, &opts, DEFAULT_EDITION);
+    let (output, len, _) = make_test(input, Some("asdf"), false, &opts, DEFAULT_EDITION, None);
     assert_eq!((output, len), (expected, 3));
 }
 
@@ -274,6 +274,41 @@ test_wrapper! {
 }"
     .to_string();
 
-    let (output, len, _) = make_test(input, Some("my_crate"), false, &opts, DEFAULT_EDITION);
+    let (output, len, _) = make_test(input, Some("my_crate"), false, &opts, DEFAULT_EDITION, None);
     assert_eq!((output, len), (expected, 1));
 }
+
+#[test]
+fn make_test_returns_result() {
+    // creates an inner function and unwraps it
+    let opts = TestOptions::default();
+    let input = "use std::io;
+let mut input = String::new();
+io::stdin().read_line(&mut input)?;
+Ok::<(), io:Error>(())";
+    let expected = "#![allow(unused)]
+fn main() { fn _inner() -> Result<(), impl core::fmt::Debug> {
+use std::io;
+let mut input = String::new();
+io::stdin().read_line(&mut input)?;
+Ok::<(), io:Error>(())
+}; _inner().unwrap() }"
+        .to_string();
+    let (output, len, _) = make_test(input, None, false, &opts, DEFAULT_EDITION, None);
+    assert_eq!((output, len), (expected, 2));
+}
+
+#[test]
+fn make_test_named_wrapper() {
+    // creates an inner function with a specific name
+    let opts = TestOptions::default();
+    let input = "assert_eq!(2+2, 4);";
+    let expected = "#![allow(unused)]
+fn main() { #[allow(non_snake_case)] fn _doctest_main__some_unique_name() {
+assert_eq!(2+2, 4);
+}; _doctest_main__some_unique_name() }"
+        .to_string();
+    let (output, len, _) =
+        make_test(input, None, false, &opts, DEFAULT_EDITION, Some("_some_unique_name"));
+    assert_eq!((output, len), (expected, 2));
+}

src/librustdoc/html/markdown.rs

@@ -248,7 +248,7 @@ impl<'a, I: Iterator<Item = Event<'a>>> Iterator for CodeBlocks<'_, 'a, I> {
                 .join("\n");
             let krate = krate.as_ref().map(|s| &**s);
             let (test, _, _) =
-                doctest::make_test(&test, krate, false, &Default::default(), edition);
+                doctest::make_test(&test, krate, false, &Default::default(), edition, None);
             let channel = if test.contains("#![feature(") { "&amp;version=nightly" } else { "" };
 
             let edition_string = format!("&amp;edition={}", edition);