Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Commit 10a7aa1

Browse files
committed
Auto merge of rust-lang#123128 - GuillaumeGomez:rollup-3l3zu6s, r=GuillaumeGomez
Rollup of 6 pull requests Successful merges: - rust-lang#121843 (Implement `-L KIND=`@RUSTC_BUILTIN/...`)` - rust-lang#122860 (coverage: Re-enable `UnreachablePropagation` for coverage builds) - rust-lang#123021 (Make `TyCtxt::coroutine_layout` take coroutine's kind parameter) - rust-lang#123024 (CFI: Enable KCFI testing of run-pass tests) - rust-lang#123083 (lib: fix some unnecessary_cast clippy lint) - rust-lang#123116 (rustdoc: Swap fields and variant documentations) r? `@ghost` `@rustbot` modify labels: rollup
2 parents 0dcc130 + c0945f0 commit 10a7aa1

File tree

28 files changed

+265
-161
lines changed

28 files changed

+265
-161
lines changed

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

Lines changed: 92 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,8 @@ use crate::llvm;
66

77
use itertools::Itertools as _;
88
use rustc_codegen_ssa::traits::{BaseTypeMethods, ConstMethods};
9-
use rustc_data_structures::fx::{FxIndexMap, FxIndexSet};
10-
use rustc_hir::def::DefKind;
11-
use rustc_hir::def_id::DefId;
9+
use rustc_data_structures::fx::{FxHashSet, FxIndexMap, FxIndexSet};
10+
use rustc_hir::def_id::{DefId, LocalDefId};
1211
use rustc_index::IndexVec;
1312
use rustc_middle::bug;
1413
use rustc_middle::mir;
@@ -335,16 +334,9 @@ fn save_function_record(
335334
);
336335
}
337336

338-
/// When finalizing the coverage map, `FunctionCoverage` only has the `CodeRegion`s and counters for
339-
/// the functions that went through codegen; such as public functions and "used" functions
340-
/// (functions referenced by other "used" or public items). Any other functions considered unused,
341-
/// or "Unreachable", were still parsed and processed through the MIR stage, but were not
342-
/// codegenned. (Note that `-Clink-dead-code` can force some unused code to be codegenned, but
343-
/// that flag is known to cause other errors, when combined with `-C instrument-coverage`; and
344-
/// `-Clink-dead-code` will not generate code for unused generic functions.)
345-
///
346-
/// We can find the unused functions (including generic functions) by the set difference of all MIR
347-
/// `DefId`s (`tcx` query `mir_keys`) minus the codegenned `DefId`s (`codegenned_and_inlined_items`).
337+
/// Each CGU will normally only emit coverage metadata for the functions that it actually generates.
338+
/// But since we don't want unused functions to disappear from coverage reports, we also scan for
339+
/// functions that were instrumented but are not participating in codegen.
348340
///
349341
/// These unused functions don't need to be codegenned, but we do need to add them to the function
350342
/// coverage map (in a single designated CGU) so that we still emit coverage mappings for them.
@@ -354,75 +346,109 @@ fn add_unused_functions(cx: &CodegenCx<'_, '_>) {
354346
assert!(cx.codegen_unit.is_code_coverage_dead_code_cgu());
355347

356348
let tcx = cx.tcx;
349+
let usage = prepare_usage_sets(tcx);
350+
351+
let is_unused_fn = |def_id: LocalDefId| -> bool {
352+
let def_id = def_id.to_def_id();
353+
354+
// To be eligible for "unused function" mappings, a definition must:
355+
// - Be function-like
356+
// - Not participate directly in codegen (or have lost all its coverage statements)
357+
// - Not have any coverage statements inlined into codegenned functions
358+
tcx.def_kind(def_id).is_fn_like()
359+
&& (!usage.all_mono_items.contains(&def_id)
360+
|| usage.missing_own_coverage.contains(&def_id))
361+
&& !usage.used_via_inlining.contains(&def_id)
362+
};
357363

358-
let eligible_def_ids = tcx.mir_keys(()).iter().filter_map(|local_def_id| {
359-
let def_id = local_def_id.to_def_id();
360-
let kind = tcx.def_kind(def_id);
361-
// `mir_keys` will give us `DefId`s for all kinds of things, not
362-
// just "functions", like consts, statics, etc. Filter those out.
363-
if !matches!(kind, DefKind::Fn | DefKind::AssocFn | DefKind::Closure) {
364-
return None;
365-
}
364+
// Scan for unused functions that were instrumented for coverage.
365+
for def_id in tcx.mir_keys(()).iter().copied().filter(|&def_id| is_unused_fn(def_id)) {
366+
// Get the coverage info from MIR, skipping functions that were never instrumented.
367+
let body = tcx.optimized_mir(def_id);
368+
let Some(function_coverage_info) = body.function_coverage_info.as_deref() else { continue };
366369

367370
// FIXME(79651): Consider trying to filter out dummy instantiations of
368371
// unused generic functions from library crates, because they can produce
369372
// "unused instantiation" in coverage reports even when they are actually
370373
// used by some downstream crate in the same binary.
371374

372-
Some(local_def_id.to_def_id())
373-
});
374-
375-
let codegenned_def_ids = codegenned_and_inlined_items(tcx);
376-
377-
// For each `DefId` that should have coverage instrumentation but wasn't
378-
// codegenned, add it to the function coverage map as an unused function.
379-
for def_id in eligible_def_ids.filter(|id| !codegenned_def_ids.contains(id)) {
380-
// Skip any function that didn't have coverage data added to it by the
381-
// coverage instrumentor.
382-
let body = tcx.instance_mir(ty::InstanceDef::Item(def_id));
383-
let Some(function_coverage_info) = body.function_coverage_info.as_deref() else {
384-
continue;
385-
};
386-
387375
debug!("generating unused fn: {def_id:?}");
388-
let instance = declare_unused_fn(tcx, def_id);
389-
add_unused_function_coverage(cx, instance, function_coverage_info);
376+
add_unused_function_coverage(cx, def_id, function_coverage_info);
390377
}
391378
}
392379

393-
/// All items participating in code generation together with (instrumented)
394-
/// items inlined into them.
395-
fn codegenned_and_inlined_items(tcx: TyCtxt<'_>) -> DefIdSet {
396-
let (items, cgus) = tcx.collect_and_partition_mono_items(());
397-
let mut visited = DefIdSet::default();
398-
let mut result = items.clone();
399-
400-
for cgu in cgus {
401-
for item in cgu.items().keys() {
402-
if let mir::mono::MonoItem::Fn(ref instance) = item {
403-
let did = instance.def_id();
404-
if !visited.insert(did) {
405-
continue;
406-
}
407-
let body = tcx.instance_mir(instance.def);
408-
for block in body.basic_blocks.iter() {
409-
for statement in &block.statements {
410-
let mir::StatementKind::Coverage(_) = statement.kind else { continue };
411-
let scope = statement.source_info.scope;
412-
if let Some(inlined) = scope.inlined_instance(&body.source_scopes) {
413-
result.insert(inlined.def_id());
414-
}
415-
}
416-
}
380+
struct UsageSets<'tcx> {
381+
all_mono_items: &'tcx DefIdSet,
382+
used_via_inlining: FxHashSet<DefId>,
383+
missing_own_coverage: FxHashSet<DefId>,
384+
}
385+
386+
/// Prepare sets of definitions that are relevant to deciding whether something
387+
/// is an "unused function" for coverage purposes.
388+
fn prepare_usage_sets<'tcx>(tcx: TyCtxt<'tcx>) -> UsageSets<'tcx> {
389+
let (all_mono_items, cgus) = tcx.collect_and_partition_mono_items(());
390+
391+
// Obtain a MIR body for each function participating in codegen, via an
392+
// arbitrary instance.
393+
let mut def_ids_seen = FxHashSet::default();
394+
let def_and_mir_for_all_mono_fns = cgus
395+
.iter()
396+
.flat_map(|cgu| cgu.items().keys())
397+
.filter_map(|item| match item {
398+
mir::mono::MonoItem::Fn(instance) => Some(instance),
399+
mir::mono::MonoItem::Static(_) | mir::mono::MonoItem::GlobalAsm(_) => None,
400+
})
401+
// We only need one arbitrary instance per definition.
402+
.filter(move |instance| def_ids_seen.insert(instance.def_id()))
403+
.map(|instance| {
404+
// We don't care about the instance, just its underlying MIR.
405+
let body = tcx.instance_mir(instance.def);
406+
(instance.def_id(), body)
407+
});
408+
409+
// Functions whose coverage statments were found inlined into other functions.
410+
let mut used_via_inlining = FxHashSet::default();
411+
// Functions that were instrumented, but had all of their coverage statements
412+
// removed by later MIR transforms (e.g. UnreachablePropagation).
413+
let mut missing_own_coverage = FxHashSet::default();
414+
415+
for (def_id, body) in def_and_mir_for_all_mono_fns {
416+
let mut saw_own_coverage = false;
417+
418+
// Inspect every coverage statement in the function's MIR.
419+
for stmt in body
420+
.basic_blocks
421+
.iter()
422+
.flat_map(|block| &block.statements)
423+
.filter(|stmt| matches!(stmt.kind, mir::StatementKind::Coverage(_)))
424+
{
425+
if let Some(inlined) = stmt.source_info.scope.inlined_instance(&body.source_scopes) {
426+
// This coverage statement was inlined from another function.
427+
used_via_inlining.insert(inlined.def_id());
428+
} else {
429+
// Non-inlined coverage statements belong to the enclosing function.
430+
saw_own_coverage = true;
417431
}
418432
}
433+
434+
if !saw_own_coverage && body.function_coverage_info.is_some() {
435+
missing_own_coverage.insert(def_id);
436+
}
419437
}
420438

421-
result
439+
UsageSets{ all_mono_items, used_via_inlining, missing_own_coverage }
422440
}
423441

424-
fn declare_unused_fn<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId) -> ty::Instance<'tcx> {
425-
ty::Instance::new(
442+
fn add_unused_function_coverage<'tcx>(
443+
cx: &CodegenCx<'_, 'tcx>,
444+
def_id: LocalDefId,
445+
function_coverage_info: &'tcx mir::coverage::FunctionCoverageInfo,
446+
) {
447+
let tcx = cx.tcx;
448+
let def_id = def_id.to_def_id();
449+
450+
// Make a dummy instance that fills in all generics with placeholders.
451+
let instance = ty::Instance::new(
426452
def_id,
427453
ty::GenericArgs::for_item(tcx, def_id, |param, _| {
428454
if let ty::GenericParamDefKind::Lifetime = param.kind {
@@ -431,14 +457,8 @@ fn declare_unused_fn<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId) -> ty::Instance<'tc
431457
tcx.mk_param_from_def(param)
432458
}
433459
}),
434-
)
435-
}
460+
);
436461

437-
fn add_unused_function_coverage<'tcx>(
438-
cx: &CodegenCx<'_, 'tcx>,
439-
instance: ty::Instance<'tcx>,
440-
function_coverage_info: &'tcx mir::coverage::FunctionCoverageInfo,
441-
) {
442462
// An unused function's mappings will automatically be rewritten to map to
443463
// zero, because none of its counters/expressions are marked as seen.
444464
let function_coverage = FunctionCoverageCollector::unused(instance, function_coverage_info);

‎compiler/rustc_codegen_llvm/src/debuginfo/metadata/enums/cpp_like.rs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -683,7 +683,8 @@ fn build_union_fields_for_direct_tag_coroutine<'ll, 'tcx>(
683683
_ => unreachable!(),
684684
};
685685

686-
let coroutine_layout = cx.tcx.optimized_mir(coroutine_def_id).coroutine_layout().unwrap();
686+
let coroutine_layout =
687+
cx.tcx.coroutine_layout(coroutine_def_id, coroutine_args.kind_ty()).unwrap();
687688

688689
let common_upvar_names = cx.tcx.closure_saved_names_of_captured_variables(coroutine_def_id);
689690
let variant_range = coroutine_args.variant_range(coroutine_def_id, cx.tcx);

‎compiler/rustc_codegen_llvm/src/debuginfo/metadata/enums/native.rs‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ pub(super) fn build_coroutine_di_node<'ll, 'tcx>(
135135
unique_type_id: UniqueTypeId<'tcx>,
136136
) -> DINodeCreationResult<'ll> {
137137
let coroutine_type = unique_type_id.expect_ty();
138-
let &ty::Coroutine(coroutine_def_id, _) = coroutine_type.kind() else {
138+
let &ty::Coroutine(coroutine_def_id, coroutine_args) = coroutine_type.kind() else {
139139
bug!("build_coroutine_di_node() called with non-coroutine type: `{:?}`", coroutine_type)
140140
};
141141

@@ -158,8 +158,10 @@ pub(super) fn build_coroutine_di_node<'ll, 'tcx>(
158158
DIFlags::FlagZero,
159159
),
160160
|cx, coroutine_type_di_node| {
161-
let coroutine_layout =
162-
cx.tcx.optimized_mir(coroutine_def_id).coroutine_layout().unwrap();
161+
let coroutine_layout = cx
162+
.tcx
163+
.coroutine_layout(coroutine_def_id, coroutine_args.as_coroutine().kind_ty())
164+
.unwrap();
163165

164166
let Variants::Multiple { tag_encoding: TagEncoding::Direct, ref variants, .. } =
165167
coroutine_type_and_layout.variants

‎compiler/rustc_const_eval/src/transform/validate.rs‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -101,18 +101,17 @@ impl<'tcx> MirPass<'tcx> for Validator {
101101
}
102102

103103
// Enforce that coroutine-closure layouts are identical.
104-
if let Some(layout) = body.coroutine_layout()
104+
if let Some(layout) = body.coroutine_layout_raw()
105105
&& let Some(by_move_body) = body.coroutine_by_move_body()
106-
&& let Some(by_move_layout) = by_move_body.coroutine_layout()
106+
&& let Some(by_move_layout) = by_move_body.coroutine_layout_raw()
107107
{
108-
if layout != by_move_layout {
109-
// If this turns out not to be true, please let compiler-errors know.
110-
// It is possible to support, but requires some changes to the layout
111-
// computation code.
108+
// FIXME(async_closures): We could do other validation here?
109+
if layout.variant_fields.len() != by_move_layout.variant_fields.len() {
112110
cfg_checker.fail(
113111
Location::START,
114112
format!(
115-
"Coroutine layout differs from by-move coroutine layout:\n\
113+
"Coroutine layout has different number of variant fields from \
114+
by-move coroutine layout:\n\
116115
layout: {layout:#?}\n\
117116
by_move_layout: {by_move_layout:#?}",
118117
),
@@ -715,13 +714,14 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
715714
// args of the coroutine. Otherwise, we prefer to use this body
716715
// since we may be in the process of computing this MIR in the
717716
// first place.
718-
let gen_body = if def_id == self.caller_body.source.def_id() {
719-
self.caller_body
717+
let layout = if def_id == self.caller_body.source.def_id() {
718+
// FIXME: This is not right for async closures.
719+
self.caller_body.coroutine_layout_raw()
720720
} else {
721-
self.tcx.optimized_mir(def_id)
721+
self.tcx.coroutine_layout(def_id, args.as_coroutine().kind_ty())
722722
};
723723

724-
let Some(layout) = gen_body.coroutine_layout() else {
724+
let Some(layout) = layout else {
725725
self.fail(
726726
location,
727727
format!("No coroutine layout for {parent_ty:?}"),

‎compiler/rustc_interface/src/tests.rs‎

Lines changed: 32 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -315,30 +315,39 @@ fn test_search_paths_tracking_hash_different_order() {
315315
json_rendered: HumanReadableErrorType::Default(ColorConfig::Never),
316316
};
317317

318+
let push = |opts: &mut Options, search_path| {
319+
opts.search_paths.push(SearchPath::from_cli_opt(
320+
"not-a-sysroot".as_ref(),
321+
&opts.target_triple,
322+
&early_dcx,
323+
search_path,
324+
));
325+
};
326+
318327
// Reference
319-
v1.search_paths.push(SearchPath::from_cli_opt(&early_dcx, "native=abc"));
320-
v1.search_paths.push(SearchPath::from_cli_opt(&early_dcx, "crate=def"));
321-
v1.search_paths.push(SearchPath::from_cli_opt(&early_dcx, "dependency=ghi"));
322-
v1.search_paths.push(SearchPath::from_cli_opt(&early_dcx, "framework=jkl"));
323-
v1.search_paths.push(SearchPath::from_cli_opt(&early_dcx, "all=mno"));
324-
325-
v2.search_paths.push(SearchPath::from_cli_opt(&early_dcx, "native=abc"));
326-
v2.search_paths.push(SearchPath::from_cli_opt(&early_dcx, "dependency=ghi"));
327-
v2.search_paths.push(SearchPath::from_cli_opt(&early_dcx, "crate=def"));
328-
v2.search_paths.push(SearchPath::from_cli_opt(&early_dcx, "framework=jkl"));
329-
v2.search_paths.push(SearchPath::from_cli_opt(&early_dcx, "all=mno"));
330-
331-
v3.search_paths.push(SearchPath::from_cli_opt(&early_dcx, "crate=def"));
332-
v3.search_paths.push(SearchPath::from_cli_opt(&early_dcx, "framework=jkl"));
333-
v3.search_paths.push(SearchPath::from_cli_opt(&early_dcx, "native=abc"));
334-
v3.search_paths.push(SearchPath::from_cli_opt(&early_dcx, "dependency=ghi"));
335-
v3.search_paths.push(SearchPath::from_cli_opt(&early_dcx, "all=mno"));
336-
337-
v4.search_paths.push(SearchPath::from_cli_opt(&early_dcx, "all=mno"));
338-
v4.search_paths.push(SearchPath::from_cli_opt(&early_dcx, "native=abc"));
339-
v4.search_paths.push(SearchPath::from_cli_opt(&early_dcx, "crate=def"));
340-
v4.search_paths.push(SearchPath::from_cli_opt(&early_dcx, "dependency=ghi"));
341-
v4.search_paths.push(SearchPath::from_cli_opt(&early_dcx, "framework=jkl"));
328+
push(&mut v1, "native=abc");
329+
push(&mut v1, "crate=def");
330+
push(&mut v1, "dependency=ghi");
331+
push(&mut v1, "framework=jkl");
332+
push(&mut v1, "all=mno");
333+
334+
push(&mut v2, "native=abc");
335+
push(&mut v2, "dependency=ghi");
336+
push(&mut v2, "crate=def");
337+
push(&mut v2, "framework=jkl");
338+
push(&mut v2, "all=mno");
339+
340+
push(&mut v3, "crate=def");
341+
push(&mut v3, "framework=jkl");
342+
push(&mut v3, "native=abc");
343+
push(&mut v3, "dependency=ghi");
344+
push(&mut v3, "all=mno");
345+
346+
push(&mut v4, "all=mno");
347+
push(&mut v4, "native=abc");
348+
push(&mut v4, "crate=def");
349+
push(&mut v4, "dependency=ghi");
350+
push(&mut v4, "framework=jkl");
342351

343352
assert_same_hash(&v1, &v2);
344353
assert_same_hash(&v1, &v3);

‎compiler/rustc_middle/src/mir/mod.rs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -652,8 +652,9 @@ impl<'tcx> Body<'tcx> {
652652
self.coroutine.as_ref().and_then(|coroutine| coroutine.resume_ty)
653653
}
654654

655+
/// Prefer going through [`TyCtxt::coroutine_layout`] rather than using this directly.
655656
#[inline]
656-
pub fn coroutine_layout(&self) -> Option<&CoroutineLayout<'tcx>> {
657+
pub fn coroutine_layout_raw(&self) -> Option<&CoroutineLayout<'tcx>> {
657658
self.coroutine.as_ref().and_then(|coroutine| coroutine.coroutine_layout.as_ref())
658659
}
659660

‎compiler/rustc_middle/src/mir/pretty.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ fn dump_matched_mir_node<'tcx, F>(
126126
Some(promoted) => write!(file, "::{promoted:?}`")?,
127127
}
128128
writeln!(file, " {disambiguator} {pass_name}")?;
129-
if let Some(ref layout) = body.coroutine_layout() {
129+
if let Some(ref layout) = body.coroutine_layout_raw() {
130130
writeln!(file, "/* coroutine_layout = {layout:#?} */")?;
131131
}
132132
writeln!(file)?;

0 commit comments

Comments
(0)

AltStyle によって変換されたページ (->オリジナル) /