Auto merge of #119754 - matthiaskrgr:rollup-7cht4m5, r=matthiaskrgr

Rollup of 10 pull requests

Successful merges:

 - #118903 (Improved support of collapse_debuginfo attribute for macros.)
 - #119033 (coverage: `llvm-cov` expects column numbers to be bytes, not code points)
 - #119598 (Fix a typo in core::ops::Deref's doc)
 - #119660 (remove an unnecessary stderr-per-bitwidth)
 - #119663 (tests: Normalize `\r\n` to `\n` in some run-make tests)
 - #119681 (coverage: Anonymize line numbers in branch views)
 - #119704 (Fix two variable binding issues in lint let_underscore)
 - #119725 (Add helper for when we want to know if an item has a host param)
 - #119738 (Add `riscv32imafc-esp-espidf` tier 3 target for the ESP32-P4.)
 - #119740 (Remove crossbeam-channel)

r? `@ghost`
`@rustbot` modify labels: rollup

Cargo.lock

@@ -3797,7 +3797,6 @@ dependencies = [
 name = "rustc_expand"
 version = "0.0.0"
 dependencies = [
- "crossbeam-channel",
  "rustc_ast",
  "rustc_ast_passes",
  "rustc_ast_pretty",

compiler/rustc_codegen_cranelift/src/debuginfo/line_info.rs

@@ -68,15 +68,7 @@ impl DebugContext {
         // In order to have a good line stepping behavior in debugger, we overwrite debug
         // locations of macro expansions with that of the outermost expansion site (when the macro is
         // annotated with `#[collapse_debuginfo]` or when `-Zdebug-macros` is provided).
-        let span = if tcx.should_collapse_debuginfo(span) {
-            span
-        } else {
-            // Walk up the macro expansion chain until we reach a non-expanded span.
-            // We also stop at the function body level because no line stepping can occur
-            // at the level above that.
-            rustc_span::hygiene::walk_chain(span, function_span.ctxt())
-        };
-
+        let span = tcx.collapsed_debuginfo(span, function_span);
         match tcx.sess.source_map().lookup_line(span.lo()) {
             Ok(SourceFileAndLine { sf: file, line }) => {
                 let line_pos = file.lines()[line];

compiler/rustc_codegen_ssa/src/mir/debuginfo.rs

@@ -228,21 +228,16 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
     /// In order to have a good line stepping behavior in debugger, we overwrite debug
     /// locations of macro expansions with that of the outermost expansion site (when the macro is
     /// annotated with `#[collapse_debuginfo]` or when `-Zdebug-macros` is provided).
-    fn adjust_span_for_debugging(&self, mut span: Span) -> Span {
+    fn adjust_span_for_debugging(&self, span: Span) -> Span {
         // Bail out if debug info emission is not enabled.
         if self.debug_context.is_none() {
             return span;
         }
-
-        if self.cx.tcx().should_collapse_debuginfo(span) {
-            // Walk up the macro expansion chain until we reach a non-expanded span.
-            // We also stop at the function body level because no line stepping can occur
-            // at the level above that.
-            // Use span of the outermost expansion site, while keeping the original lexical scope.
-            span = rustc_span::hygiene::walk_chain(span, self.mir.span.ctxt());
-        }
-
-        span
+        // Walk up the macro expansion chain until we reach a non-expanded span.
+        // We also stop at the function body level because no line stepping can occur
+        // at the level above that.
+        // Use span of the outermost expansion site, while keeping the original lexical scope.
+        self.cx.tcx().collapsed_debuginfo(span, self.mir.span)
     }
 
     fn spill_operand_to_stack(

compiler/rustc_const_eval/src/transform/check_consts/qualifs.rs

@@ -157,9 +157,7 @@ impl Qualif for NeedsNonConstDrop {
         // FIXME(effects): If `destruct` is not a `const_trait`,
         // or effects are disabled in this crate, then give up.
         let destruct_def_id = cx.tcx.require_lang_item(LangItem::Destruct, Some(cx.body.span));
-        if cx.tcx.generics_of(destruct_def_id).host_effect_index.is_none()
-            || !cx.tcx.features().effects
-        {
+        if !cx.tcx.has_host_param(destruct_def_id) || !cx.tcx.features().effects {
             return NeedsDrop::in_any_value_of_ty(cx, ty);
         }
 

compiler/rustc_expand/Cargo.toml

@@ -9,7 +9,6 @@ doctest = false
 
 [dependencies]
 # tidy-alphabetical-start
-crossbeam-channel = "0.5.0"
 rustc_ast = { path = "../rustc_ast" }
 rustc_ast_passes = { path = "../rustc_ast_passes" }
 rustc_ast_pretty = { path = "../rustc_ast_pretty" }

compiler/rustc_expand/src/proc_macro.rs

@@ -13,16 +13,16 @@ use rustc_session::config::ProcMacroExecutionStrategy;
 use rustc_span::profiling::SpannedEventArgRecorder;
 use rustc_span::{Span, DUMMY_SP};
 
-struct CrossbeamMessagePipe<T> {
-    tx: crossbeam_channel::Sender<T>,
-    rx: crossbeam_channel::Receiver<T>,
+struct MessagePipe<T> {
+    tx: std::sync::mpsc::SyncSender<T>,
+    rx: std::sync::mpsc::Receiver<T>,
 }
 
-impl<T> pm::bridge::server::MessagePipe<T> for CrossbeamMessagePipe<T> {
+impl<T> pm::bridge::server::MessagePipe<T> for MessagePipe<T> {
     fn new() -> (Self, Self) {
-        let (tx1, rx1) = crossbeam_channel::bounded(1);
-        let (tx2, rx2) = crossbeam_channel::bounded(1);
-        (CrossbeamMessagePipe { tx: tx1, rx: rx2 }, CrossbeamMessagePipe { tx: tx2, rx: rx1 })
+        let (tx1, rx1) = std::sync::mpsc::sync_channel(1);
+        let (tx2, rx2) = std::sync::mpsc::sync_channel(1);
+        (MessagePipe { tx: tx1, rx: rx2 }, MessagePipe { tx: tx2, rx: rx1 })
     }
 
     fn send(&mut self, value: T) {
@@ -35,7 +35,7 @@ impl<T> pm::bridge::server::MessagePipe<T> for CrossbeamMessagePipe<T> {
 }
 
 fn exec_strategy(ecx: &ExtCtxt<'_>) -> impl pm::bridge::server::ExecutionStrategy {
-    pm::bridge::server::MaybeCrossThread::<CrossbeamMessagePipe<_>>::new(
+    pm::bridge::server::MaybeCrossThread::<MessagePipe<_>>::new(
         ecx.sess.opts.unstable_opts.proc_macro_execution_strategy
             == ProcMacroExecutionStrategy::CrossThread,
     )

compiler/rustc_hir/src/hir.rs

@@ -1997,7 +1997,7 @@ pub enum LocalSource {
     AsyncFn,
     /// A desugared `<expr>.await`.
     AwaitDesugar,
-    /// A desugared `expr = expr`, where the LHS is a tuple, struct or array.
+    /// A desugared `expr = expr`, where the LHS is a tuple, struct, array or underscore expression.
     /// The span is that of the `=` sign.
     AssignDesugar(Span),
 }

compiler/rustc_hir_typeck/src/callee.rs

@@ -490,11 +490,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                         self.tcx.require_lang_item(hir::LangItem::FnOnce, Some(span));
                     let fn_once_output_def_id =
                         self.tcx.require_lang_item(hir::LangItem::FnOnceOutput, Some(span));
-                    if self.tcx.generics_of(fn_once_def_id).host_effect_index.is_none() {
-                        if idx == 0 && !self.tcx.is_const_fn_raw(def_id) {
-                            self.dcx().emit_err(errors::ConstSelectMustBeConst { span });
-                        }
-                    } else {
+                    if self.tcx.has_host_param(fn_once_def_id) {
                         let const_param: ty::GenericArg<'tcx> =
                             ([self.tcx.consts.false_, self.tcx.consts.true_])[idx].into();
                         self.register_predicate(traits::Obligation::new(
@@ -523,6 +519,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                         ));
 
                         self.select_obligations_where_possible(|_| {});
+                    } else if idx == 0 && !self.tcx.is_const_fn_raw(def_id) {
+                        self.dcx().emit_err(errors::ConstSelectMustBeConst { span });
                     }
                 } else {
                     self.dcx().emit_err(errors::ConstSelectMustBeFn { span, ty: arg_ty });

compiler/rustc_lint/src/let_underscore.rs

@@ -108,6 +108,10 @@ impl<'tcx> LateLintPass<'tcx> for LetUnderscore {
         if !matches!(local.pat.kind, hir::PatKind::Wild) {
             return;
         }
+
+        if matches!(local.source, rustc_hir::LocalSource::AsyncFn) {
+            return;
+        }
         if let Some(init) = local.init {
             let init_ty = cx.typeck_results().expr_ty(init);
             // If the type has a trivial Drop implementation, then it doesn't
@@ -126,6 +130,7 @@ impl<'tcx> LateLintPass<'tcx> for LetUnderscore {
                 suggestion: local.pat.span,
                 multi_suggestion_start: local.span.until(init.span),
                 multi_suggestion_end: init.span.shrink_to_hi(),
+                is_assign_desugar: matches!(local.source, rustc_hir::LocalSource::AssignDesugar(_)),
             };
             if is_sync_lock {
                 let mut span = MultiSpan::from_spans(vec![local.pat.span, init.span]);

compiler/rustc_lint/src/lints.rs

@@ -932,17 +932,19 @@ pub struct NonBindingLetSub {
     pub suggestion: Span,
     pub multi_suggestion_start: Span,
     pub multi_suggestion_end: Span,
+    pub is_assign_desugar: bool,
 }
 
 impl AddToDiagnostic for NonBindingLetSub {
     fn add_to_diagnostic_with<F>(self, diag: &mut Diagnostic, _: F)
     where
         F: Fn(&mut Diagnostic, SubdiagnosticMessage) -> SubdiagnosticMessage,
     {
+        let prefix = if self.is_assign_desugar { "let " } else { "" };
         diag.span_suggestion_verbose(
             self.suggestion,
             fluent::lint_non_binding_let_suggestion,
-            "_unused",
+            format!("{prefix}_unused"),
             Applicability::MachineApplicable,
         );
         diag.multipart_suggestion(

compiler/rustc_middle/src/ty/mod.rs

@@ -51,7 +51,7 @@ use rustc_session::lint::LintBuffer;
 pub use rustc_session::lint::RegisteredTools;
 use rustc_span::hygiene::MacroKind;
 use rustc_span::symbol::{kw, sym, Ident, Symbol};
-use rustc_span::{ExpnId, ExpnKind, Span};
+use rustc_span::{hygiene, ExpnId, ExpnKind, Span};
 use rustc_target::abi::{Align, FieldIdx, Integer, IntegerType, VariantIdx};
 pub use rustc_target::abi::{ReprFlags, ReprOptions};
 pub use rustc_type_ir::{DebugWithInfcx, InferCtxtLike, WithInfcx};
@@ -2518,21 +2518,21 @@ impl<'tcx> TyCtxt<'tcx> {
         (ident, scope)
     }
 
-    /// Returns `true` if the debuginfo for `span` should be collapsed to the outermost expansion
-    /// site. Only applies when `Span` is the result of macro expansion.
+    /// Returns corrected span if the debuginfo for `span` should be collapsed to the outermost
+    /// expansion site (with collapse_debuginfo attribute if the corresponding feature enabled).
+    /// Only applies when `Span` is the result of macro expansion.
     ///
     /// - If the `collapse_debuginfo` feature is enabled then debuginfo is not collapsed by default
-    ///   and only when a macro definition is annotated with `#[collapse_debuginfo]`.
+    ///   and only when a (some enclosing) macro definition is annotated with `#[collapse_debuginfo]`.
     /// - If `collapse_debuginfo` is not enabled, then debuginfo is collapsed by default.
     ///
     /// When `-Zdebug-macros` is provided then debuginfo will never be collapsed.
-    pub fn should_collapse_debuginfo(self, span: Span) -> bool {
-        !self.sess.opts.unstable_opts.debug_macros
-            && if self.features().collapse_debuginfo {
-                span.in_macro_expansion_with_collapse_debuginfo()
-            } else {
-                span.from_expansion()
-            }
+    pub fn collapsed_debuginfo(self, span: Span, upto: Span) -> Span {
+        if self.sess.opts.unstable_opts.debug_macros || !span.from_expansion() {
+            return span;
+        }
+        let collapse_debuginfo_enabled = self.features().collapse_debuginfo;
+        hygiene::walk_chain_collapsed(span, upto, collapse_debuginfo_enabled)
     }
 
     #[inline]

compiler/rustc_middle/src/ty/util.rs

@@ -1,7 +1,7 @@
 //! Miscellaneous type-system utilities that are too small to deserve their own modules.
 
 use crate::middle::codegen_fn_attrs::CodegenFnAttrFlags;
-use crate::query::Providers;
+use crate::query::{IntoQueryParam, Providers};
 use crate::ty::layout::IntegerExt;
 use crate::ty::{
     self, FallibleTypeFolder, ToPredicate, Ty, TyCtxt, TypeFoldable, TypeFolder, TypeSuperFoldable,
@@ -786,6 +786,13 @@ impl<'tcx> TyCtxt<'tcx> {
             || self.extern_crate(key.as_def_id()).is_some_and(|e| e.is_direct())
     }
 
+    /// Whether the item has a host effect param. This is different from `TyCtxt::is_const`,
+    /// because the item must also be "maybe const", and the crate where the item is
+    /// defined must also have the effects feature enabled.
+    pub fn has_host_param(self, def_id: impl IntoQueryParam<DefId>) -> bool {
+        self.generics_of(def_id).host_effect_index.is_some()
+    }
+
     pub fn expected_host_effect_param_for_body(self, def_id: impl Into<DefId>) -> ty::Const<'tcx> {
         let def_id = def_id.into();
         // FIXME(effects): This is suspicious and should probably not be done,

compiler/rustc_mir_transform/src/coverage/mod.rs

@@ -23,7 +23,7 @@ use rustc_middle::mir::{
 use rustc_middle::ty::TyCtxt;
 use rustc_span::def_id::LocalDefId;
 use rustc_span::source_map::SourceMap;
-use rustc_span::{Span, Symbol};
+use rustc_span::{BytePos, Pos, RelativeBytePos, Span, Symbol};
 
 /// Inserts `StatementKind::Coverage` statements that either instrument the binary with injected
 /// counters, via intrinsic `llvm.instrprof.increment`, and/or inject metadata used during codegen
@@ -107,6 +107,12 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> {
         );
 
         let mappings = self.create_mappings(&coverage_spans, &coverage_counters);
+        if mappings.is_empty() {
+            // No spans could be converted into valid mappings, so skip this function.
+            debug!("no spans could be converted into valid mappings; skipping");
+            return;
+        }
+
         self.inject_coverage_statements(bcb_has_coverage_spans, &coverage_counters);
 
         self.mir_body.function_coverage_info = Some(Box::new(FunctionCoverageInfo {
@@ -148,9 +154,9 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> {
             // Flatten the spans into individual term/span pairs.
             .flat_map(|(term, spans)| spans.iter().map(move |&span| (term, span)))
             // Convert each span to a code region, and create the final mapping.
-            .map(|(term, span)| {
-                let code_region = make_code_region(source_map, file_name, span, body_span);
-                Mapping { term, code_region }
+            .filter_map(|(term, span)| {
+                let code_region = make_code_region(source_map, file_name, span, body_span)?;
+                Some(Mapping { term, code_region })
             })
             .collect::<Vec<_>>()
     }
@@ -252,41 +258,85 @@ fn inject_statement(mir_body: &mut mir::Body<'_>, counter_kind: CoverageKind, bb
     data.statements.insert(0, statement);
 }
 
-/// Convert the Span into its file name, start line and column, and end line and column
+/// Convert the Span into its file name, 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.
 fn make_code_region(
     source_map: &SourceMap,
     file_name: Symbol,
     span: Span,
     body_span: Span,
-) -> CodeRegion {
+) -> Option<CodeRegion> {
     debug!(
         "Called make_code_region(file_name={}, span={}, body_span={})",
         file_name,
         source_map.span_to_diagnostic_string(span),
         source_map.span_to_diagnostic_string(body_span)
     );
 
-    let (file, mut start_line, mut start_col, mut end_line, mut end_col) =
-        source_map.span_to_location_info(span);
-    if span.hi() == span.lo() {
-        // Extend an empty span by one character so the region will be counted.
-        if span.hi() == body_span.hi() {
-            start_col = start_col.saturating_sub(1);
-        } else {
-            end_col = start_col + 1;
-        }
+    let lo = span.lo();
+    let hi = span.hi();
+
+    let file = source_map.lookup_source_file(lo);
+    if !file.contains(hi) {
+        debug!(?span, ?file, ?lo, ?hi, "span crosses multiple files; skipping");
+        return None;
+    }
+
+    // Column numbers need to be in bytes, so we can't use the more convenient
+    // `SourceMap` methods for looking up file coordinates.
+    let rpos_and_line_and_byte_column = |pos: BytePos| -> Option<(RelativeBytePos, 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((rpos, line_index + 1, (rpos - line_start).to_usize() + 1))
     };
-    if let Some(file) = file {
-        start_line = source_map.doctest_offset_line(&file.name, start_line);
-        end_line = source_map.doctest_offset_line(&file.name, end_line);
+
+    let (lo_rpos, mut start_line, mut start_col) = rpos_and_line_and_byte_column(lo)?;
+    let (hi_rpos, mut end_line, mut end_col) = rpos_and_line_and_byte_column(hi)?;
+
+    // If the span is empty, try to expand it horizontally by one character's
+    // worth of bytes, so that it is more visible in `llvm-cov` reports.
+    // We do this after resolving line/column numbers, so that empty spans at the
+    // end of a line get an extra column instead of wrapping to the next line.
+    if span.is_empty()
+        && body_span.contains(span)
+        && let Some(src) = &file.src
+    {
+        // Prefer to expand the end position, if it won't go outside the body span.
+        if hi < body_span.hi() {
+            let hi_rpos = hi_rpos.to_usize();
+            let nudge_bytes = src.ceil_char_boundary(hi_rpos + 1) - hi_rpos;
+            end_col += nudge_bytes;
+        } else if lo > body_span.lo() {
+            let lo_rpos = lo_rpos.to_usize();
+            let nudge_bytes = lo_rpos - src.floor_char_boundary(lo_rpos - 1);
+            // Subtract the nudge, but don't go below column 1.
+            start_col = start_col.saturating_sub(nudge_bytes).max(1);
+        }
+        // If neither nudge could be applied, stick with the empty span coordinates.
     }
-    CodeRegion {
+
+    // 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);
+
+    Some(CodeRegion {
         file_name,
         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 is_eligible_for_coverage(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool {

compiler/rustc_mir_transform/src/lib.rs

@@ -9,6 +9,7 @@
 #![feature(min_specialization)]
 #![feature(never_type)]
 #![feature(option_get_or_insert_default)]
+#![feature(round_char_boundary)]
 #![feature(trusted_step)]
 #![feature(try_blocks)]
 #![feature(yeet_expr)]