Revert "Rollup merge of #133418 - Zalathar:spans, r=jieyouxu"

This reverts commit adf9b5fcd1de43eaf0a779e10612caee8b47bede, reversing
changes made to af1ca153d4aed5ffe22445273aa388a8d3f8f4ae.

Reverting due to <https://github.com/rust-lang/rust/issues/133606>.

5 files changed, 96 insertions, 208 deletions

diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs
index 19d6726002c..a6e07ea2a60 100644
--- a/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs
+++ b/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs
@@ -1,4 +1,6 @@
-use rustc_middle::mir::coverage::{CounterId, CovTerm, ExpressionId};
+use rustc_middle::mir::coverage::{CounterId, CovTerm, ExpressionId, SourceRegion};
+
+use crate::coverageinfo::mapgen::LocalFileId;
 
 /// Must match the layout of `LLVMRustCounterKind`.
 #[derive(Copy, Clone, Debug)]
@@ -124,23 +126,37 @@ pub(crate) struct CoverageSpan {
     /// Local index into the function's local-to-global file ID table.
     /// The value at that index is itself an index into the coverage filename
     /// table in the CGU's `__llvm_covmap` section.
-    pub(crate) file_id: u32,
+    file_id: u32,
 
     /// 1-based starting line of the source code span.
-    pub(crate) start_line: u32,
+    start_line: u32,
     /// 1-based starting column of the source code span.
-    pub(crate) start_col: u32,
+    start_col: u32,
     /// 1-based ending line of the source code span.
-    pub(crate) end_line: u32,
+    end_line: u32,
     /// 1-based ending column of the source code span. High bit must be unset.
-    pub(crate) end_col: u32,
+    end_col: u32,
+}
+
+impl CoverageSpan {
+    pub(crate) fn from_source_region(
+        local_file_id: LocalFileId,
+        code_region: &SourceRegion,
+    ) -> Self {
+        let file_id = local_file_id.as_u32();
+        let &SourceRegion { start_line, start_col, end_line, end_col } = code_region;
+        // Internally, LLVM uses the high bit of `end_col` to distinguish between
+        // code regions and gap regions, so it can't be used by the column number.
+        assert!(end_col & (1u32 << 31) == 0, "high bit of `end_col` must be unset: {end_col:#X}");
+        Self { file_id, start_line, start_col, end_line, end_col }
+    }
 }
 
 /// Must match the layout of `LLVMRustCoverageCodeRegion`.
 #[derive(Clone, Debug)]
 #[repr(C)]
 pub(crate) struct CodeRegion {
-    pub(crate) cov_span: CoverageSpan,
+    pub(crate) span: CoverageSpan,
     pub(crate) counter: Counter,
 }
 
@@ -148,7 +164,7 @@ pub(crate) struct CodeRegion {
 #[derive(Clone, Debug)]
 #[repr(C)]
 pub(crate) struct BranchRegion {
-    pub(crate) cov_span: CoverageSpan,
+    pub(crate) span: CoverageSpan,
     pub(crate) true_counter: Counter,
     pub(crate) false_counter: Counter,
 }
@@ -157,7 +173,7 @@ pub(crate) struct BranchRegion {
 #[derive(Clone, Debug)]
 #[repr(C)]
 pub(crate) struct MCDCBranchRegion {
-    pub(crate) cov_span: CoverageSpan,
+    pub(crate) span: CoverageSpan,
     pub(crate) true_counter: Counter,
     pub(crate) false_counter: Counter,
     pub(crate) mcdc_branch_params: mcdc::BranchParameters,
@@ -167,6 +183,6 @@ pub(crate) struct MCDCBranchRegion {
 #[derive(Clone, Debug)]
 #[repr(C)]
 pub(crate) struct MCDCDecisionRegion {
-    pub(crate) cov_span: CoverageSpan,
+    pub(crate) span: CoverageSpan,
     pub(crate) mcdc_decision_params: mcdc::DecisionParameters,
 }
diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs
index 95746b88ced..e3c0df27883 100644
--- a/compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs
+++ b/compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs
@@ -3,9 +3,9 @@ use rustc_data_structures::fx::FxIndexSet;
 use rustc_index::bit_set::BitSet;
 use rustc_middle::mir::coverage::{
     CounterId, CovTerm, Expression, ExpressionId, FunctionCoverageInfo, Mapping, MappingKind, Op,
+    SourceRegion,
 };
 use rustc_middle::ty::Instance;
-use rustc_span::Span;
 use tracing::{debug, instrument};
 
 use crate::coverageinfo::ffi::{Counter, CounterExpression, ExprKind};
@@ -220,16 +220,16 @@ impl<'tcx> FunctionCoverage<'tcx> {
         })
     }
 
-    /// Yields all this function's coverage mappings, after simplifying away
-    /// unused counters and counter expressions.
-    pub(crate) fn mapping_spans(
+    /// Converts this function's coverage mappings into an intermediate form
+    /// that will be used by `mapgen` when preparing for FFI.
+    pub(crate) fn counter_regions(
         &self,
-    ) -> impl Iterator<Item = (MappingKind, Span)> + ExactSizeIterator + Captures<'_> {
+    ) -> impl Iterator<Item = (MappingKind, &SourceRegion)> + ExactSizeIterator {
         self.function_coverage_info.mappings.iter().map(move |mapping| {
-            let &Mapping { ref kind, span } = mapping;
+            let Mapping { kind, source_region } = mapping;
             let kind =
                 kind.map_terms(|term| if self.is_zero_term(term) { CovTerm::Zero } else { term });
-            (kind, span)
+            (kind, source_region)
         })
     }
 
diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs
index ed881418cb0..b582dd967a7 100644
--- a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs
+++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs
@@ -1,14 +1,12 @@
-mod spans;
-
 use std::ffi::CString;
-use std::sync::Arc;
+use std::iter;
 
 use itertools::Itertools as _;
 use rustc_abi::Align;
 use rustc_codegen_ssa::traits::{
     BaseTypeCodegenMethods, ConstCodegenMethods, StaticCodegenMethods,
 };
-use rustc_data_structures::fx::{FxHashSet, FxIndexMap};
+use rustc_data_structures::fx::{FxHashSet, FxIndexMap, FxIndexSet};
 use rustc_hir::def_id::{DefId, LocalDefId};
 use rustc_index::IndexVec;
 use rustc_middle::mir::coverage::MappingKind;
@@ -17,7 +15,7 @@ use rustc_middle::{bug, mir};
 use rustc_session::RemapFileNameExt;
 use rustc_session::config::RemapPathScopeComponents;
 use rustc_span::def_id::DefIdSet;
-use rustc_span::{SourceFile, StableSourceFileId};
+use rustc_span::{Span, Symbol};
 use rustc_target::spec::HasTargetSpec;
 use tracing::debug;
 
@@ -74,11 +72,11 @@ pub(crate) fn finalize(cx: &CodegenCx<'_, '_>) {
         .map(|(instance, function_coverage)| (instance, function_coverage.into_finished()))
         .collect::<Vec<_>>();
 
-    let all_files = function_coverage_entries
+    let all_file_names = function_coverage_entries
         .iter()
         .map(|(_, fn_cov)| fn_cov.function_coverage_info.body_span)
-        .map(|span| tcx.sess.source_map().lookup_source_file(span.lo()));
-    let global_file_table = GlobalFileTable::new(all_files);
+        .map(|span| span_file_name(tcx, span));
+    let global_file_table = GlobalFileTable::new(all_file_names);
 
     // Encode all filenames referenced by coverage mappings in this CGU.
     let filenames_buffer = global_file_table.make_filenames_buffer(tcx);
@@ -105,8 +103,15 @@ pub(crate) fn finalize(cx: &CodegenCx<'_, '_>) {
             encode_mappings_for_function(tcx, &global_file_table, &function_coverage);
 
         if coverage_mapping_buffer.is_empty() {
-            debug!("function has no mappings to embed; skipping");
-            continue;
+            if function_coverage.is_used() {
+                bug!(
+                    "A used function should have had coverage mapping data but did not: {}",
+                    mangled_function_name
+                );
+            } else {
+                debug!("unused function had no coverage mapping data: {}", mangled_function_name);
+                continue;
+            }
         }
 
         if !is_used {
@@ -143,34 +148,29 @@ pub(crate) fn finalize(cx: &CodegenCx<'_, '_>) {
     }
 }
 
-/// Maps "global" (per-CGU) file ID numbers to their underlying source files.
+/// Maps "global" (per-CGU) file ID numbers to their underlying filenames.
 struct GlobalFileTable {
-    /// This "raw" table doesn't include the working dir, so a file's
+    /// This "raw" table doesn't include the working dir, so a filename's
     /// global ID is its index in this set **plus one**.
-    raw_file_table: FxIndexMap<StableSourceFileId, Arc<SourceFile>>,
+    raw_file_table: FxIndexSet<Symbol>,
 }
 
 impl GlobalFileTable {
-    fn new(all_files: impl IntoIterator<Item = Arc<SourceFile>>) -> Self {
-        // Collect all of the files into a set. Files usually come in contiguous
-        // runs, so we can dedup adjacent ones to save work.
-        let mut raw_file_table = all_files
-            .into_iter()
-            .dedup_by(|a, b| a.stable_id == b.stable_id)
-            .map(|f| (f.stable_id, f))
-            .collect::<FxIndexMap<StableSourceFileId, Arc<SourceFile>>>();
+    fn new(all_file_names: impl IntoIterator<Item = Symbol>) -> Self {
+        // Collect all of the filenames into a set. Filenames usually come in
+        // contiguous runs, so we can dedup adjacent ones to save work.
+        let mut raw_file_table = all_file_names.into_iter().dedup().collect::<FxIndexSet<Symbol>>();
 
-        // Sort the file table by its underlying filenames.
-        raw_file_table.sort_unstable_by(|_, a, _, b| {
-            Ord::cmp(&a.name, &b.name).then_with(|| Ord::cmp(&a.stable_id, &b.stable_id))
-        });
+        // Sort the file table by its actual string values, not the arbitrary
+        // ordering of its symbols.
+        raw_file_table.sort_unstable_by(|a, b| a.as_str().cmp(b.as_str()));
 
         Self { raw_file_table }
     }
 
-    fn global_file_id_for_file(&self, file: &SourceFile) -> GlobalFileId {
-        let raw_id = self.raw_file_table.get_index_of(&file.stable_id).unwrap_or_else(|| {
-            bug!("file not found in prepared global file table: {:?}", file.name);
+    fn global_file_id_for_file_name(&self, file_name: Symbol) -> GlobalFileId {
+        let raw_id = self.raw_file_table.get_index_of(&file_name).unwrap_or_else(|| {
+            bug!("file name not found in prepared global file table: {file_name}");
         });
         // The raw file table doesn't include an entry for the working dir
         // (which has ID 0), so add 1 to get the correct ID.
@@ -178,27 +178,24 @@ impl GlobalFileTable {
     }
 
     fn make_filenames_buffer(&self, tcx: TyCtxt<'_>) -> Vec<u8> {
-        let mut table = Vec::with_capacity(self.raw_file_table.len() + 1);
-
         // LLVM Coverage Mapping Format version 6 (zero-based encoded as 5)
         // requires setting the first filename to the compilation directory.
         // Since rustc generates coverage maps with relative paths, the
         // compilation directory can be combined with the relative paths
         // to get absolute paths, if needed.
-        table.push(
-            tcx.sess
-                .opts
-                .working_dir
-                .for_scope(tcx.sess, RemapPathScopeComponents::MACRO)
-                .to_string_lossy(),
-        );
-
-        // Add the regular entries after the base directory.
-        table.extend(self.raw_file_table.values().map(|file| {
-            file.name.for_scope(tcx.sess, RemapPathScopeComponents::MACRO).to_string_lossy()
-        }));
-
-        llvm_cov::write_filenames_to_buffer(table.iter().map(|f| f.as_ref()))
+        use rustc_session::RemapFileNameExt;
+        use rustc_session::config::RemapPathScopeComponents;
+        let working_dir: &str = &tcx
+            .sess
+            .opts
+            .working_dir
+            .for_scope(tcx.sess, RemapPathScopeComponents::MACRO)
+            .to_string_lossy();
+
+        // Insert the working dir at index 0, before the other filenames.
+        let filenames =
+            iter::once(working_dir).chain(self.raw_file_table.iter().map(Symbol::as_str));
+        llvm_cov::write_filenames_to_buffer(filenames)
     }
 }
 
@@ -211,7 +208,7 @@ rustc_index::newtype_index! {
     /// An index into a function's list of global file IDs. That underlying list
     /// of local-to-global mappings will be embedded in the function's record in
     /// the `__llvm_covfun` linker section.
-    struct LocalFileId {}
+    pub(crate) struct LocalFileId {}
 }
 
 /// Holds a mapping from "local" (per-function) file IDs to "global" (per-CGU)
@@ -237,6 +234,13 @@ impl VirtualFileMapping {
     }
 }
 
+fn span_file_name(tcx: TyCtxt<'_>, span: Span) -> Symbol {
+    let source_file = tcx.sess.source_map().lookup_source_file(span.lo());
+    let name =
+        source_file.name.for_scope(tcx.sess, RemapPathScopeComponents::MACRO).to_string_lossy();
+    Symbol::intern(&name)
+}
+
 /// Using the expressions and counter regions collected for a single function,
 /// generate the variable-sized payload of its corresponding `__llvm_covfun`
 /// entry. The payload is returned as a vector of bytes.
@@ -247,13 +251,11 @@ fn encode_mappings_for_function(
     global_file_table: &GlobalFileTable,
     function_coverage: &FunctionCoverage<'_>,
 ) -> Vec<u8> {
-    let mapping_spans = function_coverage.mapping_spans();
-    if mapping_spans.is_empty() {
+    let counter_regions = function_coverage.counter_regions();
+    if counter_regions.is_empty() {
         return Vec::new();
     }
 
-    let fn_cov_info = function_coverage.function_coverage_info;
-
     let expressions = function_coverage.counter_expressions().collect::<Vec<_>>();
 
     let mut virtual_file_mapping = VirtualFileMapping::default();
@@ -263,39 +265,34 @@ fn encode_mappings_for_function(
     let mut mcdc_decision_regions = vec![];
 
     // Currently a function's mappings must all be in the same file as its body span.
-    let source_map = tcx.sess.source_map();
-    let source_file = source_map.lookup_source_file(fn_cov_info.body_span.lo());
+    let file_name = span_file_name(tcx, function_coverage.function_coverage_info.body_span);
 
-    // Look up the global file ID for that file.
-    let global_file_id = global_file_table.global_file_id_for_file(&source_file);
+    // Look up the global file ID for that filename.
+    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.
     let local_file_id = virtual_file_mapping.local_id_for_global(global_file_id);
+    debug!("  file id: {local_file_id:?} => {global_file_id:?} = '{file_name:?}'");
 
-    let make_cov_span = |span| {
-        spans::make_coverage_span(local_file_id, source_map, fn_cov_info, &source_file, span)
-    };
-
-    // For each coverage mapping span in this function+file, convert it to a
+    // For each counter/region pair in this function+file, convert it to a
     // form suitable for FFI.
-    for (mapping_kind, span) in mapping_spans {
-        debug!("Adding counter {mapping_kind:?} to map for {span:?}");
-        let Some(cov_span) = make_cov_span(span) else { continue };
+    for (mapping_kind, region) in counter_regions {
+        debug!("Adding counter {mapping_kind:?} to map for {region:?}");
+        let span = ffi::CoverageSpan::from_source_region(local_file_id, region);
         match mapping_kind {
             MappingKind::Code(term) => {
-                code_regions
-                    .push(ffi::CodeRegion { cov_span, counter: ffi::Counter::from_term(term) });
+                code_regions.push(ffi::CodeRegion { span, counter: ffi::Counter::from_term(term) });
             }
             MappingKind::Branch { true_term, false_term } => {
                 branch_regions.push(ffi::BranchRegion {
-                    cov_span,
+                    span,
                     true_counter: ffi::Counter::from_term(true_term),
                     false_counter: ffi::Counter::from_term(false_term),
                 });
             }
             MappingKind::MCDCBranch { true_term, false_term, mcdc_params } => {
                 mcdc_branch_regions.push(ffi::MCDCBranchRegion {
-                    cov_span,
+                    span,
                     true_counter: ffi::Counter::from_term(true_term),
                     false_counter: ffi::Counter::from_term(false_term),
                     mcdc_branch_params: ffi::mcdc::BranchParameters::from(mcdc_params),
@@ -303,7 +300,7 @@ fn encode_mappings_for_function(
             }
             MappingKind::MCDCDecision(mcdc_decision_params) => {
                 mcdc_decision_regions.push(ffi::MCDCDecisionRegion {
-                    cov_span,
+                    span,
                     mcdc_decision_params: ffi::mcdc::DecisionParameters::from(mcdc_decision_params),
                 });
             }
diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/spans.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/spans.rs
deleted file mode 100644
index 4a7721879fd..00000000000
--- a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/spans.rs
+++ /dev/null
@@ -1,124 +0,0 @@
-use rustc_middle::mir::coverage::FunctionCoverageInfo;
-use rustc_span::source_map::SourceMap;
-use rustc_span::{BytePos, Pos, SourceFile, Span};
-use tracing::debug;
-
-use crate::coverageinfo::ffi;
-use crate::coverageinfo::mapgen::LocalFileId;
-
-/// Converts the span into its start line and column, and end line and column.
-///
-/// Line numbers and column numbers are 1-based. Unlike most column numbers emitted by
-/// the compiler, these column numbers are denoted in **bytes**, because that's what
-/// LLVM's `llvm-cov` tool expects to see in coverage maps.
-///
-/// Returns `None` if the conversion failed for some reason. This shouldn't happen,
-/// but it's hard to rule out entirely (especially in the presence of complex macros
-/// or other expansions), and if it does happen then skipping a span or function is
-/// better than an ICE or `llvm-cov` failure that the user might have no way to avoid.
-pub(crate) fn make_coverage_span(
-    file_id: LocalFileId,
-    source_map: &SourceMap,
-    fn_cov_info: &FunctionCoverageInfo,
-    file: &SourceFile,
-    span: Span,
-) -> Option<ffi::CoverageSpan> {
-    let span = ensure_non_empty_span(source_map, fn_cov_info, span)?;
-
-    let lo = span.lo();
-    let hi = span.hi();
-
-    // Column numbers need to be in bytes, so we can't use the more convenient
-    // `SourceMap` methods for looking up file coordinates.
-    let line_and_byte_column = |pos: BytePos| -> Option<(usize, usize)> {
-        let rpos = file.relative_position(pos);
-        let line_index = file.lookup_line(rpos)?;
-        let line_start = file.lines()[line_index];
-        // Line numbers and column numbers are 1-based, so add 1 to each.
-        Some((line_index + 1, (rpos - line_start).to_usize() + 1))
-    };
-
-    let (mut start_line, start_col) = line_and_byte_column(lo)?;
-    let (mut end_line, end_col) = line_and_byte_column(hi)?;
-
-    // Apply an offset so that code in doctests has correct line numbers.
-    // FIXME(#79417): Currently we have no way to offset doctest _columns_.
-    start_line = source_map.doctest_offset_line(&file.name, start_line);
-    end_line = source_map.doctest_offset_line(&file.name, end_line);
-
-    check_coverage_span(ffi::CoverageSpan {
-        file_id: file_id.as_u32(),
-        start_line: start_line as u32,
-        start_col: start_col as u32,
-        end_line: end_line as u32,
-        end_col: end_col as u32,
-    })
-}
-
-fn ensure_non_empty_span(
-    source_map: &SourceMap,
-    fn_cov_info: &FunctionCoverageInfo,
-    span: Span,
-) -> Option<Span> {
-    if !span.is_empty() {
-        return Some(span);
-    }
-
-    let lo = span.lo();
-    let hi = span.hi();
-
-    // The span is empty, so try to expand it to cover an adjacent '{' or '}',
-    // but only within the bounds of the body span.
-    let try_next = hi < fn_cov_info.body_span.hi();
-    let try_prev = fn_cov_info.body_span.lo() < lo;
-    if !(try_next || try_prev) {
-        return None;
-    }
-
-    source_map
-        .span_to_source(span, |src, start, end| try {
-            // We're only checking for specific ASCII characters, so we don't
-            // have to worry about multi-byte code points.
-            if try_next && src.as_bytes()[end] == b'{' {
-                Some(span.with_hi(hi + BytePos(1)))
-            } else if try_prev && src.as_bytes()[start - 1] == b'}' {
-                Some(span.with_lo(lo - BytePos(1)))
-            } else {
-                None
-            }
-        })
-        .ok()?
-}
-
-/// If `llvm-cov` sees a source region that is improperly ordered (end < start),
-/// it will immediately exit with a fatal error. To prevent that from happening,
-/// discard regions that are improperly ordered, or might be interpreted in a
-/// way that makes them improperly ordered.
-fn check_coverage_span(cov_span: ffi::CoverageSpan) -> Option<ffi::CoverageSpan> {
-    let ffi::CoverageSpan { file_id: _, start_line, start_col, end_line, end_col } = cov_span;
-
-    // Line/column coordinates are supposed to be 1-based. If we ever emit
-    // coordinates of 0, `llvm-cov` might misinterpret them.
-    let all_nonzero = [start_line, start_col, end_line, end_col].into_iter().all(|x| x != 0);
-    // Coverage mappings use the high bit of `end_col` to indicate that a
-    // region is actually a "gap" region, so make sure it's unset.
-    let end_col_has_high_bit_unset = (end_col & (1 << 31)) == 0;
-    // If a region is improperly ordered (end < start), `llvm-cov` will exit
-    // with a fatal error, which is inconvenient for users and hard to debug.
-    let is_ordered = (start_line, start_col) <= (end_line, end_col);
-
-    if all_nonzero && end_col_has_high_bit_unset && is_ordered {
-        Some(cov_span)
-    } else {
-        debug!(
-            ?cov_span,
-            ?all_nonzero,
-            ?end_col_has_high_bit_unset,
-            ?is_ordered,
-            "Skipping source region that would be misinterpreted or rejected by LLVM"
-        );
-        // If this happens in a debug build, ICE to make it easier to notice.
-        debug_assert!(false, "Improper source region: {cov_span:?}");
-        None
-    }
-}
diff --git a/compiler/rustc_codegen_llvm/src/lib.rs b/compiler/rustc_codegen_llvm/src/lib.rs
index 9f398107fc6..3dfb86d422d 100644
--- a/compiler/rustc_codegen_llvm/src/lib.rs
+++ b/compiler/rustc_codegen_llvm/src/lib.rs
@@ -17,7 +17,6 @@
 #![feature(iter_intersperse)]
 #![feature(let_chains)]
 #![feature(rustdoc_internals)]
-#![feature(try_blocks)]
 #![warn(unreachable_pub)]
 // tidy-alphabetical-end