diff --git a/compiler/rustc_mir/src/transform/inline.rs b/compiler/rustc_mir/src/transform/inline.rs index 6e7575c1d71bb..f06172ab7da8a 100644 --- a/compiler/rustc_mir/src/transform/inline.rs +++ b/compiler/rustc_mir/src/transform/inline.rs @@ -41,6 +41,15 @@ impl<'tcx> MirPass<'tcx> for Inline { return; } + if tcx.sess.opts.debugging_opts.instrument_coverage { + // Since `Inline` happens after `InstrumentCoverage`, the function-specific coverage + // counters can be invalidated, such as by merging coverage counter statements from + // a pre-inlined function into a different function. This kind of change is invalid, + // so inlining must be skipped. Note: This check is performed here so inlining can + // be disabled without preventing other optimizations (regardless of `mir_opt_level`). + return; + } + if inline(tcx, body) { debug!("running simplify cfg on {:?}", body.source); CfgSimplifier::new(body).simplify(); diff --git a/compiler/rustc_session/src/config.rs b/compiler/rustc_session/src/config.rs index c9ddcbdb5f5cc..e42889670b9dc 100644 --- a/compiler/rustc_session/src/config.rs +++ b/compiler/rustc_session/src/config.rs @@ -1830,11 +1830,17 @@ pub fn build_session_options(matches: &getopts::Matches) -> Options { } if debugging_opts.mir_opt_level > 1 { + // Functions inlined during MIR transform can, at best, make it impossible to + // effectively cover inlined functions, and, at worst, break coverage map generation + // during LLVM codegen. For example, function counter IDs are only unique within a + // function. Inlining after these counters are injected can produce duplicate counters, + // resulting in an invalid coverage map (and ICE); so this option combination is not + // allowed. early_warn( error_format, &format!( - "`-Z mir-opt-level={}` (any level > 1) enables function inlining, which \ - limits the effectiveness of `-Z instrument-coverage`.", + "`-Z mir-opt-level={}` (or any level > 1) enables function inlining, which \ + is incompatible with `-Z instrument-coverage`. Inlining will be disabled.", debugging_opts.mir_opt_level, ), ); diff --git a/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.doctest.txt b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.doctest.txt index e1731c7223c5d..8f67170561a2a 100644 --- a/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.doctest.txt +++ b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.doctest.txt @@ -21,50 +21,86 @@ 20| |//! 21| |//! doctest returning a result: 22| 1|//! ``` - 23| 1|//! #[derive(Debug)] - 24| 1|//! struct SomeError; - 25| 1|//! let mut res = Err(SomeError); - 26| 1|//! if res.is_ok() { - 27| 0|//! res?; - 28| 1|//! } else { - 29| 1|//! res = Ok(0); - 30| 1|//! } - 31| |//! // need to be explicit because rustdoc cant infer the return type - 32| 1|//! Ok::<(), SomeError>(()) - 33| 1|//! ``` - 34| |//! - 35| |//! doctest with custom main: - 36| |//! ``` - 37| |//! #[derive(Debug)] - 38| |//! struct SomeError; - 39| |//! - 40| |//! extern crate doctest_crate; - 41| |//! - 42| 1|//! fn doctest_main() -> Result<(), SomeError> { - 43| 1|//! doctest_crate::fn_run_in_doctests(2); - 44| 1|//! Ok(()) - 45| 1|//! } - 46| |//! - 47| |//! // this `main` is not shown as covered, as it clashes with all the other - 48| |//! // `main` functions that were automatically generated for doctests - 49| |//! fn main() -> Result<(), SomeError> { - 50| |//! doctest_main() - 51| |//! } - 52| |//! ``` - 53| | - 54| |/// doctest attached to fn testing external code: - 55| |/// ``` - 56| 1|/// extern crate doctest_crate; - 57| 1|/// doctest_crate::fn_run_in_doctests(3); - 58| 1|/// ``` - 59| |/// - 60| 1|fn main() { - 61| 1| if true { - 62| 1| assert_eq!(1, 1); - 63| | } else { - 64| | assert_eq!(1, 2); - 65| | } - 66| 1|} + 23| 2|//! #[derive(Debug, PartialEq)] + ^1 + 24| 1|//! struct SomeError { + 25| 1|//! msg: String, + 26| 1|//! } + 27| 1|//! let mut res = Err(SomeError { msg: String::from("a message") }); + 28| 1|//! if res.is_ok() { + 29| 0|//! res?; + 30| |//! } else { + 31| 1|//! if *res.as_ref().unwrap_err() == *res.as_ref().unwrap_err() { + 32| 1|//! println!("{:?}", res); + 33| 1|//! } + ^0 + 34| 1|//! if *res.as_ref().unwrap_err() == *res.as_ref().unwrap_err() { + 35| 1|//! res = Ok(1); + 36| 1|//! } + ^0 + 37| 1|//! res = Ok(0); + 38| |//! } + 39| |//! // need to be explicit because rustdoc cant infer the return type + 40| 1|//! Ok::<(), SomeError>(()) + 41| 1|//! ``` + 42| |//! + 43| |//! doctest with custom main: + 44| |//! ``` + 45| 1|//! fn some_func() { + 46| 1|//! println!("called some_func()"); + 47| 1|//! } + 48| |//! + 49| |//! #[derive(Debug)] + 50| |//! struct SomeError; + 51| |//! + 52| |//! extern crate doctest_crate; + 53| |//! + 54| 1|//! fn doctest_main() -> Result<(), SomeError> { + 55| 1|//! some_func(); + 56| 1|//! doctest_crate::fn_run_in_doctests(2); + 57| 1|//! Ok(()) + 58| 1|//! } + 59| |//! + 60| |//! // this `main` is not shown as covered, as it clashes with all the other + 61| |//! // `main` functions that were automatically generated for doctests + 62| |//! fn main() -> Result<(), SomeError> { + 63| |//! doctest_main() + 64| |//! } + 65| |//! ``` + 66| | + 67| |/// doctest attached to fn testing external code: + 68| |/// ``` + 69| 1|/// extern crate doctest_crate; + 70| 1|/// doctest_crate::fn_run_in_doctests(3); + 71| 1|/// ``` + 72| |/// + 73| 1|fn main() { + 74| 1| if true { + 75| 1| assert_eq!(1, 1); + 76| | } else { + 77| | assert_eq!(1, 2); + 78| | } + 79| 1|} + 80| | + 81| |// FIXME(Swatinem): Fix known issue that coverage code region columns need to be offset by the + 82| |// doc comment line prefix (`///` or `//!`) and any additional indent (before or after the doc + 83| |// comment characters). This test produces `llvm-cov show` results demonstrating the problem. + 84| |// + 85| |// One of the above tests now includes: `derive(Debug, PartialEq)`, producing an `llvm-cov show` + 86| |// result with a distinct count for `Debug`, denoted by `^1`, but the caret points to the wrong + 87| |// column. Similarly, the `if` blocks without `else` blocks show `^0`, which should point at, or + 88| |// one character past, the `if` block's closing brace. In both cases, these are most likely off + 89| |// by the number of characters stripped from the beginning of each doc comment line: indent + 90| |// whitespace, if any, doc comment prefix (`//!` in this case) and (I assume) one space character + 91| |// (?). Note, when viewing `llvm-cov show` results in `--color` mode, the column offset errors are + 92| |// more pronounced, and show up in more places, with background color used to show some distinct + 93| |// code regions with different coverage counts. + 94| |// + 95| |// NOTE: Since the doc comment line prefix may vary, one possible solution is to replace each + 96| |// character stripped from the beginning of doc comment lines with a space. This will give coverage + 97| |// results the correct column offsets, and I think it should compile correctly, but I don't know + 98| |// what affect it might have on diagnostic messages from the compiler, and whether anyone would care + 99| |// if the indentation changed. I don't know if there is a more viable solution. ../coverage/lib/doctest_crate.rs: 1| |/// A function run only from within doctests diff --git a/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.doctest/doctest.main.-------.InstrumentCoverage.0.html b/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.doctest/doctest.main.-------.InstrumentCoverage.0.html index 8d074558aae20..333476a2df573 100644 --- a/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.doctest/doctest.main.-------.InstrumentCoverage.0.html +++ b/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.doctest/doctest.main.-------.InstrumentCoverage.0.html @@ -69,59 +69,59 @@ -
@0⦊fn main() ⦉@0{ - if @0⦊true⦉@0 { - @5⦊@4,6,7,8,9⦊assert_eq!(1, 1);⦉@4,6,7,8,9⦉@5 +
@0⦊fn main() ⦉@0{ + if @0⦊true⦉@0 { + @5⦊@4,6,7,8,9⦊assert_eq!(1, 1);⦉@4,6,7,8,9⦉@5 } else { - @11⦊@10,12,13,14,15⦊assert_eq!(1, 2);⦉@10,12,13,14,15⦉@11 + @11⦊@10,12,13,14,15⦊assert_eq!(1, 2);⦉@10,12,13,14,15⦉@11 } -}@16⦊‸⦉@16
+}@16⦊‸⦉@16
diff --git a/src/test/run-make-fulldeps/coverage/doctest.rs b/src/test/run-make-fulldeps/coverage/doctest.rs index e41d669bf0c76..ec04ea5706379 100644 --- a/src/test/run-make-fulldeps/coverage/doctest.rs +++ b/src/test/run-make-fulldeps/coverage/doctest.rs @@ -20,13 +20,21 @@ //! //! doctest returning a result: //! ``` -//! #[derive(Debug)] -//! struct SomeError; -//! let mut res = Err(SomeError); +//! #[derive(Debug, PartialEq)] +//! struct SomeError { +//! msg: String, +//! } +//! let mut res = Err(SomeError { msg: String::from("a message") }); //! if res.is_ok() { -//! res?; +//! res?; //! } else { -//! res = Ok(0); +//! if *res.as_ref().unwrap_err() == *res.as_ref().unwrap_err() { +//! println!("{:?}", res); +//! } +//! if *res.as_ref().unwrap_err() == *res.as_ref().unwrap_err() { +//! res = Ok(1); +//! } +//! res = Ok(0); //! } //! // need to be explicit because rustdoc cant infer the return type //! Ok::<(), SomeError>(()) @@ -34,12 +42,17 @@ //! //! doctest with custom main: //! ``` +//! fn some_func() { +//! println!("called some_func()"); +//! } +//! //! #[derive(Debug)] //! struct SomeError; //! //! extern crate doctest_crate; //! //! fn doctest_main() -> Result<(), SomeError> { +//! some_func(); //! doctest_crate::fn_run_in_doctests(2); //! Ok(()) //! } @@ -64,3 +77,23 @@ fn main() { assert_eq!(1, 2); } } + +// FIXME(Swatinem): Fix known issue that coverage code region columns need to be offset by the +// doc comment line prefix (`///` or `//!`) and any additional indent (before or after the doc +// comment characters). This test produces `llvm-cov show` results demonstrating the problem. +// +// One of the above tests now includes: `derive(Debug, PartialEq)`, producing an `llvm-cov show` +// result with a distinct count for `Debug`, denoted by `^1`, but the caret points to the wrong +// column. Similarly, the `if` blocks without `else` blocks show `^0`, which should point at, or +// one character past, the `if` block's closing brace. In both cases, these are most likely off +// by the number of characters stripped from the beginning of each doc comment line: indent +// whitespace, if any, doc comment prefix (`//!` in this case) and (I assume) one space character +// (?). Note, when viewing `llvm-cov show` results in `--color` mode, the column offset errors are +// more pronounced, and show up in more places, with background color used to show some distinct +// code regions with different coverage counts. +// +// NOTE: Since the doc comment line prefix may vary, one possible solution is to replace each +// character stripped from the beginning of doc comment lines with a space. This will give coverage +// results the correct column offsets, and I think it should compile correctly, but I don't know +// what affect it might have on diagnostic messages from the compiler, and whether anyone would care +// if the indentation changed. I don't know if there is a more viable solution. diff --git a/src/test/ui/mir/mir-inlining/inline-instrument-coverage-fail.rs b/src/test/ui/mir/mir-inlining/inline-instrument-coverage-fail.rs new file mode 100644 index 0000000000000..2437155d9810a --- /dev/null +++ b/src/test/ui/mir/mir-inlining/inline-instrument-coverage-fail.rs @@ -0,0 +1,21 @@ +// Ensures -Zmir-opt-level=2 (specifically, inlining) is not allowed with -Zinstrument-coverage. +// Regression test for issue #80060. +// +// needs-profiler-support +// build-pass +// compile-flags: -Zmir-opt-level=2 -Zinstrument-coverage +#[inline(never)] +fn foo() {} + +pub fn baz() { + bar(); +} + +#[inline(always)] +fn bar() { + foo(); +} + +fn main() { + bar(); +} diff --git a/src/test/ui/mir/mir-inlining/inline-instrument-coverage-fail.stderr b/src/test/ui/mir/mir-inlining/inline-instrument-coverage-fail.stderr new file mode 100644 index 0000000000000..eb50e5075ca8f --- /dev/null +++ b/src/test/ui/mir/mir-inlining/inline-instrument-coverage-fail.stderr @@ -0,0 +1,2 @@ +warning: `-Z mir-opt-level=2` (or any level > 1) enables function inlining, which is incompatible with `-Z instrument-coverage`. Inlining will be disabled. +