Skip to content

Commit

Permalink
Rollup merge of rust-lang#120183 - Zalathar:test-closure, r=compiler-…
Browse files Browse the repository at this point in the history
…errors

Add `#[coverage(off)]` to closures introduced by `#[test]` and `#[bench]`

These closures are an internal implementation detail of the `#[test]` and `#[bench]` attribute macros, so from a user perspective there is no reason to instrument them for coverage.

Skipping them makes coverage reports slightly cleaner, and will also allow other changes to span processing during coverage instrumentation, without having to worry about how they affect the `#[test]` macro.

The `#[coverage(off)]` attribute has no effect when `-Cinstrument-coverage` is not used.

Fixes rust-lang#120046.

---

Note that this PR has no effect on the user-written function that has the `#[test]` attribute attached to it. That function will still be instrumented as normal.
  • Loading branch information
matthiaskrgr authored Jan 22, 2024
2 parents be7dc79 + 6d7e80c commit f9fe759
Show file tree
Hide file tree
Showing 9 changed files with 52 additions and 18 deletions.
1 change: 1 addition & 0 deletions compiler/rustc_builtin_macros/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#![doc(rust_logo)]
#![doc(html_root_url = "https://doc.rust-lang.org/nightly/nightly-rustc/")]
#![feature(array_windows)]
#![feature(assert_matches)]
#![feature(box_patterns)]
#![feature(decl_macro)]
#![feature(if_let_guard)]
Expand Down
21 changes: 17 additions & 4 deletions compiler/rustc_builtin_macros/src/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use rustc_errors::{Applicability, DiagnosticBuilder, Level};
use rustc_expand::base::*;
use rustc_span::symbol::{sym, Ident, Symbol};
use rustc_span::{ErrorGuaranteed, FileNameDisplayPreference, Span};
use std::assert_matches::assert_matches;
use std::iter;
use thin_vec::{thin_vec, ThinVec};

Expand Down Expand Up @@ -182,6 +183,16 @@ pub fn expand_test_or_bench(
// creates $name: $expr
let field = |name, expr| cx.field_imm(sp, Ident::from_str_and_span(name, sp), expr);

// Adds `#[coverage(off)]` to a closure, so it won't be instrumented in
// `-Cinstrument-coverage` builds.
// This requires `#[allow_internal_unstable(coverage_attribute)]` on the
// corresponding macro declaration in `core::macros`.
let coverage_off = |mut expr: P<ast::Expr>| {
assert_matches!(expr.kind, ast::ExprKind::Closure(_));
expr.attrs.push(cx.attr_nested_word(sym::coverage, sym::off, sp));
expr
};

let test_fn = if is_bench {
// A simple ident for a lambda
let b = Ident::from_str_and_span("b", attr_sp);
Expand All @@ -190,8 +201,9 @@ pub fn expand_test_or_bench(
sp,
cx.expr_path(test_path("StaticBenchFn")),
thin_vec![
// #[coverage(off)]
// |b| self::test::assert_test_result(
cx.lambda1(
coverage_off(cx.lambda1(
sp,
cx.expr_call(
sp,
Expand All @@ -206,16 +218,17 @@ pub fn expand_test_or_bench(
],
),
b,
), // )
)), // )
],
)
} else {
cx.expr_call(
sp,
cx.expr_path(test_path("StaticTestFn")),
thin_vec![
// #[coverage(off)]
// || {
cx.lambda0(
coverage_off(cx.lambda0(
sp,
// test::assert_test_result(
cx.expr_call(
Expand All @@ -230,7 +243,7 @@ pub fn expand_test_or_bench(
), // )
],
), // }
), // )
)), // )
],
)
};
Expand Down
4 changes: 2 additions & 2 deletions library/core/src/macros/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1596,7 +1596,7 @@ pub(crate) mod builtin {
///
/// [the reference]: ../../../reference/attributes/testing.html#the-test-attribute
#[stable(feature = "rust1", since = "1.0.0")]
#[allow_internal_unstable(test, rustc_attrs)]
#[allow_internal_unstable(test, rustc_attrs, coverage_attribute)]
#[rustc_builtin_macro]
pub macro test($item:item) {
/* compiler built-in */
Expand All @@ -1609,7 +1609,7 @@ pub(crate) mod builtin {
soft,
reason = "`bench` is a part of custom test frameworks which are unstable"
)]
#[allow_internal_unstable(test, rustc_attrs)]
#[allow_internal_unstable(test, rustc_attrs, coverage_attribute)]
#[rustc_builtin_macro]
pub macro bench($item:item) {
/* compiler built-in */
Expand Down
8 changes: 8 additions & 0 deletions tests/coverage/bench.cov-map
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
Function name: bench::my_bench
Raw bytes (9): 0x[01, 01, 00, 01, 01, 08, 01, 00, 27]
Number of files: 1
- file 0 => global file 1
Number of expressions: 0
Number of file 0 mappings: 1
- Code(Counter(0)) at (prev + 8, 1) to (start + 0, 39)

9 changes: 9 additions & 0 deletions tests/coverage/bench.coverage
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
LL| |#![feature(test)]
LL| |// edition: 2021
LL| |// compile-flags: --test
LL| |
LL| |extern crate test;
LL| |
LL| |#[bench]
LL| 1|fn my_bench(_b: &mut test::Bencher) {}

8 changes: 8 additions & 0 deletions tests/coverage/bench.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
#![feature(test)]
// edition: 2021
// compile-flags: --test

extern crate test;

#[bench]
fn my_bench(_b: &mut test::Bencher) {}
8 changes: 0 additions & 8 deletions tests/coverage/test_harness.cov-map
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,6 @@ Number of expressions: 0
Number of file 0 mappings: 1
- Code(Counter(0)) at (prev + 10, 1) to (start + 0, 16)

Function name: test_harness::my_test::{closure#0}
Raw bytes (9): 0x[01, 01, 00, 01, 01, 09, 01, 00, 08]
Number of files: 1
- file 0 => global file 1
Number of expressions: 0
Number of file 0 mappings: 1
- Code(Counter(0)) at (prev + 9, 1) to (start + 0, 8)

Function name: test_harness::unused (unused)
Raw bytes (9): 0x[01, 01, 00, 01, 00, 07, 01, 00, 0f]
Number of files: 1
Expand Down
2 changes: 1 addition & 1 deletion tests/coverage/test_harness.coverage
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,6 @@
LL| |#[allow(dead_code)]
LL| 0|fn unused() {}
LL| |
LL| 1|#[test]
LL| |#[test]
LL| 1|fn my_test() {}

9 changes: 6 additions & 3 deletions tests/pretty/tests-are-sorted.pp
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@
should_panic: test::ShouldPanic::No,
test_type: test::TestType::Unknown,
},
testfn: test::StaticTestFn(|| test::assert_test_result(m_test())),
testfn: test::StaticTestFn(#[coverage(off)] ||
test::assert_test_result(m_test())),
};
fn m_test() {}

Expand All @@ -51,7 +52,8 @@
should_panic: test::ShouldPanic::No,
test_type: test::TestType::Unknown,
},
testfn: test::StaticTestFn(|| test::assert_test_result(z_test())),
testfn: test::StaticTestFn(#[coverage(off)] ||
test::assert_test_result(z_test())),
};
#[ignore = "not yet implemented"]
fn z_test() {}
Expand All @@ -75,7 +77,8 @@
should_panic: test::ShouldPanic::No,
test_type: test::TestType::Unknown,
},
testfn: test::StaticTestFn(|| test::assert_test_result(a_test())),
testfn: test::StaticTestFn(#[coverage(off)] ||
test::assert_test_result(a_test())),
};
fn a_test() {}
#[rustc_main]
Expand Down

0 comments on commit f9fe759

Please sign in to comment.