coverage: Unbox and simplify `bcb_filtered_successors` #116589

Zalathar · 2023-10-10T04:47:34Z

This is a small cleanup in the coverage instrumentor's graph-building code.

This function already has access to the MIR body, so instead of taking a reference to a terminator, it's simpler and easier to pass in a basic block index.

There is no need to box the returned iterator if we instead add appropriate lifetime captures, and make short_circuit_preorder generic over the type of iterator it expects.

We can also greatly simplify the function's implementation by observing that the only difference between its two cases is whether we take all of a BB's successors, or just the first one.

@rustbot label +A-code-coverage

rustbot · 2023-10-10T04:47:41Z

r? @cjgillot

(rustbot has picked a reviewer for you, use r? to override)

rustbot · 2023-10-10T04:47:44Z

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

Zalathar · 2023-10-10T04:57:43Z

compiler/rustc_mir_transform/src/coverage/graph.rs

-
-    #[inline(always)]
-    pub fn terminator<'a, 'tcx>(&self, mir_body: &'a mir::Body<'tcx>) -> &'a Terminator<'tcx> {
-        &mir_body[self.last_bb()].terminator()
-    }


With the change from passing in a terminator ref to just passing in a basic block index, this method ended up being used only from one test, so it was less hassle to just inline and delete it.

Zalathar · 2023-10-10T05:12:44Z

Hmm, I can probably also replace ShortCircuitPreorder with std::iter::from_fn; I'll see how that looks.

Zalathar · 2023-10-10T05:36:51Z

Yep, that's much nicer.

Instead of defining a named struct, we can use `std::iter::from_fn` and store intermediate state in a closure.

This function already has access to the MIR body, so instead of taking a reference to a terminator, it's simpler and easier to pass in a basic block index. There is no need to box the returned iterator if we instead add appropriate lifetime captures, since `short_circuit_preorder` is now generic over the type of iterator it expects. We can also greatly simplify the function's implementation by observing that the only difference between its two cases is whether we take all of a BB's successors, or just the first one.

oli-obk · 2023-10-10T09:26:43Z

@bors r+ rollup

bors · 2023-10-10T09:26:46Z

📌 Commit 5d62945 has been approved by oli-obk

It is now in the queue for this repository.

…iaskrgr Rollup of 4 pull requests Successful merges: - rust-lang#116586 (use env variable to control thread ids in rustc_log) - rust-lang#116589 (coverage: Unbox and simplify `bcb_filtered_successors`) - rust-lang#116595 (-Zmir-enable-passes: document that this may enable unsound passes) - rust-lang#116596 (reorder files in solve) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#116589 - Zalathar:successors, r=oli-obk coverage: Unbox and simplify `bcb_filtered_successors` This is a small cleanup in the coverage instrumentor's graph-building code. --- This function already has access to the MIR body, so instead of taking a reference to a terminator, it's simpler and easier to pass in a basic block index. There is no need to box the returned iterator if we instead add appropriate lifetime captures, and make `short_circuit_preorder` generic over the type of iterator it expects. We can also greatly simplify the function's implementation by observing that the only difference between its two cases is whether we take all of a BB's successors, or just the first one. --- `@rustbot` label +A-code-coverage

rustbot assigned cjgillot Oct 10, 2023

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 10, 2023

rustbot added the A-code-coverage Area: Source-based code coverage (-Cinstrument-coverage) label Oct 10, 2023

Zalathar force-pushed the successors branch from 7555538 to f155194 Compare October 10, 2023 04:54

Zalathar commented Oct 10, 2023

View reviewed changes

Zalathar force-pushed the successors branch from f155194 to 248b9fc Compare October 10, 2023 05:36

Zalathar added 2 commits October 10, 2023 18:44

coverage: Replace ShortCircuitPreorder with a single function

f214497

Instead of defining a named struct, we can use `std::iter::from_fn` and store intermediate state in a closure.

Zalathar force-pushed the successors branch from 248b9fc to 5d62945 Compare October 10, 2023 07:46

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 10, 2023

matthiaskrgr mentioned this pull request Oct 10, 2023

Rollup of 4 pull requests #116598

Merged

Swatinem approved these changes Oct 10, 2023

View reviewed changes

bors merged commit f4de82c into rust-lang:master Oct 10, 2023
11 checks passed

rustbot added this to the 1.75.0 milestone Oct 10, 2023

Zalathar deleted the successors branch October 10, 2023 18:39

Dajamante mentioned this pull request Oct 12, 2023

Pull upstream master 2023 10 12 ferrocene/ferrocene#48

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

coverage: Unbox and simplify `bcb_filtered_successors` #116589

coverage: Unbox and simplify `bcb_filtered_successors` #116589

Zalathar commented Oct 10, 2023 •

edited

Loading

rustbot commented Oct 10, 2023

rustbot commented Oct 10, 2023

Zalathar Oct 10, 2023 •

edited

Loading

Zalathar commented Oct 10, 2023

Zalathar commented Oct 10, 2023

oli-obk commented Oct 10, 2023

bors commented Oct 10, 2023

coverage: Unbox and simplify bcb_filtered_successors #116589

coverage: Unbox and simplify bcb_filtered_successors #116589

Conversation

Zalathar commented Oct 10, 2023 • edited Loading

rustbot commented Oct 10, 2023

rustbot commented Oct 10, 2023

Zalathar Oct 10, 2023 • edited Loading

Choose a reason for hiding this comment

Zalathar commented Oct 10, 2023

Zalathar commented Oct 10, 2023

oli-obk commented Oct 10, 2023

bors commented Oct 10, 2023

coverage: Unbox and simplify `bcb_filtered_successors` #116589

coverage: Unbox and simplify `bcb_filtered_successors` #116589

Zalathar commented Oct 10, 2023 •

edited

Loading

Zalathar Oct 10, 2023 •

edited

Loading