diff options
| author | Stuart Cook <Zalathar@users.noreply.github.com> | 2025-07-29 23:50:35 +1000 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2025-07-29 23:50:35 +1000 |
| commit | f8370285cf81e8af24c7f95b77cbb83c3d6db1cf (patch) | |
| tree | bbe35d24a1ca049bb13fb494e3004152d6545b1f /compiler/rustc_mir_transform | |
| parent | 7278554d82fa474a4e8b5c67afb009e11e41a841 (diff) | |
| parent | 682f744f89e1c014e91b2fd7058c75f2056ac870 (diff) | |
| download | rust-f8370285cf81e8af24c7f95b77cbb83c3d6db1cf.tar.gz rust-f8370285cf81e8af24c7f95b77cbb83c3d6db1cf.zip | |
Rollup merge of #144560 - Zalathar:auto-derived, r=compiler-errors
coverage: Treat `#[automatically_derived]` as `#[coverage(off)]` One of the contributing factors behind https://github.com/rust-lang/rust/issues/141577#issuecomment-3120667286 was the presence of derive-macro-generated code containing nested closures. Coverage instrumentation already has a heuristic for skipping code marked with `#[automatically_derived]` (rust-lang/rust#120185), because derived code is usually not worth instrumenting, and also has a tendency to trigger vexing edge-case bugs in coverage instrumentation or coverage codegen. However, the existing heuristic only applied to the associated items directly within an auto-derived impl block, and had no effect on closures or nested items within those associated items. This PR therefore extends the search for `#[coverage(..)]` attributes to also treat `#[automatically_derived]` as an implied `#[coverage(off)]` for the purposes of coverage instrumentation. --- This change doesn’t rule out an entire category of bugs, because it only affects code that actually uses the auto-derived attribute. But it should reduce the overall chance of edge-case macro span bugs being observed in the wild.
Diffstat (limited to 'compiler/rustc_mir_transform')
| -rw-r--r-- | compiler/rustc_mir_transform/src/coverage/query.rs | 48 |
1 files changed, 25 insertions, 23 deletions
diff --git a/compiler/rustc_mir_transform/src/coverage/query.rs b/compiler/rustc_mir_transform/src/coverage/query.rs index 551f720c869..73795e47d2e 100644 --- a/compiler/rustc_mir_transform/src/coverage/query.rs +++ b/compiler/rustc_mir_transform/src/coverage/query.rs @@ -1,4 +1,4 @@ -use rustc_attr_data_structures::{AttributeKind, CoverageStatus, find_attr}; +use rustc_attr_data_structures::{AttributeKind, CoverageAttrKind, find_attr}; use rustc_index::bit_set::DenseBitSet; use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags; use rustc_middle::mir::coverage::{BasicCoverageBlock, CoverageIdsInfo, CoverageKind, MappingKind}; @@ -32,16 +32,6 @@ fn is_eligible_for_coverage(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool { return false; } - // Don't instrument functions with `#[automatically_derived]` on their - // enclosing impl block, on the assumption that most users won't care about - // coverage for derived impls. - if let Some(impl_of) = tcx.impl_of_assoc(def_id.to_def_id()) - && tcx.is_automatically_derived(impl_of) - { - trace!("InstrumentCoverage skipped for {def_id:?} (automatically derived)"); - return false; - } - if tcx.codegen_fn_attrs(def_id).flags.contains(CodegenFnAttrFlags::NAKED) { trace!("InstrumentCoverage skipped for {def_id:?} (`#[naked]`)"); return false; @@ -57,20 +47,32 @@ fn is_eligible_for_coverage(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool { /// Query implementation for `coverage_attr_on`. fn coverage_attr_on(tcx: TyCtxt<'_>, def_id: LocalDefId) -> bool { - // Check for annotations directly on this def. - if let Some(coverage_status) = - find_attr!(tcx.get_all_attrs(def_id), AttributeKind::Coverage(_, status) => status) + // Check for a `#[coverage(..)]` attribute on this def. + if let Some(kind) = + find_attr!(tcx.get_all_attrs(def_id), AttributeKind::Coverage(_sp, kind) => kind) { - *coverage_status == CoverageStatus::On - } else { - match tcx.opt_local_parent(def_id) { - // Check the parent def (and so on recursively) until we find an - // enclosing attribute or reach the crate root. - Some(parent) => tcx.coverage_attr_on(parent), - // We reached the crate root without seeing a coverage attribute, so - // allow coverage instrumentation by default. - None => true, + match kind { + CoverageAttrKind::On => return true, + CoverageAttrKind::Off => return false, } + }; + + // Treat `#[automatically_derived]` as an implied `#[coverage(off)]`, on + // the assumption that most users won't want coverage for derived impls. + // + // This affects not just the associated items of an impl block, but also + // any closures and other nested functions within those associated items. + if tcx.is_automatically_derived(def_id.to_def_id()) { + return false; + } + + // Check the parent def (and so on recursively) until we find an + // enclosing attribute or reach the crate root. + match tcx.opt_local_parent(def_id) { + Some(parent) => tcx.coverage_attr_on(parent), + // We reached the crate root without seeing a coverage attribute, so + // allow coverage instrumentation by default. + None => true, } } |
