Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

MIR Inline is incompatible with coverage #80521

Merged
merged 1 commit into from
Jan 7, 2021
Merged

MIR Inline is incompatible with coverage #80521

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions compiler/rustc_mir/src/transform/inline.rs
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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)]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Swatinem - just a heads up, I'm planning to submit a minor change for coverage here, but I'm also adding some changes and a FIXME comment at the bottom of doctest.rs describing and showing the column offset issue. I suggested one possible solution in the comment.

^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
Open in desktop
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