Remove inherent methods from coverageinfo::ffi::Counter

This patch also moves `Regions` to a different module, since that type can stay
put when the FFI bindings move to another crate.

Author: Zalathar

compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs

@@ -1,5 +1,3 @@
-use rustc_middle::mir::coverage::{CounterId, CovTerm, ExpressionId};
-
 /// Must match the layout of `LLVMRustCounterKind`.
 #[derive(Copy, Clone, Debug)]
 #[repr(C)]
@@ -26,30 +24,12 @@ pub(crate) enum CounterKind {
 pub(crate) struct Counter {
     // Important: The layout (order and types of fields) must match its C++ counterpart.
     pub(crate) kind: CounterKind,
-    id: u32,
+    pub(crate) id: u32,
 }
 
 impl Counter {
     /// A `Counter` of kind `Zero`. For this counter kind, the `id` is not used.
     pub(crate) const ZERO: Self = Self { kind: CounterKind::Zero, id: 0 };
-
-    /// Constructs a new `Counter` of kind `CounterValueReference`.
-    pub(crate) fn counter_value_reference(counter_id: CounterId) -> Self {
-        Self { kind: CounterKind::CounterValueReference, id: counter_id.as_u32() }
-    }
-
-    /// Constructs a new `Counter` of kind `Expression`.
-    pub(crate) fn expression(expression_id: ExpressionId) -> Self {
-        Self { kind: CounterKind::Expression, id: expression_id.as_u32() }
-    }
-
-    pub(crate) fn from_term(term: CovTerm) -> Self {
-        match term {
-            CovTerm::Zero => Self::ZERO,
-            CovTerm::Counter(id) => Self::counter_value_reference(id),
-            CovTerm::Expression(id) => Self::expression(id),
-        }
-    }
 }
 
 /// Corresponds to enum `llvm::coverage::CounterExpression::ExprKind`.
@@ -94,29 +74,6 @@ pub(crate) struct CoverageSpan {
     pub(crate) end_col: u32,
 }
 
-/// Holds tables of the various region types in one struct.
-///
-/// Don't pass this struct across FFI; pass the individual region tables as
-/// pointer/length pairs instead.
-///
-/// Each field name has a `_regions` suffix for improved readability after
-/// exhaustive destructing, which ensures that all region types are handled.
-#[derive(Clone, Debug, Default)]
-pub(crate) struct Regions {
-    pub(crate) code_regions: Vec<CodeRegion>,
-    pub(crate) expansion_regions: Vec<ExpansionRegion>,
-    pub(crate) branch_regions: Vec<BranchRegion>,
-}
-
-impl Regions {
-    /// Returns true if none of this structure's tables contain any regions.
-    pub(crate) fn has_no_regions(&self) -> bool {
-        let Self { code_regions, expansion_regions, branch_regions } = self;
-
-        code_regions.is_empty() && expansion_regions.is_empty() && branch_regions.is_empty()
-    }
-}
-
 /// Must match the layout of `LLVMRustCoverageCodeRegion`.
 #[derive(Clone, Debug)]
 #[repr(C)]

compiler/rustc_codegen_llvm/src/coverageinfo/llvm_cov.rs

@@ -57,12 +57,35 @@ pub(crate) fn write_filenames_to_buffer(filenames: &[impl AsRef<str>]) -> Vec<u8
     })
 }
 
+/// Holds tables of the various region types in one struct.
+///
+/// Don't pass this struct across FFI; pass the individual region tables as
+/// pointer/length pairs instead.
+///
+/// Each field name has a `_regions` suffix for improved readability after
+/// exhaustive destructing, which ensures that all region types are handled.
+#[derive(Clone, Debug, Default)]
+pub(crate) struct Regions {
+    pub(crate) code_regions: Vec<ffi::CodeRegion>,
+    pub(crate) expansion_regions: Vec<ffi::ExpansionRegion>,
+    pub(crate) branch_regions: Vec<ffi::BranchRegion>,
+}
+
+impl Regions {
+    /// Returns true if none of this structure's tables contain any regions.
+    pub(crate) fn has_no_regions(&self) -> bool {
+        let Self { code_regions, expansion_regions, branch_regions } = self;
+
+        code_regions.is_empty() && expansion_regions.is_empty() && branch_regions.is_empty()
+    }
+}
+
 pub(crate) fn write_function_mappings_to_buffer(
     virtual_file_mapping: &[u32],
     expressions: &[ffi::CounterExpression],
-    regions: &ffi::Regions,
+    regions: &Regions,
 ) -> Vec<u8> {
-    let ffi::Regions { code_regions, expansion_regions, branch_regions } = regions;
+    let Regions { code_regions, expansion_regions, branch_regions } = regions;
 
     // SAFETY:
     // - All types are FFI-compatible and have matching representations in Rust/C++.

compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs

@@ -10,8 +10,8 @@ use std::sync::Arc;
 use rustc_abi::Align;
 use rustc_codegen_ssa::traits::{BaseTypeCodegenMethods as _, ConstCodegenMethods};
 use rustc_middle::mir::coverage::{
-    BasicCoverageBlock, CovTerm, CoverageIdsInfo, Expression, FunctionCoverageInfo, Mapping,
-    MappingKind, Op,
+    BasicCoverageBlock, CounterId, CovTerm, CoverageIdsInfo, Expression, ExpressionId,
+    FunctionCoverageInfo, Mapping, MappingKind, Op,
 };
 use rustc_middle::ty::{Instance, TyCtxt};
 use rustc_span::{SourceFile, Span};
@@ -36,7 +36,7 @@ pub(crate) struct CovfunRecord<'tcx> {
 
     virtual_file_mapping: VirtualFileMapping,
     expressions: Vec<ffi::CounterExpression>,
-    regions: ffi::Regions,
+    regions: llvm_cov::Regions,
 }
 
 impl<'tcx> CovfunRecord<'tcx> {
@@ -64,7 +64,7 @@ pub(crate) fn prepare_covfun_record<'tcx>(
         is_used,
         virtual_file_mapping: VirtualFileMapping::default(),
         expressions,
-        regions: ffi::Regions::default(),
+        regions: llvm_cov::Regions::default(),
     };
 
     fill_region_tables(tcx, fn_cov_info, ids_info, &mut covfun);
@@ -77,10 +77,21 @@ pub(crate) fn prepare_covfun_record<'tcx>(
     Some(covfun)
 }
 
+pub(crate) fn counter_for_term(term: CovTerm) -> ffi::Counter {
+    use ffi::Counter;
+    match term {
+        CovTerm::Zero => Counter::ZERO,
+        CovTerm::Counter(id) => {
+            Counter { kind: ffi::CounterKind::CounterValueReference, id: CounterId::as_u32(id) }
+        }
+        CovTerm::Expression(id) => {
+            Counter { kind: ffi::CounterKind::Expression, id: ExpressionId::as_u32(id) }
+        }
+    }
+}
+
 /// Convert the function's coverage-counter expressions into a form suitable for FFI.
 fn prepare_expressions(ids_info: &CoverageIdsInfo) -> Vec<ffi::CounterExpression> {
-    let counter_for_term = ffi::Counter::from_term;
-
     // 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.
@@ -113,7 +124,7 @@ fn fill_region_tables<'tcx>(
         } else {
             CovTerm::Zero
         };
-        ffi::Counter::from_term(term)
+        counter_for_term(term)
     };
 
     // Currently a function's mappings must all be in the same file, so use the
@@ -136,7 +147,7 @@ fn fill_region_tables<'tcx>(
         if discard_all { None } else { spans::make_coords(source_map, &source_file, span) }
     };
 
-    let ffi::Regions {
+    let llvm_cov::Regions {
         code_regions,
         expansion_regions: _, // FIXME(Zalathar): Fill out support for expansion regions
         branch_regions,