Skip to content

Commit

Permalink
Inlining enabled by -mir-opt-level > 1 is incompatible with coverage
Browse files Browse the repository at this point in the history 
Fixes: #80060

Also adds additional test cases for coverage of doctests.
  • Loading branch information
@richkadel
richkadel committed Jan 4, 2021
1 parent bbcaed0 commit e4aa99f
Show file tree
Hide file tree
Showing 7 changed files with 210 additions and 103 deletions.
9 changes: 9 additions & 0 deletions compiler/rustc_mir/src/transform/inline.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
10 changes: 8 additions & 2 deletions compiler/rustc_session/src/config.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
),
);
Expand Down
124 changes: 80 additions & 44 deletions src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.doctest.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
104 changes: 52 additions & 52 deletions ...overage-spanview/expected_mir_dump.doctest/doctest.main.-------.InstrumentCoverage.0.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,59 +69,59 @@
</style>
</head>
<body>
<div class="code" style="counter-reset: line 59"><span class="line"><span><span class="code even" style="--layer: 1"><span class="annotation">@0⦊</span>fn main() <span class="annotation">⦉@0</span></span></span><span class="code" style="--layer: 0">{</span></span>
<span class="line"><span class="code" style="--layer: 0"> if </span><span><span class="code even" style="--layer: 1" title="61:8-61:12: @0[1]: _1 = const true
61:8-61:12: @0[2]: FakeRead(ForMatchedPlace, _1)"><span class="annotation">@0⦊</span>true<span class="annotation">⦉@0</span></span></span><span class="code" style="--layer: 0"> {</span></span>
<span class="line"><span class="code" style="--layer: 0"> </span><span><span class="code odd" style="--layer: 1" title="62:9-62:26: @5[0]: _2 = const ()"><span class="annotation">@5⦊</span></span></span><span class="code even" style="--layer: 2" title="62:9-62:26: @6[5]: _75 = const main::promoted[3]
62:9-62:26: @6[6]: _18 = &amp;(*_75)
62:9-62:26: @6[7]: _17 = &amp;(*_18)
62:9-62:26: @6[8]: _16 = move _17 as &amp;[&amp;str] (Pointer(Unsize))
62:9-62:26: @6[17]: _26 = &amp;(*_8)
62:9-62:26: @6[18]: _25 = &amp;_26
62:9-62:26: @6[21]: _28 = &amp;(*_9)
62:9-62:26: @6[22]: _27 = &amp;_28
62:9-62:26: @6[23]: _24 = (move _25, move _27)
62:9-62:26: @6[26]: FakeRead(ForMatchedPlace, _24)
62:9-62:26: @6[28]: _29 = (_24.0: &amp;&amp;i32)
62:9-62:26: @6[30]: _30 = (_24.1: &amp;&amp;i32)
62:9-62:26: @6[33]: _32 = &amp;(*_29)
62:9-62:26: @6[35]: _33 = &lt;&amp;i32 as Debug&gt;::fmt as for&lt;&#39;r, &#39;s, &#39;t0&gt; fn(&amp;&#39;r &amp;i32, &amp;&#39;s mut std::fmt::Formatter&lt;&#39;t0&gt;) -&gt; std::result::Result&lt;(), std::fmt::Error&gt; (Pointer(ReifyFnPointer))
62:9-62:26: @6.Call: _31 = ArgumentV1::new::&lt;&amp;i32&gt;(move _32, move _33) -&gt; [return: bb7, unwind: bb17]
62:9-62:26: @7[4]: _35 = &amp;(*_30)
62:9-62:26: @7[6]: _36 = &lt;&amp;i32 as Debug&gt;::fmt as for&lt;&#39;r, &#39;s, &#39;t0&gt; fn(&amp;&#39;r &amp;i32, &amp;&#39;s mut std::fmt::Formatter&lt;&#39;t0&gt;) -&gt; std::result::Result&lt;(), std::fmt::Error&gt; (Pointer(ReifyFnPointer))
62:9-62:26: @7.Call: _34 = ArgumentV1::new::&lt;&amp;i32&gt;(move _35, move _36) -&gt; [return: bb8, unwind: bb17]
62:9-62:26: @8[2]: _23 = [move _31, move _34]
62:9-62:26: @8[7]: _22 = &amp;_23
62:9-62:26: @8[8]: _21 = &amp;(*_22)
62:9-62:26: @8[9]: _20 = move _21 as &amp;[std::fmt::ArgumentV1] (Pointer(Unsize))
62:9-62:26: @8.Call: _15 = Arguments::new_v1(move _16, move _20) -&gt; [return: bb9, unwind: bb17]
62:9-62:26: @9.Call: core::panicking::panic_fmt(move _15) -&gt; bb17"><span class="annotation">@4,6,7,8,9⦊</span>assert_eq!(1, 1);<span class="annotation">⦉@4,6,7,8,9</span></span><span><span class="code odd" style="--layer: 1" title="62:9-62:26: @5[0]: _2 = const ()"><span class="annotation">⦉@5</span></span></span><span class="code" style="--layer: 0"></span></span>
<div class="code" style="counter-reset: line 72"><span class="line"><span><span class="code even" style="--layer: 1"><span class="annotation">@0⦊</span>fn main() <span class="annotation">⦉@0</span></span></span><span class="code" style="--layer: 0">{</span></span>
<span class="line"><span class="code" style="--layer: 0"> if </span><span><span class="code even" style="--layer: 1" title="74:8-74:12: @0[1]: _1 = const true
74:8-74:12: @0[2]: FakeRead(ForMatchedPlace, _1)"><span class="annotation">@0⦊</span>true<span class="annotation">⦉@0</span></span></span><span class="code" style="--layer: 0"> {</span></span>
<span class="line"><span class="code" style="--layer: 0"> </span><span><span class="code odd" style="--layer: 1" title="75:9-75:26: @5[0]: _2 = const ()"><span class="annotation">@5⦊</span></span></span><span class="code even" style="--layer: 2" title="75:9-75:26: @6[5]: _75 = const main::promoted[3]
75:9-75:26: @6[6]: _18 = &amp;(*_75)
75:9-75:26: @6[7]: _17 = &amp;(*_18)
75:9-75:26: @6[8]: _16 = move _17 as &amp;[&amp;str] (Pointer(Unsize))
75:9-75:26: @6[17]: _26 = &amp;(*_8)
75:9-75:26: @6[18]: _25 = &amp;_26
75:9-75:26: @6[21]: _28 = &amp;(*_9)
75:9-75:26: @6[22]: _27 = &amp;_28
75:9-75:26: @6[23]: _24 = (move _25, move _27)
75:9-75:26: @6[26]: FakeRead(ForMatchedPlace, _24)
75:9-75:26: @6[28]: _29 = (_24.0: &amp;&amp;i32)
75:9-75:26: @6[30]: _30 = (_24.1: &amp;&amp;i32)
75:9-75:26: @6[33]: _32 = &amp;(*_29)
75:9-75:26: @6[35]: _33 = &lt;&amp;i32 as Debug&gt;::fmt as for&lt;&#39;r, &#39;s, &#39;t0&gt; fn(&amp;&#39;r &amp;i32, &amp;&#39;s mut std::fmt::Formatter&lt;&#39;t0&gt;) -&gt; std::result::Result&lt;(), std::fmt::Error&gt; (Pointer(ReifyFnPointer))
75:9-75:26: @6.Call: _31 = ArgumentV1::new::&lt;&amp;i32&gt;(move _32, move _33) -&gt; [return: bb7, unwind: bb17]
75:9-75:26: @7[4]: _35 = &amp;(*_30)
75:9-75:26: @7[6]: _36 = &lt;&amp;i32 as Debug&gt;::fmt as for&lt;&#39;r, &#39;s, &#39;t0&gt; fn(&amp;&#39;r &amp;i32, &amp;&#39;s mut std::fmt::Formatter&lt;&#39;t0&gt;) -&gt; std::result::Result&lt;(), std::fmt::Error&gt; (Pointer(ReifyFnPointer))
75:9-75:26: @7.Call: _34 = ArgumentV1::new::&lt;&amp;i32&gt;(move _35, move _36) -&gt; [return: bb8, unwind: bb17]
75:9-75:26: @8[2]: _23 = [move _31, move _34]
75:9-75:26: @8[7]: _22 = &amp;_23
75:9-75:26: @8[8]: _21 = &amp;(*_22)
75:9-75:26: @8[9]: _20 = move _21 as &amp;[std::fmt::ArgumentV1] (Pointer(Unsize))
75:9-75:26: @8.Call: _15 = Arguments::new_v1(move _16, move _20) -&gt; [return: bb9, unwind: bb17]
75:9-75:26: @9.Call: core::panicking::panic_fmt(move _15) -&gt; bb17"><span class="annotation">@4,6,7,8,9⦊</span>assert_eq!(1, 1);<span class="annotation">⦉@4,6,7,8,9</span></span><span><span class="code odd" style="--layer: 1" title="75:9-75:26: @5[0]: _2 = const ()"><span class="annotation">⦉@5</span></span></span><span class="code" style="--layer: 0"></span></span>
<span class="line"><span class="code" style="--layer: 0"> } else {</span></span>
<span class="line"><span class="code" style="--layer: 0"> </span><span><span class="code even" style="--layer: 1" title="64:9-64:26: @11[0]: _37 = const ()"><span class="annotation">@11⦊</span></span></span><span class="code even" style="--layer: 2" title="64:9-64:26: @12[5]: _72 = const main::promoted[0]
64:9-64:26: @12[6]: _53 = &amp;(*_72)
64:9-64:26: @12[7]: _52 = &amp;(*_53)
64:9-64:26: @12[8]: _51 = move _52 as &amp;[&amp;str] (Pointer(Unsize))
64:9-64:26: @12[17]: _61 = &amp;(*_43)
64:9-64:26: @12[18]: _60 = &amp;_61
64:9-64:26: @12[21]: _63 = &amp;(*_44)
64:9-64:26: @12[22]: _62 = &amp;_63
64:9-64:26: @12[23]: _59 = (move _60, move _62)
64:9-64:26: @12[26]: FakeRead(ForMatchedPlace, _59)
64:9-64:26: @12[28]: _64 = (_59.0: &amp;&amp;i32)
64:9-64:26: @12[30]: _65 = (_59.1: &amp;&amp;i32)
64:9-64:26: @12[33]: _67 = &amp;(*_64)
64:9-64:26: @12[35]: _68 = &lt;&amp;i32 as Debug&gt;::fmt as for&lt;&#39;r, &#39;s, &#39;t0&gt; fn(&amp;&#39;r &amp;i32, &amp;&#39;s mut std::fmt::Formatter&lt;&#39;t0&gt;) -&gt; std::result::Result&lt;(), std::fmt::Error&gt; (Pointer(ReifyFnPointer))
64:9-64:26: @12.Call: _66 = ArgumentV1::new::&lt;&amp;i32&gt;(move _67, move _68) -&gt; [return: bb13, unwind: bb17]
64:9-64:26: @13[4]: _70 = &amp;(*_65)
64:9-64:26: @13[6]: _71 = &lt;&amp;i32 as Debug&gt;::fmt as for&lt;&#39;r, &#39;s, &#39;t0&gt; fn(&amp;&#39;r &amp;i32, &amp;&#39;s mut std::fmt::Formatter&lt;&#39;t0&gt;) -&gt; std::result::Result&lt;(), std::fmt::Error&gt; (Pointer(ReifyFnPointer))
64:9-64:26: @13.Call: _69 = ArgumentV1::new::&lt;&amp;i32&gt;(move _70, move _71) -&gt; [return: bb14, unwind: bb17]
64:9-64:26: @14[2]: _58 = [move _66, move _69]
64:9-64:26: @14[7]: _57 = &amp;_58
64:9-64:26: @14[8]: _56 = &amp;(*_57)
64:9-64:26: @14[9]: _55 = move _56 as &amp;[std::fmt::ArgumentV1] (Pointer(Unsize))
64:9-64:26: @14.Call: _50 = Arguments::new_v1(move _51, move _55) -&gt; [return: bb15, unwind: bb17]
64:9-64:26: @15.Call: core::panicking::panic_fmt(move _50) -&gt; bb17"><span class="annotation">@10,12,13,14,15⦊</span>assert_eq!(1, 2);<span class="annotation">⦉@10,12,13,14,15</span></span><span><span class="code even" style="--layer: 1" title="64:9-64:26: @11[0]: _37 = const ()"><span class="annotation">⦉@11</span></span></span><span class="code" style="--layer: 0"></span></span>
<span class="line"><span class="code" style="--layer: 0"> </span><span><span class="code even" style="--layer: 1" title="77:9-77:26: @11[0]: _37 = const ()"><span class="annotation">@11⦊</span></span></span><span class="code even" style="--layer: 2" title="77:9-77:26: @12[5]: _72 = const main::promoted[0]
77:9-77:26: @12[6]: _53 = &amp;(*_72)
77:9-77:26: @12[7]: _52 = &amp;(*_53)
77:9-77:26: @12[8]: _51 = move _52 as &amp;[&amp;str] (Pointer(Unsize))
77:9-77:26: @12[17]: _61 = &amp;(*_43)
77:9-77:26: @12[18]: _60 = &amp;_61
77:9-77:26: @12[21]: _63 = &amp;(*_44)
77:9-77:26: @12[22]: _62 = &amp;_63
77:9-77:26: @12[23]: _59 = (move _60, move _62)
77:9-77:26: @12[26]: FakeRead(ForMatchedPlace, _59)
77:9-77:26: @12[28]: _64 = (_59.0: &amp;&amp;i32)
77:9-77:26: @12[30]: _65 = (_59.1: &amp;&amp;i32)
77:9-77:26: @12[33]: _67 = &amp;(*_64)
77:9-77:26: @12[35]: _68 = &lt;&amp;i32 as Debug&gt;::fmt as for&lt;&#39;r, &#39;s, &#39;t0&gt; fn(&amp;&#39;r &amp;i32, &amp;&#39;s mut std::fmt::Formatter&lt;&#39;t0&gt;) -&gt; std::result::Result&lt;(), std::fmt::Error&gt; (Pointer(ReifyFnPointer))
77:9-77:26: @12.Call: _66 = ArgumentV1::new::&lt;&amp;i32&gt;(move _67, move _68) -&gt; [return: bb13, unwind: bb17]
77:9-77:26: @13[4]: _70 = &amp;(*_65)
77:9-77:26: @13[6]: _71 = &lt;&amp;i32 as Debug&gt;::fmt as for&lt;&#39;r, &#39;s, &#39;t0&gt; fn(&amp;&#39;r &amp;i32, &amp;&#39;s mut std::fmt::Formatter&lt;&#39;t0&gt;) -&gt; std::result::Result&lt;(), std::fmt::Error&gt; (Pointer(ReifyFnPointer))
77:9-77:26: @13.Call: _69 = ArgumentV1::new::&lt;&amp;i32&gt;(move _70, move _71) -&gt; [return: bb14, unwind: bb17]
77:9-77:26: @14[2]: _58 = [move _66, move _69]
77:9-77:26: @14[7]: _57 = &amp;_58
77:9-77:26: @14[8]: _56 = &amp;(*_57)
77:9-77:26: @14[9]: _55 = move _56 as &amp;[std::fmt::ArgumentV1] (Pointer(Unsize))
77:9-77:26: @14.Call: _50 = Arguments::new_v1(move _51, move _55) -&gt; [return: bb15, unwind: bb17]
77:9-77:26: @15.Call: core::panicking::panic_fmt(move _50) -&gt; bb17"><span class="annotation">@10,12,13,14,15⦊</span>assert_eq!(1, 2);<span class="annotation">⦉@10,12,13,14,15</span></span><span><span class="code even" style="--layer: 1" title="77:9-77:26: @11[0]: _37 = const ()"><span class="annotation">⦉@11</span></span></span><span class="code" style="--layer: 0"></span></span>
<span class="line"><span class="code" style="--layer: 0"> }</span></span>
<span class="line"><span class="code" style="--layer: 0">}</span><span><span class="code odd" style="--layer: 1" title="66:2-66:2: @16.Return: return"><span class="annotation">@16⦊</span>‸<span class="annotation">⦉@16</span></span></span></span></div>
<span class="line"><span class="code" style="--layer: 0">}</span><span><span class="code odd" style="--layer: 1" title="79:2-79:2: @16.Return: return"><span class="annotation">@16⦊</span>‸<span class="annotation">⦉@16</span></span></span></span></div>
</body>
</html>
Loading

0 comments on commit e4aa99f

Please sign in to comment.