Fixes multiple issue with counters, with simplification

Includes a change to the implicit else span in ast_lowering, so coverage
of the implicit else no longer spans the `then` block.

Adds coverage for unused closures and async function bodies.

Fixes: #78542

Author: richkadel

compiler/rustc_ast_lowering/src/expr.rs

@@ -347,7 +347,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
         // `_ => else_block` where `else_block` is `{}` if there's `None`:
         let else_pat = self.pat_wild(span);
         let (else_expr, contains_else_clause) = match else_opt {
-            None => (self.expr_block_empty(span), false),
+            None => (self.expr_block_empty(span.shrink_to_hi()), false),
             Some(els) => (self.lower_expr(els), true),
         };
         let else_arm = self.arm(else_pat, else_expr);

compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs

@@ -5,7 +5,7 @@ use crate::llvm;
 use llvm::coverageinfo::CounterMappingRegion;
 use rustc_codegen_ssa::coverageinfo::map::{Counter, CounterExpression};
 use rustc_codegen_ssa::traits::ConstMethods;
-use rustc_data_structures::fx::FxIndexSet;
+use rustc_data_structures::fx::{FxHashSet, FxIndexSet};
 use rustc_llvm::RustString;
 use rustc_middle::mir::coverage::CodeRegion;
 
@@ -42,6 +42,11 @@ pub fn finalize<'ll, 'tcx>(cx: &CodegenCx<'ll, 'tcx>) {
         return;
     }
 
+    let covered_def_ids = function_coverage_map
+        .keys()
+        .map(|instance| instance.def.def_id())
+        .collect::<FxHashSet<_>>();
+
     let mut mapgen = CoverageMapGenerator::new();
 
     // Encode coverage mappings and generate function records
@@ -52,7 +57,7 @@ pub fn finalize<'ll, 'tcx>(cx: &CodegenCx<'ll, 'tcx>) {
         let mangled_function_name = cx.tcx.symbol_name(instance).to_string();
         let function_source_hash = function_coverage.source_hash();
         let (expressions, counter_regions) =
-            function_coverage.get_expressions_and_counter_regions();
+            function_coverage.get_expressions_and_counter_regions(&covered_def_ids);
 
         let coverage_mapping_buffer = llvm::build_byte_buffer(|coverage_mapping_buffer| {
             mapgen.write_coverage_mapping(expressions, counter_regions, coverage_mapping_buffer);
@@ -141,36 +146,14 @@ impl CoverageMapGenerator {
                 virtual_file_mapping.push(filenames_index as u32);
             }
             debug!("Adding counter {:?} to map for {:?}", counter, region);
-            match counter.kind {
-                rustc_codegen_ssa::coverageinfo::map::CounterKind::Zero => {
-                    // Assume `Zero` counters are either from `Unreachable` code regions, or they
-                    // should at least be handled similarly, the `gap_region` signifies a code span
-                    // that should be marked as known, but uncovered (counted as zero executions)
-                    // _except_ where other code regions overlap. For example, a closure that is
-                    // defined but never used will probably not get codegenned, and therefore would
-                    // not have any coverage, but the MIR in which the closure is defined can at
-                    // least inject the span for the closure as a `gap_region`, ensuring the code
-                    // region is at least known about, and can be flagged as uncovered if necessary.
-                    mapping_regions.push(CounterMappingRegion::gap_region(
-                        counter,
-                        current_file_id,
-                        start_line,
-                        start_col,
-                        end_line,
-                        end_col,
-                    ));
-                }
-                _ => {
-                    mapping_regions.push(CounterMappingRegion::code_region(
-                        counter,
-                        current_file_id,
-                        start_line,
-                        start_col,
-                        end_line,
-                        end_col,
-                    ));
-                }
-            }
+            mapping_regions.push(CounterMappingRegion::code_region(
+                counter,
+                current_file_id,
+                start_line,
+                start_col,
+                end_line,
+                end_col,
+            ));
         }
 
         // Encode and append the current function's coverage mapping data

compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs

@@ -15,6 +15,7 @@ use rustc_middle::mir::coverage::{
     CodeRegion, CounterValueReference, ExpressionOperandId, InjectedExpressionId, Op,
 };
 use rustc_middle::ty::Instance;
+use rustc_span::def_id::DefId;
 
 use std::cell::RefCell;
 use std::ffi::CString;
@@ -127,7 +128,12 @@ impl CoverageInfoBuilderMethods<'tcx> for Builder<'a, 'll, 'tcx> {
         }
     }
 
-    fn add_coverage_unreachable(&mut self, instance: Instance<'tcx>, region: CodeRegion) -> bool {
+    fn add_coverage_unreachable(
+        &mut self,
+        instance: Instance<'tcx>,
+        closure_def_id: Option<DefId>,
+        region: CodeRegion,
+    ) -> bool {
         if let Some(coverage_context) = self.coverage_context() {
             debug!(
                 "adding unreachable code to coverage_map: instance={:?}, at {:?}",
@@ -137,7 +143,7 @@ impl CoverageInfoBuilderMethods<'tcx> for Builder<'a, 'll, 'tcx> {
             coverage_map
                 .entry(instance)
                 .or_insert_with(|| FunctionCoverage::new(self.tcx, instance))
-                .add_unreachable_region(region);
+                .add_unreachable(closure_def_id, region);
             true
         } else {
             false

compiler/rustc_codegen_ssa/src/coverageinfo/map.rs

@@ -1,12 +1,14 @@
 pub use super::ffi::*;
 
+use rustc_data_structures::fx::FxHashSet;
 use rustc_index::vec::IndexVec;
 use rustc_middle::mir::coverage::{
     CodeRegion, CounterValueReference, ExpressionOperandId, InjectedExpressionId,
     InjectedExpressionIndex, MappedExpressionIndex, Op,
 };
 use rustc_middle::ty::Instance;
 use rustc_middle::ty::TyCtxt;
+use rustc_span::def_id::DefId;
 
 #[derive(Clone, Debug)]
 pub struct Expression {
@@ -16,6 +18,12 @@ pub struct Expression {
     region: Option<CodeRegion>,
 }
 
+#[derive(Debug)]
+struct Unreachable {
+    closure_def_id: Option<DefId>,
+    region: CodeRegion,
+}
+
 /// Collects all of the coverage regions associated with (a) injected counters, (b) counter
 /// expressions (additions or subtraction), and (c) unreachable regions (always counted as zero),
 /// for a given Function. Counters and counter expressions have non-overlapping `id`s because they
@@ -33,7 +41,7 @@ pub struct FunctionCoverage<'tcx> {
     source_hash: u64,
     counters: IndexVec<CounterValueReference, Option<CodeRegion>>,
     expressions: IndexVec<InjectedExpressionIndex, Option<Expression>>,
-    unreachable_regions: Vec<CodeRegion>,
+    unreachables: Vec<Unreachable>,
 }
 
 impl<'tcx> FunctionCoverage<'tcx> {
@@ -48,7 +56,7 @@ impl<'tcx> FunctionCoverage<'tcx> {
             source_hash: 0, // will be set with the first `add_counter()`
             counters: IndexVec::from_elem_n(None, coverageinfo.num_counters as usize),
             expressions: IndexVec::from_elem_n(None, coverageinfo.num_expressions as usize),
-            unreachable_regions: Vec::new(),
+            unreachables: Vec::new(),
         }
     }
 
@@ -100,8 +108,8 @@ impl<'tcx> FunctionCoverage<'tcx> {
     }
 
     /// Add a region that will be marked as "unreachable", with a constant "zero counter".
-    pub fn add_unreachable_region(&mut self, region: CodeRegion) {
-        self.unreachable_regions.push(region)
+    pub fn add_unreachable(&mut self, closure_def_id: Option<DefId>, region: CodeRegion) {
+        self.unreachables.push(Unreachable { closure_def_id, region })
     }
 
     /// Return the source hash, generated from the HIR node structure, and used to indicate whether
@@ -113,8 +121,9 @@ impl<'tcx> FunctionCoverage<'tcx> {
     /// Generate an array of CounterExpressions, and an iterator over all `Counter`s and their
     /// associated `Regions` (from which the LLVM-specific `CoverageMapGenerator` will create
     /// `CounterMappingRegion`s.
-    pub fn get_expressions_and_counter_regions<'a>(
+    pub fn get_expressions_and_counter_regions<'a, 'b: 'a>(
         &'a self,
+        covered_def_ids: &'b FxHashSet<DefId>,
     ) -> (Vec<CounterExpression>, impl Iterator<Item = (Counter, &'a CodeRegion)>) {
         assert!(
             self.source_hash != 0,
@@ -124,10 +133,10 @@ impl<'tcx> FunctionCoverage<'tcx> {
 
         let counter_regions = self.counter_regions();
         let (counter_expressions, expression_regions) = self.expressions_with_regions();
-        let unreachable_regions = self.unreachable_regions();
+        let unreachables = self.unreachables(covered_def_ids);
 
         let counter_regions =
-            counter_regions.chain(expression_regions.into_iter().chain(unreachable_regions));
+            counter_regions.chain(expression_regions.into_iter().chain(unreachables));
         (counter_expressions, counter_regions)
     }
 
@@ -250,8 +259,20 @@ impl<'tcx> FunctionCoverage<'tcx> {
         (counter_expressions, expression_regions.into_iter())
     }
 
-    fn unreachable_regions<'a>(&'a self) -> impl Iterator<Item = (Counter, &'a CodeRegion)> {
-        self.unreachable_regions.iter().map(|region| (Counter::zero(), region))
+    fn unreachables<'a, 'b: 'a>(
+        &'a self,
+        covered_def_ids: &'b FxHashSet<DefId>,
+    ) -> impl Iterator<Item = (Counter, &'a CodeRegion)> {
+        self.unreachables.iter().filter_map(move |unreachable| {
+            if let Some(closure_def_id) = unreachable.closure_def_id {
+                if covered_def_ids.contains(&closure_def_id) {
+                    debug!("unreachable {:?} IS COVERED AND WILL BE FILTERED", unreachable);
+                    return None;
+                }
+            }
+            debug!("unreachable {:?} IS NOT COVERED... ADDING Counter::zero()", unreachable);
+            Some((Counter::zero(), &unreachable.region))
+        })
     }
 
     fn expression_index(&self, id_descending_from_max: u32) -> InjectedExpressionIndex {

compiler/rustc_codegen_ssa/src/mir/coverageinfo.rs

@@ -36,9 +36,10 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
             CoverageKind::Expression { id, lhs, op, rhs } => {
                 bx.add_coverage_counter_expression(self.instance, id, lhs, op, rhs, code_region);
             }
-            CoverageKind::Unreachable => {
+            CoverageKind::Unreachable { closure_def_id } => {
                 bx.add_coverage_unreachable(
                     self.instance,
+                    closure_def_id,
                     code_region.expect("unreachable regions always have code regions"),
                 );
             }

compiler/rustc_codegen_ssa/src/traits/coverageinfo.rs

@@ -1,6 +1,7 @@
 use super::BackendTypes;
 use rustc_middle::mir::coverage::*;
 use rustc_middle::ty::Instance;
+use rustc_span::def_id::DefId;
 
 pub trait CoverageInfoMethods: BackendTypes {
     fn coverageinfo_finalize(&self);
@@ -41,5 +42,10 @@ pub trait CoverageInfoBuilderMethods<'tcx>: BackendTypes {
 
     /// Returns true if the region was added to the coverage map; false if `-Z instrument-coverage`
     /// is not enabled (a coverage map is not being generated).
-    fn add_coverage_unreachable(&mut self, instance: Instance<'tcx>, region: CodeRegion) -> bool;
+    fn add_coverage_unreachable(
+        &mut self,
+        instance: Instance<'tcx>,
+        closure_def_id: Option<DefId>,
+        region: CodeRegion,
+    ) -> bool;
 }

compiler/rustc_middle/src/mir/coverage.rs

@@ -1,6 +1,7 @@
 //! Metadata from source code coverage analysis and instrumentation.
 
 use rustc_macros::HashStable;
+use rustc_span::def_id::DefId;
 use rustc_span::Symbol;
 
 use std::cmp::Ord;
@@ -104,7 +105,9 @@ pub enum CoverageKind {
         op: Op,
         rhs: ExpressionOperandId,
     },
-    Unreachable,
+    Unreachable {
+        closure_def_id: Option<DefId>,
+    },
 }
 
 impl CoverageKind {
@@ -113,7 +116,7 @@ impl CoverageKind {
         match *self {
             Counter { id, .. } => ExpressionOperandId::from(id),
             Expression { id, .. } => ExpressionOperandId::from(id),
-            Unreachable => bug!("Unreachable coverage cannot be part of an expression"),
+            Unreachable { .. } => bug!("Unreachable coverage cannot be part of an expression"),
         }
     }
 
@@ -132,7 +135,10 @@ impl CoverageKind {
     }
 
     pub fn is_unreachable(&self) -> bool {
-        *self == Self::Unreachable
+        match self {
+            Self::Unreachable { .. } => true,
+            _ => false,
+        }
     }
 }
 
@@ -149,7 +155,9 @@ impl Debug for CoverageKind {
                 if *op == Op::Add { "+" } else { "-" },
                 rhs.index(),
             ),
-            Unreachable => write!(fmt, "Unreachable"),
+            Unreachable { closure_def_id } => {
+                write!(fmt, "Unreachable(closure_def_id = {:?})", closure_def_id)
+            }
         }
     }
 }

compiler/rustc_mir/src/transform/coverage/counters.rs

@@ -148,9 +148,7 @@ impl<'a> BcbCounters<'a> {
 
         let mut bcbs_with_coverage = BitSet::new_empty(num_bcbs);
         for covspan in coverage_spans {
-            if let Some(bcb) = covspan.bcb {
-                bcbs_with_coverage.insert(bcb);
-            }
+            bcbs_with_coverage.insert(covspan.bcb);
         }
 
         // Walk the `CoverageGraph`. For each `BasicCoverageBlock` node with an associated

compiler/rustc_mir/src/transform/coverage/debug.rs

@@ -657,19 +657,10 @@ fn span_viewables(
     for coverage_span in coverage_spans {
         let tooltip = coverage_span.format_coverage_statements(tcx, mir_body);
         let CoverageSpan { span, bcb, .. } = coverage_span;
-        if let Some(bcb) = *bcb {
-            let bcb_data = &basic_coverage_blocks[bcb];
-            let id = bcb_data.id();
-            let leader_bb = bcb_data.leader_bb();
-            span_viewables.push(SpanViewable { bb: leader_bb, span: *span, id, tooltip });
-        } else {
-            span_viewables.push(SpanViewable {
-                bb: mir::START_BLOCK,
-                span: *span,
-                id: String::from("Unreachable"),
-                tooltip,
-            });
-        }
+        let bcb_data = &basic_coverage_blocks[*bcb];
+        let id = bcb_data.id();
+        let leader_bb = bcb_data.leader_bb();
+        span_viewables.push(SpanViewable { bb: leader_bb, span: *span, id, tooltip });
     }
     span_viewables
 }

compiler/rustc_mir/src/transform/coverage/graph.rs

@@ -118,18 +118,8 @@ impl CoverageGraph {
 
             match term.kind {
                 TerminatorKind::Return { .. }
-                // FIXME(richkadel): Add test(s) for `Abort` coverage.
                 | TerminatorKind::Abort
-                // FIXME(richkadel): Add test(s) for `Assert` coverage.
-                // Should `Assert` be handled like `FalseUnwind` instead? Since we filter out unwind
-                // branches when creating the BCB CFG, aren't `Assert`s (without unwinds) just like
-                // `FalseUnwinds` (which are kind of like `Goto`s)?
-                | TerminatorKind::Assert { .. }
-                // FIXME(richkadel): Add test(s) for `Yield` coverage, and confirm coverage is
-                // sensible for code using the `yield` keyword.
                 | TerminatorKind::Yield { .. }
-                // FIXME(richkadel): Also add coverage tests using async/await, and threading.
-
                 | TerminatorKind::SwitchInt { .. } => {
                     // The `bb` has more than one _outgoing_ edge, or exits the function. Save the
                     // current sequence of `basic_blocks` gathered to this point, as a new
@@ -147,13 +137,21 @@ impl CoverageGraph {
                     // `Terminator`s `successors()` list) checking the number of successors won't
                     // work.
                 }
+                // The following `TerminatorKind`s are either not expected outside an unwind branch,
+                // or they should not (under normal circumstances) branch. Coverage graphs are
+                // simplified by assuring coverage results are accurate for well-behaved programs.
+                // Programs that panic and unwind may record slightly inaccurate coverage results
+                // for a coverage region containing the `Terminator` that began the panic. This
+                // is as intended. (See Issue #78544 for a possible future option to support
+                // coverage in test programs that panic.)
                 TerminatorKind::Goto { .. }
                 | TerminatorKind::Resume
                 | TerminatorKind::Unreachable
                 | TerminatorKind::Drop { .. }
                 | TerminatorKind::DropAndReplace { .. }
                 | TerminatorKind::Call { .. }
                 | TerminatorKind::GeneratorDrop
+                | TerminatorKind::Assert { .. }
                 | TerminatorKind::FalseEdge { .. }
                 | TerminatorKind::FalseUnwind { .. }
                 | TerminatorKind::InlineAsm { .. } => {}