!! (WIP) Use the existing filename groupings in mapgen

Author: Zalathar

compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs

@@ -222,36 +222,20 @@ impl<'tcx> FunctionCoverage<'tcx> {
         self.source_hash
     }
 
-    /// 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(
-        &self,
-    ) -> (Vec<CounterExpression>, impl Iterator<Item = (Counter, &CodeRegion)>) {
+    #[track_caller]
+    fn assert_source_hash_is_set(&self) {
         assert!(
             self.source_hash != 0 || !self.is_used,
             "No counters provided the source_hash for used function: {:?}",
             self.instance
         );
-
-        let counter_expressions = self.counter_expressions();
-        // Expression IDs are indices into `self.expressions`, and on the LLVM
-        // side they will be treated as indices into `counter_expressions`, so
-        // the two vectors should correspond 1:1.
-        assert_eq!(self.expressions.len(), counter_expressions.len());
-
-        let all_mappings = self.mappings_per_file.values().flatten();
-        let counter_regions = all_mappings.map(|(mapping_kind, code_region)| {
-            let counter = Counter::from_term(*mapping_kind);
-            (counter, code_region)
-        });
-
-        (counter_expressions, counter_regions)
     }
 
     /// Convert this function's coverage expression data into a form that can be
     /// passed through FFI to LLVM.
-    fn counter_expressions(&self) -> Vec<CounterExpression> {
+    pub(crate) fn counter_expressions(&self) -> Vec<CounterExpression> {
+        self.assert_source_hash_is_set();
+
         // We know that LLVM will optimize out any unused expressions before
         // producing the final coverage map, so there's no need to do the same
         // thing on the Rust side unless we're confident we can do much better.
@@ -278,4 +262,18 @@ impl<'tcx> FunctionCoverage<'tcx> {
             })
             .collect::<Vec<_>>()
     }
+
+    pub(crate) fn has_mappings(&self) -> bool {
+        self.mappings_per_file.values().any(|vec| !vec.is_empty())
+    }
+
+    pub(crate) fn all_mappings_by_filename(
+        &self,
+    ) -> impl Iterator<Item = (Symbol, &[(CovTerm, CodeRegion)])> {
+        self.assert_source_hash_is_set();
+        self.mappings_per_file
+            .iter()
+            .map(|(file_name, mappings)| (*file_name, mappings.as_slice()))
+            .filter(|(_, mappings)| !mappings.is_empty())
+    }
 }

compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs

@@ -1,6 +1,6 @@
 use crate::common::CodegenCx;
 use crate::coverageinfo;
-use crate::coverageinfo::ffi::CounterMappingRegion;
+use crate::coverageinfo::ffi::{Counter, CounterMappingRegion};
 use crate::coverageinfo::map_data::FunctionCoverage;
 use crate::llvm;
 
@@ -159,49 +159,45 @@ fn encode_mappings_for_function(
     global_file_table: &mut GlobalFileTable,
     function_coverage: &FunctionCoverage<'_>,
 ) -> Vec<u8> {
-    let (expressions, counter_regions) = function_coverage.get_expressions_and_counter_regions();
-
-    let mut counter_regions = counter_regions.collect::<Vec<_>>();
-    if counter_regions.is_empty() {
+    if !function_coverage.has_mappings() {
         return Vec::new();
     }
 
+    let expressions = function_coverage.counter_expressions();
+
     let mut virtual_file_mapping = Vec::new();
-    let mut mapping_regions = Vec::with_capacity(counter_regions.len());
-
-    // Convert the list of (Counter, CodeRegion) pairs to an array of `CounterMappingRegion`, sorted
-    // by filename and position. Capture any new files to compute the `CounterMappingRegion`s
-    // `file_id` (indexing files referenced by the current function), and construct the
-    // function-specific `virtual_file_mapping` from `file_id` to its index in the module's
-    // `filenames` array.
-    counter_regions.sort_by_key(|(_counter, region)| region.file_name);
-    for (local_file_id, counter_regions_for_file) in
-        (0u32..).zip(counter_regions.group_by(|(_, a), (_, b)| a.file_name == b.file_name))
+    let mut mapping_regions = Vec::new();
+
+    for (local_file_id, (file_name, mappings_for_file)) in
+        (0u32..).zip(function_coverage.all_mappings_by_filename())
     {
+        assert!(!mappings_for_file.is_empty(), "{mappings_for_file:?}");
         // Look up (or allocate) the global file ID for this file name.
-        let file_name = counter_regions_for_file[0].1.file_name;
         let global_file_id = global_file_table.global_file_id_for_file_name(file_name);
 
         // Associate that global file ID with a local file ID for this function.
         assert_eq!(local_file_id as usize, virtual_file_mapping.len());
         virtual_file_mapping.push(global_file_id);
         debug!("  file_id: local {local_file_id} => global {global_file_id} = '{file_name:?}'");
 
-        // For each counter/region pair in this function+file, convert it
-        // to a form suitable for FFI.
-        for &(counter, region) in counter_regions_for_file {
+        mapping_regions.extend(mappings_for_file.iter().map(|(mapping_kind, region)| {
+            let counter = Counter::from_term(*mapping_kind);
             let CodeRegion { file_name: _, start_line, start_col, end_line, end_col } = *region;
 
             debug!("Adding counter {counter:?} to map for {region:?}");
-            mapping_regions.push(CounterMappingRegion::code_region(
+            CounterMappingRegion::code_region(
                 counter,
                 local_file_id,
                 start_line,
                 start_col,
                 end_line,
                 end_col,
-            ));
-        }
+            )
+        }));
+    }
+
+    if mapping_regions.is_empty() {
+        bug!("no mappings generated for function: {function_coverage:?}");
     }
 
     // Encode the function's coverage mappings into a buffer.