From 4f7a47798eb3455aed74fde5cd8e81927d2db07a Mon Sep 17 00:00:00 2001 From: Zalathar Date: Wed, 17 Apr 2024 11:41:40 +1000 Subject: [PATCH 1/8] coverage: Branch coverage test for let-else --- tests/coverage/branch/let-else.cov-map | 18 ++++++++++++ tests/coverage/branch/let-else.coverage | 37 +++++++++++++++++++++++++ tests/coverage/branch/let-else.rs | 35 +++++++++++++++++++++++ 3 files changed, 90 insertions(+) create mode 100644 tests/coverage/branch/let-else.cov-map create mode 100644 tests/coverage/branch/let-else.coverage create mode 100644 tests/coverage/branch/let-else.rs diff --git a/tests/coverage/branch/let-else.cov-map b/tests/coverage/branch/let-else.cov-map new file mode 100644 index 0000000000000..ad987bd6bb1e9 --- /dev/null +++ b/tests/coverage/branch/let-else.cov-map @@ -0,0 +1,18 @@ +Function name: let_else::let_else +Raw bytes (38): 0x[01, 01, 02, 05, 09, 09, 02, 06, 01, 0c, 01, 01, 10, 02, 03, 0e, 00, 0f, 05, 00, 13, 00, 18, 09, 01, 09, 01, 0f, 02, 04, 05, 00, 0b, 07, 01, 01, 00, 02] +Number of files: 1 +- file 0 => global file 1 +Number of expressions: 2 +- expression 0 operands: lhs = Counter(1), rhs = Counter(2) +- expression 1 operands: lhs = Counter(2), rhs = Expression(0, Sub) +Number of file 0 mappings: 6 +- Code(Counter(0)) at (prev + 12, 1) to (start + 1, 16) +- Code(Expression(0, Sub)) at (prev + 3, 14) to (start + 0, 15) + = (c1 - c2) +- Code(Counter(1)) at (prev + 0, 19) to (start + 0, 24) +- Code(Counter(2)) at (prev + 1, 9) to (start + 1, 15) +- Code(Expression(0, Sub)) at (prev + 4, 5) to (start + 0, 11) + = (c1 - c2) +- Code(Expression(1, Add)) at (prev + 1, 1) to (start + 0, 2) + = (c2 + (c1 - c2)) + diff --git a/tests/coverage/branch/let-else.coverage b/tests/coverage/branch/let-else.coverage new file mode 100644 index 0000000000000..83730e1dfbafe --- /dev/null +++ b/tests/coverage/branch/let-else.coverage @@ -0,0 +1,37 @@ + LL| |#![feature(coverage_attribute)] + LL| |//@ edition: 2021 + LL| |//@ compile-flags: -Zcoverage-options=branch + LL| |//@ llvm-cov-flags: --show-branches=count + LL| | + LL| |macro_rules! no_merge { + LL| | () => { + LL| | for _ in 0..1 {} + LL| | }; + LL| |} + LL| | + LL| 3|fn let_else(value: Option<&str>) { + LL| 3| no_merge!(); + LL| | + LL| 3| let Some(x) = value else { + ^2 + LL| 1| say("none"); + LL| 1| return; + LL| | }; + LL| | + LL| 2| say(x); + LL| 3|} + LL| | + LL| |#[coverage(off)] + LL| |fn say(message: &str) { + LL| | core::hint::black_box(message); + LL| |} + LL| | + LL| |#[coverage(off)] + LL| |fn main() { + LL| | let_else(Some("x")); + LL| | let_else(Some("x")); + LL| | let_else(None); + LL| |} + LL| | + LL| |// FIXME(#124118) Actually instrument let-else for branch coverage. + diff --git a/tests/coverage/branch/let-else.rs b/tests/coverage/branch/let-else.rs new file mode 100644 index 0000000000000..af0665d8241eb --- /dev/null +++ b/tests/coverage/branch/let-else.rs @@ -0,0 +1,35 @@ +#![feature(coverage_attribute)] +//@ edition: 2021 +//@ compile-flags: -Zcoverage-options=branch +//@ llvm-cov-flags: --show-branches=count + +macro_rules! no_merge { + () => { + for _ in 0..1 {} + }; +} + +fn let_else(value: Option<&str>) { + no_merge!(); + + let Some(x) = value else { + say("none"); + return; + }; + + say(x); +} + +#[coverage(off)] +fn say(message: &str) { + core::hint::black_box(message); +} + +#[coverage(off)] +fn main() { + let_else(Some("x")); + let_else(Some("x")); + let_else(None); +} + +// FIXME(#124118) Actually instrument let-else for branch coverage. From 7f432dfb23f264f5c368464f849663a518750a93 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Wed, 17 Apr 2024 11:41:40 +1000 Subject: [PATCH 2/8] coverage: Branch coverage test for if-let and let-chains --- tests/coverage/branch/if-let.cov-map | 41 ++++++++++++++++++ tests/coverage/branch/if-let.coverage | 62 +++++++++++++++++++++++++++ tests/coverage/branch/if-let.rs | 58 +++++++++++++++++++++++++ 3 files changed, 161 insertions(+) create mode 100644 tests/coverage/branch/if-let.cov-map create mode 100644 tests/coverage/branch/if-let.coverage create mode 100644 tests/coverage/branch/if-let.rs diff --git a/tests/coverage/branch/if-let.cov-map b/tests/coverage/branch/if-let.cov-map new file mode 100644 index 0000000000000..c12df8d98018d --- /dev/null +++ b/tests/coverage/branch/if-let.cov-map @@ -0,0 +1,41 @@ +Function name: if_let::if_let +Raw bytes (38): 0x[01, 01, 02, 05, 09, 09, 02, 06, 01, 0c, 01, 01, 10, 02, 03, 11, 00, 12, 05, 00, 16, 00, 1b, 02, 00, 1c, 02, 06, 09, 02, 0c, 02, 06, 07, 03, 05, 01, 02] +Number of files: 1 +- file 0 => global file 1 +Number of expressions: 2 +- expression 0 operands: lhs = Counter(1), rhs = Counter(2) +- expression 1 operands: lhs = Counter(2), rhs = Expression(0, Sub) +Number of file 0 mappings: 6 +- Code(Counter(0)) at (prev + 12, 1) to (start + 1, 16) +- Code(Expression(0, Sub)) at (prev + 3, 17) to (start + 0, 18) + = (c1 - c2) +- Code(Counter(1)) at (prev + 0, 22) to (start + 0, 27) +- Code(Expression(0, Sub)) at (prev + 0, 28) to (start + 2, 6) + = (c1 - c2) +- Code(Counter(2)) at (prev + 2, 12) to (start + 2, 6) +- Code(Expression(1, Add)) at (prev + 3, 5) to (start + 1, 2) + = (c2 + (c1 - c2)) + +Function name: if_let::if_let_chain +Raw bytes (52): 0x[01, 01, 04, 01, 05, 05, 09, 0f, 0d, 05, 09, 08, 01, 17, 01, 00, 33, 02, 01, 11, 00, 12, 01, 00, 16, 00, 17, 0d, 01, 15, 00, 16, 02, 00, 1a, 00, 1b, 0d, 01, 05, 03, 06, 0f, 03, 0c, 02, 06, 0b, 03, 05, 01, 02] +Number of files: 1 +- file 0 => global file 1 +Number of expressions: 4 +- expression 0 operands: lhs = Counter(0), rhs = Counter(1) +- expression 1 operands: lhs = Counter(1), rhs = Counter(2) +- expression 2 operands: lhs = Expression(3, Add), rhs = Counter(3) +- expression 3 operands: lhs = Counter(1), rhs = Counter(2) +Number of file 0 mappings: 8 +- Code(Counter(0)) at (prev + 23, 1) to (start + 0, 51) +- Code(Expression(0, Sub)) at (prev + 1, 17) to (start + 0, 18) + = (c0 - c1) +- Code(Counter(0)) at (prev + 0, 22) to (start + 0, 23) +- Code(Counter(3)) at (prev + 1, 21) to (start + 0, 22) +- Code(Expression(0, Sub)) at (prev + 0, 26) to (start + 0, 27) + = (c0 - c1) +- Code(Counter(3)) at (prev + 1, 5) to (start + 3, 6) +- Code(Expression(3, Add)) at (prev + 3, 12) to (start + 2, 6) + = (c1 + c2) +- Code(Expression(2, Add)) at (prev + 3, 5) to (start + 1, 2) + = ((c1 + c2) + c3) + diff --git a/tests/coverage/branch/if-let.coverage b/tests/coverage/branch/if-let.coverage new file mode 100644 index 0000000000000..f30c5d34eca17 --- /dev/null +++ b/tests/coverage/branch/if-let.coverage @@ -0,0 +1,62 @@ + LL| |#![feature(coverage_attribute, let_chains)] + LL| |//@ edition: 2021 + LL| |//@ compile-flags: -Zcoverage-options=branch + LL| |//@ llvm-cov-flags: --show-branches=count + LL| | + LL| |macro_rules! no_merge { + LL| | () => { + LL| | for _ in 0..1 {} + LL| | }; + LL| |} + LL| | + LL| 3|fn if_let(input: Option<&str>) { + LL| 3| no_merge!(); + LL| | + LL| 3| if let Some(x) = input { + ^2 + LL| 2| say(x); + LL| 2| } else { + LL| 1| say("none"); + LL| 1| } + LL| 3| say("done"); + LL| 3|} + LL| | + LL| 15|fn if_let_chain(a: Option<&str>, b: Option<&str>) { + LL| 15| if let Some(x) = a + ^12 + LL| 12| && let Some(y) = b + ^8 + LL| 8| { + LL| 8| say(x); + LL| 8| say(y); + LL| 8| } else { + LL| 7| say("not both"); + LL| 7| } + LL| 15| say("done"); + LL| 15|} + LL| | + LL| |#[coverage(off)] + LL| |fn say(message: &str) { + LL| | core::hint::black_box(message); + LL| |} + LL| | + LL| |#[coverage(off)] + LL| |fn main() { + LL| | if_let(Some("x")); + LL| | if_let(Some("x")); + LL| | if_let(None); + LL| | + LL| | for _ in 0..8 { + LL| | if_let_chain(Some("a"), Some("b")); + LL| | } + LL| | for _ in 0..4 { + LL| | if_let_chain(Some("a"), None); + LL| | } + LL| | for _ in 0..2 { + LL| | if_let_chain(None, Some("b")); + LL| | } + LL| | if_let_chain(None, None); + LL| |} + LL| | + LL| |// FIXME(#124118) Actually instrument if-let and let-chains for branch coverage. + diff --git a/tests/coverage/branch/if-let.rs b/tests/coverage/branch/if-let.rs new file mode 100644 index 0000000000000..13db00a82b126 --- /dev/null +++ b/tests/coverage/branch/if-let.rs @@ -0,0 +1,58 @@ +#![feature(coverage_attribute, let_chains)] +//@ edition: 2021 +//@ compile-flags: -Zcoverage-options=branch +//@ llvm-cov-flags: --show-branches=count + +macro_rules! no_merge { + () => { + for _ in 0..1 {} + }; +} + +fn if_let(input: Option<&str>) { + no_merge!(); + + if let Some(x) = input { + say(x); + } else { + say("none"); + } + say("done"); +} + +fn if_let_chain(a: Option<&str>, b: Option<&str>) { + if let Some(x) = a + && let Some(y) = b + { + say(x); + say(y); + } else { + say("not both"); + } + say("done"); +} + +#[coverage(off)] +fn say(message: &str) { + core::hint::black_box(message); +} + +#[coverage(off)] +fn main() { + if_let(Some("x")); + if_let(Some("x")); + if_let(None); + + for _ in 0..8 { + if_let_chain(Some("a"), Some("b")); + } + for _ in 0..4 { + if_let_chain(Some("a"), None); + } + for _ in 0..2 { + if_let_chain(None, Some("b")); + } + if_let_chain(None, None); +} + +// FIXME(#124118) Actually instrument if-let and let-chains for branch coverage. From 3de87feba23c6b9df520f284b51390c02ea8d12a Mon Sep 17 00:00:00 2001 From: Zalathar Date: Wed, 17 Apr 2024 12:32:11 +1000 Subject: [PATCH 3/8] coverage: Branch coverage tests for match arms --- tests/coverage/branch/match-arms.cov-map | 92 ++++++++++++++++ tests/coverage/branch/match-arms.coverage | 105 +++++++++++++++++++ tests/coverage/branch/match-arms.rs | 90 ++++++++++++++++ tests/coverage/branch/match-trivial.cov-map | 17 +++ tests/coverage/branch/match-trivial.coverage | 49 +++++++++ tests/coverage/branch/match-trivial.rs | 48 +++++++++ 6 files changed, 401 insertions(+) create mode 100644 tests/coverage/branch/match-arms.cov-map create mode 100644 tests/coverage/branch/match-arms.coverage create mode 100644 tests/coverage/branch/match-arms.rs create mode 100644 tests/coverage/branch/match-trivial.cov-map create mode 100644 tests/coverage/branch/match-trivial.coverage create mode 100644 tests/coverage/branch/match-trivial.rs diff --git a/tests/coverage/branch/match-arms.cov-map b/tests/coverage/branch/match-arms.cov-map new file mode 100644 index 0000000000000..1f17f11baaa07 --- /dev/null +++ b/tests/coverage/branch/match-arms.cov-map @@ -0,0 +1,92 @@ +Function name: match_arms::guards +Raw bytes (88): 0x[01, 01, 08, 07, 15, 0b, 11, 0f, 0d, 00, 09, 17, 25, 1b, 21, 1f, 1d, 03, 19, 0c, 01, 30, 01, 01, 10, 29, 03, 0b, 00, 10, 19, 01, 11, 00, 29, 20, 19, 09, 00, 17, 00, 1b, 1d, 01, 11, 00, 29, 20, 1d, 0d, 00, 17, 00, 1b, 21, 01, 11, 00, 29, 20, 21, 11, 00, 17, 00, 1b, 25, 01, 11, 00, 29, 20, 25, 15, 00, 17, 00, 1b, 03, 01, 0e, 00, 18, 13, 03, 05, 01, 02] +Number of files: 1 +- file 0 => global file 1 +Number of expressions: 8 +- expression 0 operands: lhs = Expression(1, Add), rhs = Counter(5) +- expression 1 operands: lhs = Expression(2, Add), rhs = Counter(4) +- expression 2 operands: lhs = Expression(3, Add), rhs = Counter(3) +- expression 3 operands: lhs = Zero, rhs = Counter(2) +- expression 4 operands: lhs = Expression(5, Add), rhs = Counter(9) +- expression 5 operands: lhs = Expression(6, Add), rhs = Counter(8) +- expression 6 operands: lhs = Expression(7, Add), rhs = Counter(7) +- expression 7 operands: lhs = Expression(0, Add), rhs = Counter(6) +Number of file 0 mappings: 12 +- Code(Counter(0)) at (prev + 48, 1) to (start + 1, 16) +- Code(Counter(10)) at (prev + 3, 11) to (start + 0, 16) +- Code(Counter(6)) at (prev + 1, 17) to (start + 0, 41) +- Branch { true: Counter(6), false: Counter(2) } at (prev + 0, 23) to (start + 0, 27) + true = c6 + false = c2 +- Code(Counter(7)) at (prev + 1, 17) to (start + 0, 41) +- Branch { true: Counter(7), false: Counter(3) } at (prev + 0, 23) to (start + 0, 27) + true = c7 + false = c3 +- Code(Counter(8)) at (prev + 1, 17) to (start + 0, 41) +- Branch { true: Counter(8), false: Counter(4) } at (prev + 0, 23) to (start + 0, 27) + true = c8 + false = c4 +- Code(Counter(9)) at (prev + 1, 17) to (start + 0, 41) +- Branch { true: Counter(9), false: Counter(5) } at (prev + 0, 23) to (start + 0, 27) + true = c9 + false = c5 +- Code(Expression(0, Add)) at (prev + 1, 14) to (start + 0, 24) + = ((((Zero + c2) + c3) + c4) + c5) +- Code(Expression(4, Add)) at (prev + 3, 5) to (start + 1, 2) + = ((((((((Zero + c2) + c3) + c4) + c5) + c6) + c7) + c8) + c9) + +Function name: match_arms::match_arms +Raw bytes (51): 0x[01, 01, 06, 05, 07, 0b, 11, 09, 0d, 13, 02, 17, 09, 11, 0d, 07, 01, 18, 01, 01, 10, 05, 03, 0b, 00, 10, 11, 01, 11, 00, 21, 0d, 01, 11, 00, 21, 09, 01, 11, 00, 21, 02, 01, 11, 00, 21, 0f, 03, 05, 01, 02] +Number of files: 1 +- file 0 => global file 1 +Number of expressions: 6 +- expression 0 operands: lhs = Counter(1), rhs = Expression(1, Add) +- expression 1 operands: lhs = Expression(2, Add), rhs = Counter(4) +- expression 2 operands: lhs = Counter(2), rhs = Counter(3) +- expression 3 operands: lhs = Expression(4, Add), rhs = Expression(0, Sub) +- expression 4 operands: lhs = Expression(5, Add), rhs = Counter(2) +- expression 5 operands: lhs = Counter(4), rhs = Counter(3) +Number of file 0 mappings: 7 +- Code(Counter(0)) at (prev + 24, 1) to (start + 1, 16) +- Code(Counter(1)) at (prev + 3, 11) to (start + 0, 16) +- Code(Counter(4)) at (prev + 1, 17) to (start + 0, 33) +- Code(Counter(3)) at (prev + 1, 17) to (start + 0, 33) +- Code(Counter(2)) at (prev + 1, 17) to (start + 0, 33) +- Code(Expression(0, Sub)) at (prev + 1, 17) to (start + 0, 33) + = (c1 - ((c2 + c3) + c4)) +- Code(Expression(3, Add)) at (prev + 3, 5) to (start + 1, 2) + = (((c4 + c3) + c2) + (c1 - ((c2 + c3) + c4))) + +Function name: match_arms::or_patterns +Raw bytes (75): 0x[01, 01, 0d, 11, 0d, 05, 2f, 33, 11, 09, 0d, 09, 2a, 05, 2f, 33, 11, 09, 0d, 03, 27, 09, 2a, 05, 2f, 33, 11, 09, 0d, 09, 01, 25, 01, 01, 10, 05, 03, 0b, 00, 10, 11, 01, 11, 00, 12, 0d, 00, 1e, 00, 1f, 03, 00, 24, 00, 2e, 09, 01, 11, 00, 12, 2a, 00, 1e, 00, 1f, 27, 00, 24, 00, 2e, 23, 03, 05, 01, 02] +Number of files: 1 +- file 0 => global file 1 +Number of expressions: 13 +- expression 0 operands: lhs = Counter(4), rhs = Counter(3) +- expression 1 operands: lhs = Counter(1), rhs = Expression(11, Add) +- expression 2 operands: lhs = Expression(12, Add), rhs = Counter(4) +- expression 3 operands: lhs = Counter(2), rhs = Counter(3) +- expression 4 operands: lhs = Counter(2), rhs = Expression(10, Sub) +- expression 5 operands: lhs = Counter(1), rhs = Expression(11, Add) +- expression 6 operands: lhs = Expression(12, Add), rhs = Counter(4) +- expression 7 operands: lhs = Counter(2), rhs = Counter(3) +- expression 8 operands: lhs = Expression(0, Add), rhs = Expression(9, Add) +- expression 9 operands: lhs = Counter(2), rhs = Expression(10, Sub) +- expression 10 operands: lhs = Counter(1), rhs = Expression(11, Add) +- expression 11 operands: lhs = Expression(12, Add), rhs = Counter(4) +- expression 12 operands: lhs = Counter(2), rhs = Counter(3) +Number of file 0 mappings: 9 +- Code(Counter(0)) at (prev + 37, 1) to (start + 1, 16) +- Code(Counter(1)) at (prev + 3, 11) to (start + 0, 16) +- Code(Counter(4)) at (prev + 1, 17) to (start + 0, 18) +- Code(Counter(3)) at (prev + 0, 30) to (start + 0, 31) +- Code(Expression(0, Add)) at (prev + 0, 36) to (start + 0, 46) + = (c4 + c3) +- Code(Counter(2)) at (prev + 1, 17) to (start + 0, 18) +- Code(Expression(10, Sub)) at (prev + 0, 30) to (start + 0, 31) + = (c1 - ((c2 + c3) + c4)) +- Code(Expression(9, Add)) at (prev + 0, 36) to (start + 0, 46) + = (c2 + (c1 - ((c2 + c3) + c4))) +- Code(Expression(8, Add)) at (prev + 3, 5) to (start + 1, 2) + = ((c4 + c3) + (c2 + (c1 - ((c2 + c3) + c4)))) + diff --git a/tests/coverage/branch/match-arms.coverage b/tests/coverage/branch/match-arms.coverage new file mode 100644 index 0000000000000..ea8a6f97ab154 --- /dev/null +++ b/tests/coverage/branch/match-arms.coverage @@ -0,0 +1,105 @@ + LL| |#![feature(coverage_attribute)] + LL| |//@ edition: 2021 + LL| |//@ compile-flags: -Zcoverage-options=branch + LL| |//@ llvm-cov-flags: --show-branches=count + LL| | + LL| |// Tests for branch coverage of various kinds of match arms. + LL| | + LL| |// Helper macro to prevent start-of-function spans from being merged into + LL| |// spans on the lines we care about. + LL| |macro_rules! no_merge { + LL| | () => { + LL| | for _ in 0..1 {} + LL| | }; + LL| |} + LL| | + LL| |#[derive(Clone, Copy, Debug)] + LL| |enum Enum { + LL| | A(u32), + LL| | B(u32), + LL| | C(u32), + LL| | D(u32), + LL| |} + LL| | + LL| 15|fn match_arms(value: Enum) { + LL| 15| no_merge!(); + LL| | + LL| 15| match value { + LL| 8| Enum::D(d) => consume(d), + LL| 4| Enum::C(c) => consume(c), + LL| 2| Enum::B(b) => consume(b), + LL| 1| Enum::A(a) => consume(a), + LL| | } + LL| | + LL| 15| consume(0); + LL| 15|} + LL| | + LL| 15|fn or_patterns(value: Enum) { + LL| 15| no_merge!(); + LL| | + LL| 15| match value { + LL| 12| Enum::D(x) | Enum::C(x) => consume(x), + ^8 ^4 + LL| 3| Enum::B(y) | Enum::A(y) => consume(y), + ^2 ^1 + LL| | } + LL| | + LL| 15| consume(0); + LL| 15|} + LL| | + LL| 45|fn guards(value: Enum, cond: bool) { + LL| 45| no_merge!(); + LL| | + LL| 3| match value { + LL| 8| Enum::D(d) if cond => consume(d), + ------------------ + | Branch (LL:23): [True: 8, False: 16] + ------------------ + LL| 4| Enum::C(c) if cond => consume(c), + ------------------ + | Branch (LL:23): [True: 4, False: 8] + ------------------ + LL| 2| Enum::B(b) if cond => consume(b), + ------------------ + | Branch (LL:23): [True: 2, False: 4] + ------------------ + LL| 1| Enum::A(a) if cond => consume(a), + ------------------ + | Branch (LL:23): [True: 1, False: 2] + ------------------ + LL| 30| _ => consume(0), + LL| | } + LL| | + LL| 45| consume(0); + LL| 45|} + LL| | + LL| |#[coverage(off)] + LL| |fn consume(x: T) { + LL| | core::hint::black_box(x); + LL| |} + LL| | + LL| |#[coverage(off)] + LL| |fn main() { + LL| | #[coverage(off)] + LL| | fn call_everything(e: Enum) { + LL| | match_arms(e); + LL| | or_patterns(e); + LL| | for cond in [false, false, true] { + LL| | guards(e, cond); + LL| | } + LL| | } + LL| | + LL| | call_everything(Enum::A(0)); + LL| | for b in 0..2 { + LL| | call_everything(Enum::B(b)); + LL| | } + LL| | for c in 0..4 { + LL| | call_everything(Enum::C(c)); + LL| | } + LL| | for d in 0..8 { + LL| | call_everything(Enum::D(d)); + LL| | } + LL| |} + LL| | + LL| |// FIXME(#124118) Actually instrument match arms for branch coverage. + diff --git a/tests/coverage/branch/match-arms.rs b/tests/coverage/branch/match-arms.rs new file mode 100644 index 0000000000000..63151f59ffe9b --- /dev/null +++ b/tests/coverage/branch/match-arms.rs @@ -0,0 +1,90 @@ +#![feature(coverage_attribute)] +//@ edition: 2021 +//@ compile-flags: -Zcoverage-options=branch +//@ llvm-cov-flags: --show-branches=count + +// Tests for branch coverage of various kinds of match arms. + +// Helper macro to prevent start-of-function spans from being merged into +// spans on the lines we care about. +macro_rules! no_merge { + () => { + for _ in 0..1 {} + }; +} + +#[derive(Clone, Copy, Debug)] +enum Enum { + A(u32), + B(u32), + C(u32), + D(u32), +} + +fn match_arms(value: Enum) { + no_merge!(); + + match value { + Enum::D(d) => consume(d), + Enum::C(c) => consume(c), + Enum::B(b) => consume(b), + Enum::A(a) => consume(a), + } + + consume(0); +} + +fn or_patterns(value: Enum) { + no_merge!(); + + match value { + Enum::D(x) | Enum::C(x) => consume(x), + Enum::B(y) | Enum::A(y) => consume(y), + } + + consume(0); +} + +fn guards(value: Enum, cond: bool) { + no_merge!(); + + match value { + Enum::D(d) if cond => consume(d), + Enum::C(c) if cond => consume(c), + Enum::B(b) if cond => consume(b), + Enum::A(a) if cond => consume(a), + _ => consume(0), + } + + consume(0); +} + +#[coverage(off)] +fn consume(x: T) { + core::hint::black_box(x); +} + +#[coverage(off)] +fn main() { + #[coverage(off)] + fn call_everything(e: Enum) { + match_arms(e); + or_patterns(e); + for cond in [false, false, true] { + guards(e, cond); + } + } + + call_everything(Enum::A(0)); + for b in 0..2 { + call_everything(Enum::B(b)); + } + for c in 0..4 { + call_everything(Enum::C(c)); + } + for d in 0..8 { + call_everything(Enum::D(d)); + } +} + +// FIXME(#124118) Actually instrument match arms for branch coverage. diff --git a/tests/coverage/branch/match-trivial.cov-map b/tests/coverage/branch/match-trivial.cov-map new file mode 100644 index 0000000000000..1136a529b255c --- /dev/null +++ b/tests/coverage/branch/match-trivial.cov-map @@ -0,0 +1,17 @@ +Function name: match_trivial::_uninhabited (unused) +Raw bytes (9): 0x[01, 01, 00, 01, 00, 16, 01, 01, 10] +Number of files: 1 +- file 0 => global file 1 +Number of expressions: 0 +Number of file 0 mappings: 1 +- Code(Zero) at (prev + 22, 1) to (start + 1, 16) + +Function name: match_trivial::trivial +Raw bytes (14): 0x[01, 01, 00, 02, 01, 1e, 01, 01, 10, 05, 03, 0b, 05, 02] +Number of files: 1 +- file 0 => global file 1 +Number of expressions: 0 +Number of file 0 mappings: 2 +- Code(Counter(0)) at (prev + 30, 1) to (start + 1, 16) +- Code(Counter(1)) at (prev + 3, 11) to (start + 5, 2) + diff --git a/tests/coverage/branch/match-trivial.coverage b/tests/coverage/branch/match-trivial.coverage new file mode 100644 index 0000000000000..4ffb172e1b675 --- /dev/null +++ b/tests/coverage/branch/match-trivial.coverage @@ -0,0 +1,49 @@ + LL| |#![feature(coverage_attribute)] + LL| |//@ edition: 2021 + LL| |//@ compile-flags: -Zcoverage-options=branch + LL| |//@ llvm-cov-flags: --show-branches=count + LL| | + LL| |// When instrumenting match expressions for branch coverage, make sure we don't + LL| |// cause an ICE or produce weird coverage output for matches with <2 arms. + LL| | + LL| |// Helper macro to prevent start-of-function spans from being merged into + LL| |// spans on the lines we care about. + LL| |macro_rules! no_merge { + LL| | () => { + LL| | for _ in 0..1 {} + LL| | }; + LL| |} + LL| | + LL| |enum Uninhabited {} + LL| |enum Trivial { + LL| | Value, + LL| |} + LL| | + LL| 0|fn _uninhabited(x: Uninhabited) { + LL| 0| no_merge!(); + LL| | + LL| | match x {} + LL| | + LL| | consume("done"); + LL| |} + LL| | + LL| 1|fn trivial(x: Trivial) { + LL| 1| no_merge!(); + LL| | + LL| 1| match x { + LL| 1| Trivial::Value => consume("trivial"), + LL| 1| } + LL| 1| + LL| 1| consume("done"); + LL| 1|} + LL| | + LL| |#[coverage(off)] + LL| |fn consume(x: T) { + LL| | core::hint::black_box(x); + LL| |} + LL| | + LL| |#[coverage(off)] + LL| |fn main() { + LL| | trivial(Trivial::Value); + LL| |} + diff --git a/tests/coverage/branch/match-trivial.rs b/tests/coverage/branch/match-trivial.rs new file mode 100644 index 0000000000000..db8887a26b7a4 --- /dev/null +++ b/tests/coverage/branch/match-trivial.rs @@ -0,0 +1,48 @@ +#![feature(coverage_attribute)] +//@ edition: 2021 +//@ compile-flags: -Zcoverage-options=branch +//@ llvm-cov-flags: --show-branches=count + +// When instrumenting match expressions for branch coverage, make sure we don't +// cause an ICE or produce weird coverage output for matches with <2 arms. + +// Helper macro to prevent start-of-function spans from being merged into +// spans on the lines we care about. +macro_rules! no_merge { + () => { + for _ in 0..1 {} + }; +} + +enum Uninhabited {} +enum Trivial { + Value, +} + +fn _uninhabited(x: Uninhabited) { + no_merge!(); + + match x {} + + consume("done"); +} + +fn trivial(x: Trivial) { + no_merge!(); + + match x { + Trivial::Value => consume("trivial"), + } + + consume("done"); +} + +#[coverage(off)] +fn consume(x: T) { + core::hint::black_box(x); +} + +#[coverage(off)] +fn main() { + trivial(Trivial::Value); +} From da37b14121d0fed6e49a4ed27abd2b9e7fc1c486 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Fri, 19 Apr 2024 14:50:25 +1000 Subject: [PATCH 4/8] coverage: Move mir-opt coverage tests into a subdirectory --- .../instrument_coverage.bar.InstrumentCoverage.diff | 0 .../instrument_coverage.main.InstrumentCoverage.diff | 0 tests/mir-opt/{ => coverage}/instrument_coverage.rs | 0 .../instrument_coverage_cleanup.main.CleanupPostBorrowck.diff | 0 .../instrument_coverage_cleanup.main.InstrumentCoverage.diff | 0 tests/mir-opt/{ => coverage}/instrument_coverage_cleanup.rs | 0 6 files changed, 0 insertions(+), 0 deletions(-) rename tests/mir-opt/{ => coverage}/instrument_coverage.bar.InstrumentCoverage.diff (100%) rename tests/mir-opt/{ => coverage}/instrument_coverage.main.InstrumentCoverage.diff (100%) rename tests/mir-opt/{ => coverage}/instrument_coverage.rs (100%) rename tests/mir-opt/{ => coverage}/instrument_coverage_cleanup.main.CleanupPostBorrowck.diff (100%) rename tests/mir-opt/{ => coverage}/instrument_coverage_cleanup.main.InstrumentCoverage.diff (100%) rename tests/mir-opt/{ => coverage}/instrument_coverage_cleanup.rs (100%) diff --git a/tests/mir-opt/instrument_coverage.bar.InstrumentCoverage.diff b/tests/mir-opt/coverage/instrument_coverage.bar.InstrumentCoverage.diff similarity index 100% rename from tests/mir-opt/instrument_coverage.bar.InstrumentCoverage.diff rename to tests/mir-opt/coverage/instrument_coverage.bar.InstrumentCoverage.diff diff --git a/tests/mir-opt/instrument_coverage.main.InstrumentCoverage.diff b/tests/mir-opt/coverage/instrument_coverage.main.InstrumentCoverage.diff similarity index 100% rename from tests/mir-opt/instrument_coverage.main.InstrumentCoverage.diff rename to tests/mir-opt/coverage/instrument_coverage.main.InstrumentCoverage.diff diff --git a/tests/mir-opt/instrument_coverage.rs b/tests/mir-opt/coverage/instrument_coverage.rs similarity index 100% rename from tests/mir-opt/instrument_coverage.rs rename to tests/mir-opt/coverage/instrument_coverage.rs diff --git a/tests/mir-opt/instrument_coverage_cleanup.main.CleanupPostBorrowck.diff b/tests/mir-opt/coverage/instrument_coverage_cleanup.main.CleanupPostBorrowck.diff similarity index 100% rename from tests/mir-opt/instrument_coverage_cleanup.main.CleanupPostBorrowck.diff rename to tests/mir-opt/coverage/instrument_coverage_cleanup.main.CleanupPostBorrowck.diff diff --git a/tests/mir-opt/instrument_coverage_cleanup.main.InstrumentCoverage.diff b/tests/mir-opt/coverage/instrument_coverage_cleanup.main.InstrumentCoverage.diff similarity index 100% rename from tests/mir-opt/instrument_coverage_cleanup.main.InstrumentCoverage.diff rename to tests/mir-opt/coverage/instrument_coverage_cleanup.main.InstrumentCoverage.diff diff --git a/tests/mir-opt/instrument_coverage_cleanup.rs b/tests/mir-opt/coverage/instrument_coverage_cleanup.rs similarity index 100% rename from tests/mir-opt/instrument_coverage_cleanup.rs rename to tests/mir-opt/coverage/instrument_coverage_cleanup.rs From a892c2387e64ba080a6503d422931f3a0916826a Mon Sep 17 00:00:00 2001 From: Zalathar Date: Fri, 19 Apr 2024 15:35:23 +1000 Subject: [PATCH 5/8] coverage: Add a mir-opt test for branch coverage of match arms --- ...ch_match_arms.main.InstrumentCoverage.diff | 138 ++++++++++++++++++ tests/mir-opt/coverage/branch_match_arms.rs | 27 ++++ 2 files changed, 165 insertions(+) create mode 100644 tests/mir-opt/coverage/branch_match_arms.main.InstrumentCoverage.diff create mode 100644 tests/mir-opt/coverage/branch_match_arms.rs diff --git a/tests/mir-opt/coverage/branch_match_arms.main.InstrumentCoverage.diff b/tests/mir-opt/coverage/branch_match_arms.main.InstrumentCoverage.diff new file mode 100644 index 0000000000000..e60f71f47b1ed --- /dev/null +++ b/tests/mir-opt/coverage/branch_match_arms.main.InstrumentCoverage.diff @@ -0,0 +1,138 @@ +- // MIR for `main` before InstrumentCoverage ++ // MIR for `main` after InstrumentCoverage + + fn main() -> () { + let mut _0: (); + let mut _1: Enum; + let mut _2: isize; + let _3: u32; + let mut _4: u32; + let _5: u32; + let mut _6: u32; + let _7: u32; + let mut _8: u32; + let _9: u32; + let mut _10: u32; + scope 1 { + debug d => _3; + } + scope 2 { + debug c => _5; + } + scope 3 { + debug b => _7; + } + scope 4 { + debug a => _9; + } + ++ coverage ExpressionId(0) => Expression { lhs: Counter(1), op: Add, rhs: Counter(2) }; ++ coverage ExpressionId(1) => Expression { lhs: Expression(0), op: Add, rhs: Counter(3) }; ++ coverage ExpressionId(2) => Expression { lhs: Counter(0), op: Subtract, rhs: Expression(1) }; ++ coverage ExpressionId(3) => Expression { lhs: Counter(3), op: Add, rhs: Counter(2) }; ++ coverage ExpressionId(4) => Expression { lhs: Expression(3), op: Add, rhs: Counter(1) }; ++ coverage ExpressionId(5) => Expression { lhs: Expression(4), op: Add, rhs: Expression(2) }; ++ coverage Code(Counter(0)) => $DIR/branch_match_arms.rs:14:1 - 15:21; ++ coverage Code(Counter(3)) => $DIR/branch_match_arms.rs:16:17 - 16:33; ++ coverage Code(Counter(2)) => $DIR/branch_match_arms.rs:17:17 - 17:33; ++ coverage Code(Counter(1)) => $DIR/branch_match_arms.rs:18:17 - 18:33; ++ coverage Code(Expression(2)) => $DIR/branch_match_arms.rs:19:17 - 19:33; ++ coverage Code(Expression(5)) => $DIR/branch_match_arms.rs:21:1 - 21:2; ++ + bb0: { ++ Coverage::CounterIncrement(0); + StorageLive(_1); + _1 = Enum::A(const 0_u32); + PlaceMention(_1); + _2 = discriminant(_1); + switchInt(move _2) -> [0: bb5, 1: bb4, 2: bb3, 3: bb2, otherwise: bb1]; + } + + bb1: { + FakeRead(ForMatchedPlace(None), _1); + unreachable; + } + + bb2: { ++ Coverage::CounterIncrement(3); + falseEdge -> [real: bb6, imaginary: bb3]; + } + + bb3: { ++ Coverage::CounterIncrement(2); + falseEdge -> [real: bb8, imaginary: bb4]; + } + + bb4: { ++ Coverage::CounterIncrement(1); + falseEdge -> [real: bb10, imaginary: bb5]; + } + + bb5: { ++ Coverage::ExpressionUsed(2); + StorageLive(_9); + _9 = ((_1 as A).0: u32); + StorageLive(_10); + _10 = _9; + _0 = consume(move _10) -> [return: bb12, unwind: bb14]; + } + + bb6: { + StorageLive(_3); + _3 = ((_1 as D).0: u32); + StorageLive(_4); + _4 = _3; + _0 = consume(move _4) -> [return: bb7, unwind: bb14]; + } + + bb7: { + StorageDead(_4); + StorageDead(_3); + goto -> bb13; + } + + bb8: { + StorageLive(_5); + _5 = ((_1 as C).0: u32); + StorageLive(_6); + _6 = _5; + _0 = consume(move _6) -> [return: bb9, unwind: bb14]; + } + + bb9: { + StorageDead(_6); + StorageDead(_5); + goto -> bb13; + } + + bb10: { + StorageLive(_7); + _7 = ((_1 as B).0: u32); + StorageLive(_8); + _8 = _7; + _0 = consume(move _8) -> [return: bb11, unwind: bb14]; + } + + bb11: { + StorageDead(_8); + StorageDead(_7); + goto -> bb13; + } + + bb12: { + StorageDead(_10); + StorageDead(_9); + goto -> bb13; + } + + bb13: { ++ Coverage::ExpressionUsed(5); + StorageDead(_1); + return; + } + + bb14 (cleanup): { + resume; + } + } + diff --git a/tests/mir-opt/coverage/branch_match_arms.rs b/tests/mir-opt/coverage/branch_match_arms.rs new file mode 100644 index 0000000000000..18764b38d6e3e --- /dev/null +++ b/tests/mir-opt/coverage/branch_match_arms.rs @@ -0,0 +1,27 @@ +#![feature(coverage_attribute)] +//@ test-mir-pass: InstrumentCoverage +//@ compile-flags: -Cinstrument-coverage -Zno-profiler-runtime -Zcoverage-options=branch +// skip-filecheck + +enum Enum { + A(u32), + B(u32), + C(u32), + D(u32), +} + +// EMIT_MIR branch_match_arms.main.InstrumentCoverage.diff +fn main() { + match Enum::A(0) { + Enum::D(d) => consume(d), + Enum::C(c) => consume(c), + Enum::B(b) => consume(b), + Enum::A(a) => consume(a), + } +} + +#[inline(never)] +#[coverage(off)] +fn consume(x: u32) { + core::hint::black_box(x); +} From 97bf5536827ea7a1ba6b7cf856dd2b22184d2527 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Sun, 21 Apr 2024 12:06:03 +1000 Subject: [PATCH 6/8] coverage: Detach MC/DC branch spans from regular branch spans MC/DC's reliance on the existing branch coverage types is making it much harder to improve branch coverage. --- compiler/rustc_middle/src/mir/coverage.rs | 4 +- compiler/rustc_middle/src/mir/pretty.rs | 2 +- .../rustc_mir_build/src/build/coverageinfo.rs | 47 +++++------- .../rustc_mir_transform/src/coverage/mod.rs | 16 +++- .../rustc_mir_transform/src/coverage/spans.rs | 10 ++- .../src/coverage/spans/from_mir.rs | 74 ++++++++++++++----- 6 files changed, 101 insertions(+), 52 deletions(-) diff --git a/compiler/rustc_middle/src/mir/coverage.rs b/compiler/rustc_middle/src/mir/coverage.rs index b1d0c815ae06b..04011fd41948e 100644 --- a/compiler/rustc_middle/src/mir/coverage.rs +++ b/compiler/rustc_middle/src/mir/coverage.rs @@ -314,7 +314,9 @@ impl Default for ConditionInfo { #[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)] pub struct MCDCBranchSpan { pub span: Span, - pub condition_info: ConditionInfo, + /// If `None`, this actually represents a normal branch span inserted for + /// code that was too complex for MC/DC. + pub condition_info: Option, pub true_marker: BlockMarkerId, pub false_marker: BlockMarkerId, } diff --git a/compiler/rustc_middle/src/mir/pretty.rs b/compiler/rustc_middle/src/mir/pretty.rs index 8291404ebf31f..0c9ceae63c1cd 100644 --- a/compiler/rustc_middle/src/mir/pretty.rs +++ b/compiler/rustc_middle/src/mir/pretty.rs @@ -491,7 +491,7 @@ fn write_coverage_branch_info( writeln!( w, "{INDENT}coverage mcdc branch {{ condition_id: {:?}, true: {true_marker:?}, false: {false_marker:?} }} => {span:?}", - condition_info.condition_id + condition_info.map(|info| info.condition_id) )?; } diff --git a/compiler/rustc_mir_build/src/build/coverageinfo.rs b/compiler/rustc_mir_build/src/build/coverageinfo.rs index 57f809ef7cf8a..15963c1234f8e 100644 --- a/compiler/rustc_mir_build/src/build/coverageinfo.rs +++ b/compiler/rustc_mir_build/src/build/coverageinfo.rs @@ -22,6 +22,7 @@ pub(crate) struct BranchInfoBuilder { num_block_markers: usize, branch_spans: Vec, + mcdc_branch_spans: Vec, mcdc_decision_spans: Vec, mcdc_state: Option, @@ -95,13 +96,7 @@ impl BranchInfoBuilder { } } - fn record_conditions_operation(&mut self, logical_op: LogicalOp, span: Span) { - if let Some(mcdc_state) = self.mcdc_state.as_mut() { - mcdc_state.record_conditions(logical_op, span); - } - } - - fn fetch_condition_info( + fn fetch_mcdc_condition_info( &mut self, tcx: TyCtxt<'_>, true_marker: BlockMarkerId, @@ -121,14 +116,9 @@ impl BranchInfoBuilder { _ => { // Do not generate mcdc mappings and statements for decisions with too many conditions. let rebase_idx = self.mcdc_branch_spans.len() - decision.conditions_num + 1; - let to_normal_branches = self.mcdc_branch_spans.split_off(rebase_idx); - self.branch_spans.extend(to_normal_branches.into_iter().map( - |MCDCBranchSpan { span, true_marker, false_marker, .. }| BranchSpan { - span, - true_marker, - false_marker, - }, - )); + for branch in &mut self.mcdc_branch_spans[rebase_idx..] { + branch.condition_info = None; + } // ConditionInfo of this branch shall also be reset. condition_info = None; @@ -157,7 +147,7 @@ impl BranchInfoBuilder { branch_spans, mcdc_branch_spans, mcdc_decision_spans, - .. + mcdc_state: _, } = self; if num_block_markers == 0 { @@ -355,27 +345,30 @@ impl Builder<'_, '_> { let true_marker = inject_branch_marker(then_block); let false_marker = inject_branch_marker(else_block); - if let Some(condition_info) = - branch_info.fetch_condition_info(self.tcx, true_marker, false_marker) - { + if branch_info.mcdc_state.is_some() { + let condition_info = + branch_info.fetch_mcdc_condition_info(self.tcx, true_marker, false_marker); branch_info.mcdc_branch_spans.push(MCDCBranchSpan { span: source_info.span, condition_info, true_marker, false_marker, }); - } else { - branch_info.branch_spans.push(BranchSpan { - span: source_info.span, - true_marker, - false_marker, - }); + return; } + + branch_info.branch_spans.push(BranchSpan { + span: source_info.span, + true_marker, + false_marker, + }); } pub(crate) fn visit_coverage_branch_operation(&mut self, logical_op: LogicalOp, span: Span) { - if let Some(branch_info) = self.coverage_branch_info.as_mut() { - branch_info.record_conditions_operation(logical_op, span); + if let Some(branch_info) = self.coverage_branch_info.as_mut() + && let Some(mcdc_state) = branch_info.mcdc_state.as_mut() + { + mcdc_state.record_conditions(logical_op, span); } } } diff --git a/compiler/rustc_mir_transform/src/coverage/mod.rs b/compiler/rustc_mir_transform/src/coverage/mod.rs index 9e8648b0f930d..d1516605fb603 100644 --- a/compiler/rustc_mir_transform/src/coverage/mod.rs +++ b/compiler/rustc_mir_transform/src/coverage/mod.rs @@ -149,13 +149,21 @@ fn create_mappings<'tcx>( true_term: term_for_bcb(true_bcb), false_term: term_for_bcb(false_bcb), }, - BcbMappingKind::MCDCBranch { true_bcb, false_bcb, condition_info } => { - MappingKind::MCDCBranch { + BcbMappingKind::MCDCBranch { true_bcb, false_bcb, condition_info: None } => { + MappingKind::Branch { true_term: term_for_bcb(true_bcb), false_term: term_for_bcb(false_bcb), - mcdc_params: condition_info, } } + BcbMappingKind::MCDCBranch { + true_bcb, + false_bcb, + condition_info: Some(mcdc_params), + } => MappingKind::MCDCBranch { + true_term: term_for_bcb(true_bcb), + false_term: term_for_bcb(false_bcb), + mcdc_params, + }, BcbMappingKind::MCDCDecision { bitmap_idx, conditions_num, .. } => { MappingKind::MCDCDecision(DecisionInfo { bitmap_idx, conditions_num }) } @@ -249,7 +257,7 @@ fn inject_mcdc_statements<'tcx>( for (true_bcb, false_bcb, condition_id) in coverage_spans.all_bcb_mappings().filter_map(|mapping| match mapping.kind { BcbMappingKind::MCDCBranch { true_bcb, false_bcb, condition_info } => { - Some((true_bcb, false_bcb, condition_info.condition_id)) + Some((true_bcb, false_bcb, condition_info?.condition_id)) } _ => None, }) diff --git a/compiler/rustc_mir_transform/src/coverage/spans.rs b/compiler/rustc_mir_transform/src/coverage/spans.rs index a4cd8a38c66a2..8d2241783f647 100644 --- a/compiler/rustc_mir_transform/src/coverage/spans.rs +++ b/compiler/rustc_mir_transform/src/coverage/spans.rs @@ -21,7 +21,9 @@ pub(super) enum BcbMappingKind { MCDCBranch { true_bcb: BasicCoverageBlock, false_bcb: BasicCoverageBlock, - condition_info: ConditionInfo, + /// If `None`, this actually represents a normal branch mapping inserted + /// for code that was too complex for MC/DC. + condition_info: Option, }, /// Associates a mcdc decision with its join BCB. MCDCDecision { end_bcbs: BTreeSet, bitmap_idx: u32, conditions_num: u16 }, @@ -85,6 +87,12 @@ pub(super) fn generate_coverage_spans( })); mappings.extend(from_mir::extract_branch_mappings( + mir_body, + hir_info, + basic_coverage_blocks, + )); + + mappings.extend(from_mir::extract_mcdc_mappings( mir_body, hir_info.body_span, basic_coverage_blocks, diff --git a/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs b/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs index b9919a2ae884e..3c64591d43e9c 100644 --- a/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs +++ b/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs @@ -366,15 +366,10 @@ impl SpanFromMir { } } -pub(super) fn extract_branch_mappings( +fn resolve_block_markers( + branch_info: &mir::coverage::BranchInfo, mir_body: &mir::Body<'_>, - body_span: Span, - basic_coverage_blocks: &CoverageGraph, -) -> Vec { - let Some(branch_info) = mir_body.coverage_branch_info.as_deref() else { - return vec![]; - }; - +) -> IndexVec> { let mut block_markers = IndexVec::>::from_elem_n( None, branch_info.num_block_markers, @@ -389,6 +384,58 @@ pub(super) fn extract_branch_mappings( } } + block_markers +} + +// FIXME: There is currently a lot of redundancy between +// `extract_branch_mappings` and `extract_mcdc_mappings`. This is needed so +// that they can each be modified without interfering with the other, but in +// the long term we should try to bring them together again when branch coverage +// and MC/DC coverage support are more mature. + +pub(super) fn extract_branch_mappings( + mir_body: &mir::Body<'_>, + hir_info: &ExtractedHirInfo, + basic_coverage_blocks: &CoverageGraph, +) -> Vec { + let Some(branch_info) = mir_body.coverage_branch_info.as_deref() else { return vec![] }; + + let block_markers = resolve_block_markers(branch_info, mir_body); + + branch_info + .branch_spans + .iter() + .filter_map(|&BranchSpan { span: raw_span, true_marker, false_marker }| { + // For now, ignore any branch span that was introduced by + // expansion. This makes things like assert macros less noisy. + if !raw_span.ctxt().outer_expn_data().is_root() { + return None; + } + let (span, _) = + unexpand_into_body_span_with_visible_macro(raw_span, hir_info.body_span)?; + + let bcb_from_marker = + |marker: BlockMarkerId| basic_coverage_blocks.bcb_from_bb(block_markers[marker]?); + + let true_bcb = bcb_from_marker(true_marker)?; + let false_bcb = bcb_from_marker(false_marker)?; + + Some(BcbMapping { kind: BcbMappingKind::Branch { true_bcb, false_bcb }, span }) + }) + .collect::>() +} + +pub(super) fn extract_mcdc_mappings( + mir_body: &mir::Body<'_>, + body_span: Span, + basic_coverage_blocks: &CoverageGraph, +) -> Vec { + let Some(branch_info) = mir_body.coverage_branch_info.as_deref() else { + return vec![]; + }; + + let block_markers = resolve_block_markers(branch_info, mir_body); + let bcb_from_marker = |marker: BlockMarkerId| basic_coverage_blocks.bcb_from_bb(block_markers[marker]?); @@ -406,12 +453,6 @@ pub(super) fn extract_branch_mappings( Some((span, true_bcb, false_bcb)) }; - let branch_filter_map = |&BranchSpan { span: raw_span, true_marker, false_marker }| { - check_branch_bcb(raw_span, true_marker, false_marker).map(|(span, true_bcb, false_bcb)| { - BcbMapping { kind: BcbMappingKind::Branch { true_bcb, false_bcb }, span } - }) - }; - let mcdc_branch_filter_map = |&MCDCBranchSpan { span: raw_span, true_marker, false_marker, condition_info }| { check_branch_bcb(raw_span, true_marker, false_marker).map( @@ -446,10 +487,7 @@ pub(super) fn extract_branch_mappings( }) }; - branch_info - .branch_spans - .iter() - .filter_map(branch_filter_map) + std::iter::empty() .chain(branch_info.mcdc_branch_spans.iter().filter_map(mcdc_branch_filter_map)) .chain(branch_info.mcdc_decision_spans.iter().filter_map(decision_filter_map)) .collect::>() From b5a22be6a3ce55e4bbac62767cda62c38d47e162 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Sun, 21 Apr 2024 12:20:09 +1000 Subject: [PATCH 7/8] coverage: Move some helper code into `BranchInfoBuilder` --- .../rustc_mir_build/src/build/coverageinfo.rs | 66 +++++++++++-------- 1 file changed, 40 insertions(+), 26 deletions(-) diff --git a/compiler/rustc_mir_build/src/build/coverageinfo.rs b/compiler/rustc_mir_build/src/build/coverageinfo.rs index 15963c1234f8e..9e9ccd3dc2d01 100644 --- a/compiler/rustc_mir_build/src/build/coverageinfo.rs +++ b/compiler/rustc_mir_build/src/build/coverageinfo.rs @@ -7,13 +7,13 @@ use rustc_middle::mir::coverage::{ BlockMarkerId, BranchSpan, ConditionId, ConditionInfo, CoverageKind, MCDCBranchSpan, MCDCDecisionSpan, }; -use rustc_middle::mir::{self, BasicBlock, UnOp}; +use rustc_middle::mir::{self, BasicBlock, SourceInfo, UnOp}; use rustc_middle::thir::{ExprId, ExprKind, LogicalOp, Thir}; use rustc_middle::ty::TyCtxt; use rustc_span::def_id::LocalDefId; use rustc_span::Span; -use crate::build::Builder; +use crate::build::{Builder, CFG}; use crate::errors::MCDCExceedsConditionNumLimit; pub(crate) struct BranchInfoBuilder { @@ -134,12 +134,42 @@ impl BranchInfoBuilder { condition_info } + fn add_two_way_branch<'tcx>( + &mut self, + cfg: &mut CFG<'tcx>, + source_info: SourceInfo, + true_block: BasicBlock, + false_block: BasicBlock, + ) { + let true_marker = self.inject_block_marker(cfg, source_info, true_block); + let false_marker = self.inject_block_marker(cfg, source_info, false_block); + + self.branch_spans.push(BranchSpan { span: source_info.span, true_marker, false_marker }); + } + fn next_block_marker_id(&mut self) -> BlockMarkerId { let id = BlockMarkerId::from_usize(self.num_block_markers); self.num_block_markers += 1; id } + fn inject_block_marker( + &mut self, + cfg: &mut CFG<'_>, + source_info: SourceInfo, + block: BasicBlock, + ) -> BlockMarkerId { + let id = self.next_block_marker_id(); + + let marker_statement = mir::Statement { + source_info, + kind: mir::StatementKind::Coverage(CoverageKind::BlockMarker { id }), + }; + cfg.push(block, marker_statement); + + id + } + pub(crate) fn into_done(self) -> Option> { let Self { nots: _, @@ -315,7 +345,7 @@ impl Builder<'_, '_> { mut else_block: BasicBlock, ) { // Bail out if branch coverage is not enabled for this function. - let Some(branch_info) = self.coverage_branch_info.as_ref() else { return }; + let Some(branch_info) = self.coverage_branch_info.as_mut() else { return }; // If this condition expression is nested within one or more `!` expressions, // replace it with the enclosing `!` collected by `visit_unary_not`. @@ -325,27 +355,15 @@ impl Builder<'_, '_> { std::mem::swap(&mut then_block, &mut else_block); } } - let source_info = self.source_info(self.thir[expr_id].span); - - // Now that we have `source_info`, we can upgrade to a &mut reference. - let branch_info = self.coverage_branch_info.as_mut().expect("upgrading & to &mut"); - - let mut inject_branch_marker = |block: BasicBlock| { - let id = branch_info.next_block_marker_id(); - - let marker_statement = mir::Statement { - source_info, - kind: mir::StatementKind::Coverage(CoverageKind::BlockMarker { id }), - }; - self.cfg.push(block, marker_statement); - - id - }; - let true_marker = inject_branch_marker(then_block); - let false_marker = inject_branch_marker(else_block); + let source_info = SourceInfo { span: self.thir[expr_id].span, scope: self.source_scope }; + // Separate path for handling branches when MC/DC is enabled. if branch_info.mcdc_state.is_some() { + let mut inject_block_marker = + |block| branch_info.inject_block_marker(&mut self.cfg, source_info, block); + let true_marker = inject_block_marker(then_block); + let false_marker = inject_block_marker(else_block); let condition_info = branch_info.fetch_mcdc_condition_info(self.tcx, true_marker, false_marker); branch_info.mcdc_branch_spans.push(MCDCBranchSpan { @@ -357,11 +375,7 @@ impl Builder<'_, '_> { return; } - branch_info.branch_spans.push(BranchSpan { - span: source_info.span, - true_marker, - false_marker, - }); + branch_info.add_two_way_branch(&mut self.cfg, source_info, then_block, else_block); } pub(crate) fn visit_coverage_branch_operation(&mut self, logical_op: LogicalOp, span: Span) { From 2b6adb06fb8307f19bd42d2fce3ad338dc6112ef Mon Sep 17 00:00:00 2001 From: Zalathar Date: Sun, 21 Apr 2024 13:21:58 +1000 Subject: [PATCH 8/8] coverage: Separate branch pairs from other mapping kinds This clears the way for larger changes to how branches are handled by the coverage instrumentor, in order to support branch coverage for more language constructs. --- .../rustc_mir_transform/src/coverage/mod.rs | 22 +++++++---- .../rustc_mir_transform/src/coverage/spans.rs | 37 +++++++++++++------ .../src/coverage/spans/from_mir.rs | 10 ++--- 3 files changed, 44 insertions(+), 25 deletions(-) diff --git a/compiler/rustc_mir_transform/src/coverage/mod.rs b/compiler/rustc_mir_transform/src/coverage/mod.rs index d1516605fb603..0b15c52c28143 100644 --- a/compiler/rustc_mir_transform/src/coverage/mod.rs +++ b/compiler/rustc_mir_transform/src/coverage/mod.rs @@ -9,7 +9,7 @@ mod tests; use self::counters::{CounterIncrementSite, CoverageCounters}; use self::graph::{BasicCoverageBlock, CoverageGraph}; -use self::spans::{BcbMapping, BcbMappingKind, CoverageSpans}; +use self::spans::{BcbBranchPair, BcbMapping, BcbMappingKind, CoverageSpans}; use crate::MirPass; @@ -141,14 +141,10 @@ fn create_mappings<'tcx>( let mut mappings = Vec::new(); - mappings.extend(coverage_spans.all_bcb_mappings().filter_map( + mappings.extend(coverage_spans.mappings.iter().filter_map( |BcbMapping { kind: bcb_mapping_kind, span }| { let kind = match *bcb_mapping_kind { BcbMappingKind::Code(bcb) => MappingKind::Code(term_for_bcb(bcb)), - BcbMappingKind::Branch { true_bcb, false_bcb } => MappingKind::Branch { - true_term: term_for_bcb(true_bcb), - false_term: term_for_bcb(false_bcb), - }, BcbMappingKind::MCDCBranch { true_bcb, false_bcb, condition_info: None } => { MappingKind::Branch { true_term: term_for_bcb(true_bcb), @@ -173,6 +169,16 @@ fn create_mappings<'tcx>( }, )); + mappings.extend(coverage_spans.branch_pairs.iter().filter_map( + |&BcbBranchPair { span, true_bcb, false_bcb }| { + let true_term = term_for_bcb(true_bcb); + let false_term = term_for_bcb(false_bcb); + let kind = MappingKind::Branch { true_term, false_term }; + let code_region = make_code_region(source_map, file_name, span, body_span)?; + Some(Mapping { kind, code_region }) + }, + )); + mappings } @@ -241,7 +247,7 @@ fn inject_mcdc_statements<'tcx>( // Inject test vector update first because `inject_statement` always insert new statement at head. for (end_bcbs, bitmap_idx) in - coverage_spans.all_bcb_mappings().filter_map(|mapping| match &mapping.kind { + coverage_spans.mappings.iter().filter_map(|mapping| match &mapping.kind { BcbMappingKind::MCDCDecision { end_bcbs, bitmap_idx, .. } => { Some((end_bcbs, *bitmap_idx)) } @@ -255,7 +261,7 @@ fn inject_mcdc_statements<'tcx>( } for (true_bcb, false_bcb, condition_id) in - coverage_spans.all_bcb_mappings().filter_map(|mapping| match mapping.kind { + coverage_spans.mappings.iter().filter_map(|mapping| match mapping.kind { BcbMappingKind::MCDCBranch { true_bcb, false_bcb, condition_info } => { Some((true_bcb, false_bcb, condition_info?.condition_id)) } diff --git a/compiler/rustc_mir_transform/src/coverage/spans.rs b/compiler/rustc_mir_transform/src/coverage/spans.rs index 8d2241783f647..88f18b7208574 100644 --- a/compiler/rustc_mir_transform/src/coverage/spans.rs +++ b/compiler/rustc_mir_transform/src/coverage/spans.rs @@ -15,8 +15,10 @@ mod from_mir; pub(super) enum BcbMappingKind { /// Associates an ordinary executable code span with its corresponding BCB. Code(BasicCoverageBlock), - /// Associates a branch span with BCBs for its true and false arms. - Branch { true_bcb: BasicCoverageBlock, false_bcb: BasicCoverageBlock }, + + // Ordinary branch mappings are stored separately, so they don't have a + // variant in this enum. + // /// Associates a mcdc branch span with condition info besides fields for normal branch. MCDCBranch { true_bcb: BasicCoverageBlock, @@ -35,9 +37,20 @@ pub(super) struct BcbMapping { pub(super) span: Span, } +/// This is separate from [`BcbMappingKind`] to help prepare for larger changes +/// that will be needed for improved branch coverage in the future. +/// (See .) +#[derive(Debug)] +pub(super) struct BcbBranchPair { + pub(super) span: Span, + pub(super) true_bcb: BasicCoverageBlock, + pub(super) false_bcb: BasicCoverageBlock, +} + pub(super) struct CoverageSpans { bcb_has_mappings: BitSet, - mappings: Vec, + pub(super) mappings: Vec, + pub(super) branch_pairs: Vec, test_vector_bitmap_bytes: u32, } @@ -46,10 +59,6 @@ impl CoverageSpans { self.bcb_has_mappings.contains(bcb) } - pub(super) fn all_bcb_mappings(&self) -> impl Iterator { - self.mappings.iter() - } - pub(super) fn test_vector_bitmap_bytes(&self) -> u32 { self.test_vector_bitmap_bytes } @@ -65,6 +74,7 @@ pub(super) fn generate_coverage_spans( basic_coverage_blocks: &CoverageGraph, ) -> Option { let mut mappings = vec![]; + let mut branch_pairs = vec![]; if hir_info.is_async_fn { // An async function desugars into a function that returns a future, @@ -86,7 +96,7 @@ pub(super) fn generate_coverage_spans( BcbMapping { kind: BcbMappingKind::Code(bcb), span } })); - mappings.extend(from_mir::extract_branch_mappings( + branch_pairs.extend(from_mir::extract_branch_pairs( mir_body, hir_info, basic_coverage_blocks, @@ -99,7 +109,7 @@ pub(super) fn generate_coverage_spans( )); } - if mappings.is_empty() { + if mappings.is_empty() && branch_pairs.is_empty() { return None; } @@ -112,8 +122,7 @@ pub(super) fn generate_coverage_spans( for BcbMapping { kind, span: _ } in &mappings { match *kind { BcbMappingKind::Code(bcb) => insert(bcb), - BcbMappingKind::Branch { true_bcb, false_bcb } - | BcbMappingKind::MCDCBranch { true_bcb, false_bcb, .. } => { + BcbMappingKind::MCDCBranch { true_bcb, false_bcb, .. } => { insert(true_bcb); insert(false_bcb); } @@ -126,8 +135,12 @@ pub(super) fn generate_coverage_spans( } } } + for &BcbBranchPair { true_bcb, false_bcb, .. } in &branch_pairs { + insert(true_bcb); + insert(false_bcb); + } - Some(CoverageSpans { bcb_has_mappings, mappings, test_vector_bitmap_bytes }) + Some(CoverageSpans { bcb_has_mappings, mappings, branch_pairs, test_vector_bitmap_bytes }) } #[derive(Debug)] diff --git a/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs b/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs index 3c64591d43e9c..64f21d74b1cd5 100644 --- a/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs +++ b/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs @@ -13,7 +13,7 @@ use rustc_span::{ExpnKind, MacroKind, Span, Symbol}; use crate::coverage::graph::{ BasicCoverageBlock, BasicCoverageBlockData, CoverageGraph, START_BCB, }; -use crate::coverage::spans::{BcbMapping, BcbMappingKind}; +use crate::coverage::spans::{BcbBranchPair, BcbMapping, BcbMappingKind}; use crate::coverage::ExtractedHirInfo; /// Traverses the MIR body to produce an initial collection of coverage-relevant @@ -388,16 +388,16 @@ fn resolve_block_markers( } // FIXME: There is currently a lot of redundancy between -// `extract_branch_mappings` and `extract_mcdc_mappings`. This is needed so +// `extract_branch_pairs` and `extract_mcdc_mappings`. This is needed so // that they can each be modified without interfering with the other, but in // the long term we should try to bring them together again when branch coverage // and MC/DC coverage support are more mature. -pub(super) fn extract_branch_mappings( +pub(super) fn extract_branch_pairs( mir_body: &mir::Body<'_>, hir_info: &ExtractedHirInfo, basic_coverage_blocks: &CoverageGraph, -) -> Vec { +) -> Vec { let Some(branch_info) = mir_body.coverage_branch_info.as_deref() else { return vec![] }; let block_markers = resolve_block_markers(branch_info, mir_body); @@ -420,7 +420,7 @@ pub(super) fn extract_branch_mappings( let true_bcb = bcb_from_marker(true_marker)?; let false_bcb = bcb_from_marker(false_marker)?; - Some(BcbMapping { kind: BcbMappingKind::Branch { true_bcb, false_bcb }, span }) + Some(BcbBranchPair { span, true_bcb, false_bcb }) }) .collect::>() }