Skip to content

Commit

Permalink
Auto merge of rust-lang#127360 - GuillaumeGomez:rollup-f0zs1qr, r=Gui…
Browse files Browse the repository at this point in the history
…llaumeGomez

Rollup of 7 pull requests

Successful merges:

 - rust-lang#124290 (DependencyList: removed outdated comment)
 - rust-lang#126709 (Migrate `include_bytes_deps`, `optimization-remarks-dir-pgo`, `optimization-remarks-dir`, `issue-40535` and `rmeta-preferred` `run-make` tests to rmake)
 - rust-lang#127214 (Use the native unwind function in miri where possible)
 - rust-lang#127320 (Update windows-bindgen to 0.58.0)
 - rust-lang#127349 (Tweak `-1 as usize` suggestion)
 - rust-lang#127352 (coverage: Rename `mir::coverage::BranchInfo` to `CoverageInfoHi`)
 - rust-lang#127359 (Improve run make llvm ident code)

r? `@ghost`
`@rustbot` modify labels: rollup
  • Loading branch information
bors committed Jul 5, 2024
2 parents 2ad6630 + 4fd3b12 commit 11dd90f
Show file tree
Hide file tree
Showing 44 changed files with 511 additions and 1,058 deletions.
8 changes: 4 additions & 4 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -6356,9 +6356,9 @@ dependencies = [

[[package]]
name = "windows-bindgen"
version = "0.57.0"
version = "0.58.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "1ccb96113d6277ba543c0f77e1c5494af8094bf9daf9b85acdc3f1b620e7c7b4"
checksum = "91cd28d93c692351f3a6e5615567c56756e330bee1c99c6bdd57bfc5ab15f589"
dependencies = [
"proc-macro2",
"rayon",
Expand All @@ -6379,9 +6379,9 @@ dependencies = [

[[package]]
name = "windows-metadata"
version = "0.57.0"
version = "0.58.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "8308d076825b9d9e5abc64f8113e96d02b2aeeba869b20fdd65c7e70cda13dfc"
checksum = "2e837f3c3012cfe9e7086302a93f441a7999439be1ad4c530d55d2f6d2921809"

[[package]]
name = "windows-sys"
Expand Down
13 changes: 11 additions & 2 deletions compiler/rustc_hir_typeck/src/op.rs
Original file line number Diff line number Diff line change
Expand Up @@ -838,8 +838,17 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
},
) = ex.kind
{
err.span_suggestion(
ex.span,
let span = if let hir::Node::Expr(parent) =
self.tcx.parent_hir_node(ex.hir_id)
&& let hir::ExprKind::Cast(..) = parent.kind
{
// `-1 as usize` -> `usize::MAX`
parent.span
} else {
ex.span
};
err.span_suggestion_verbose(
span,
format!(
"you may have meant the maximum value of `{actual}`",
),
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_middle/src/middle/dependency_format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,15 @@
//! For all the gory details, see the provider of the `dependency_formats`
//! query.

// FIXME: move this file to rustc_metadata::dependency_format, but
// this will introduce circular dependency between rustc_metadata and rustc_middle

use rustc_macros::{Decodable, Encodable, HashStable};
use rustc_session::config::CrateType;

/// A list of dependencies for a certain crate type.
///
/// The length of this vector is the same as the number of external crates used.
/// The value is None if the crate does not need to be linked (it was found
/// statically in another dylib), or Some(kind) if it needs to be linked as
/// `kind` (either static or dynamic).
pub type DependencyList = Vec<Linkage>;

/// A mapping of all required dependencies for a particular flavor of output.
Expand Down
11 changes: 8 additions & 3 deletions compiler/rustc_middle/src/mir/coverage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ pub enum CoverageKind {
SpanMarker,

/// Marks its enclosing basic block with an ID that can be referred to by
/// side data in [`BranchInfo`].
/// side data in [`CoverageInfoHi`].
///
/// Should be erased before codegen (at some point after `InstrumentCoverage`).
BlockMarker { id: BlockMarkerId },
Expand Down Expand Up @@ -274,10 +274,15 @@ pub struct FunctionCoverageInfo {
pub mcdc_num_condition_bitmaps: usize,
}

/// Branch information recorded during THIR-to-MIR lowering, and stored in MIR.
/// Coverage information for a function, recorded during MIR building and
/// attached to the corresponding `mir::Body`. Used by the `InstrumentCoverage`
/// MIR pass.
///
/// ("Hi" indicates that this is "high-level" information collected at the
/// THIR/MIR boundary, before the MIR-based coverage instrumentation pass.)
#[derive(Clone, Debug)]
#[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)]
pub struct BranchInfo {
pub struct CoverageInfoHi {
/// 1 more than the highest-numbered [`CoverageKind::BlockMarker`] that was
/// injected into the MIR body. This makes it possible to allocate per-ID
/// data structures without having to scan the entire body first.
Expand Down
13 changes: 7 additions & 6 deletions compiler/rustc_middle/src/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -430,11 +430,12 @@ pub struct Body<'tcx> {

pub tainted_by_errors: Option<ErrorGuaranteed>,

/// Branch coverage information collected during MIR building, to be used by
/// the `InstrumentCoverage` pass.
/// Coverage information collected from THIR/MIR during MIR building,
/// to be used by the `InstrumentCoverage` pass.
///
/// Only present if branch coverage is enabled and this function is eligible.
pub coverage_branch_info: Option<Box<coverage::BranchInfo>>,
/// Only present if coverage is enabled and this function is eligible.
/// Boxed to limit space overhead in non-coverage builds.
pub coverage_info_hi: Option<Box<coverage::CoverageInfoHi>>,

/// Per-function coverage information added by the `InstrumentCoverage`
/// pass, to be used in conjunction with the coverage statements injected
Expand Down Expand Up @@ -484,7 +485,7 @@ impl<'tcx> Body<'tcx> {
is_polymorphic: false,
injection_phase: None,
tainted_by_errors,
coverage_branch_info: None,
coverage_info_hi: None,
function_coverage_info: None,
};
body.is_polymorphic = body.has_non_region_param();
Expand Down Expand Up @@ -515,7 +516,7 @@ impl<'tcx> Body<'tcx> {
is_polymorphic: false,
injection_phase: None,
tainted_by_errors: None,
coverage_branch_info: None,
coverage_info_hi: None,
function_coverage_info: None,
};
body.is_polymorphic = body.has_non_region_param();
Expand Down
25 changes: 17 additions & 8 deletions compiler/rustc_middle/src/mir/pretty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -473,8 +473,8 @@ pub fn write_mir_intro<'tcx>(
// Add an empty line before the first block is printed.
writeln!(w)?;

if let Some(branch_info) = &body.coverage_branch_info {
write_coverage_branch_info(branch_info, w)?;
if let Some(coverage_info_hi) = &body.coverage_info_hi {
write_coverage_info_hi(coverage_info_hi, w)?;
}
if let Some(function_coverage_info) = &body.function_coverage_info {
write_function_coverage_info(function_coverage_info, w)?;
Expand All @@ -483,18 +483,26 @@ pub fn write_mir_intro<'tcx>(
Ok(())
}

fn write_coverage_branch_info(
branch_info: &coverage::BranchInfo,
fn write_coverage_info_hi(
coverage_info_hi: &coverage::CoverageInfoHi,
w: &mut dyn io::Write,
) -> io::Result<()> {
let coverage::BranchInfo { branch_spans, mcdc_branch_spans, mcdc_decision_spans, .. } =
branch_info;
let coverage::CoverageInfoHi {
num_block_markers: _,
branch_spans,
mcdc_branch_spans,
mcdc_decision_spans,
} = coverage_info_hi;

// Only add an extra trailing newline if we printed at least one thing.
let mut did_print = false;

for coverage::BranchSpan { span, true_marker, false_marker } in branch_spans {
writeln!(
w,
"{INDENT}coverage branch {{ true: {true_marker:?}, false: {false_marker:?} }} => {span:?}",
)?;
did_print = true;
}

for coverage::MCDCBranchSpan {
Expand All @@ -510,6 +518,7 @@ fn write_coverage_branch_info(
"{INDENT}coverage mcdc branch {{ condition_id: {:?}, true: {true_marker:?}, false: {false_marker:?}, depth: {decision_depth:?} }} => {span:?}",
condition_info.map(|info| info.condition_id)
)?;
did_print = true;
}

for coverage::MCDCDecisionSpan { span, num_conditions, end_markers, decision_depth } in
Expand All @@ -519,10 +528,10 @@ fn write_coverage_branch_info(
w,
"{INDENT}coverage mcdc decision {{ num_conditions: {num_conditions:?}, end: {end_markers:?}, depth: {decision_depth:?} }} => {span:?}"
)?;
did_print = true;
}

if !branch_spans.is_empty() || !mcdc_branch_spans.is_empty() || !mcdc_decision_spans.is_empty()
{
if did_print {
writeln!(w)?;
}

Expand Down
107 changes: 60 additions & 47 deletions compiler/rustc_mir_build/src/build/coverageinfo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use std::assert_matches::assert_matches;
use std::collections::hash_map::Entry;

use rustc_data_structures::fx::FxHashMap;
use rustc_middle::mir::coverage::{BlockMarkerId, BranchSpan, CoverageKind};
use rustc_middle::mir::coverage::{BlockMarkerId, BranchSpan, CoverageInfoHi, CoverageKind};
use rustc_middle::mir::{self, BasicBlock, SourceInfo, UnOp};
use rustc_middle::thir::{ExprId, ExprKind, Pat, Thir};
use rustc_middle::ty::TyCtxt;
Expand All @@ -13,16 +13,25 @@ use crate::build::{Builder, CFG};

mod mcdc;

pub(crate) struct BranchInfoBuilder {
/// Collects coverage-related information during MIR building, to eventually be
/// turned into a function's [`CoverageInfoHi`] when MIR building is complete.
pub(crate) struct CoverageInfoBuilder {
/// Maps condition expressions to their enclosing `!`, for better instrumentation.
nots: FxHashMap<ExprId, NotInfo>,

markers: BlockMarkerGen,
branch_spans: Vec<BranchSpan>,

/// Present if branch coverage is enabled.
branch_info: Option<BranchInfo>,
/// Present if MC/DC coverage is enabled.
mcdc_info: Option<MCDCInfoBuilder>,
}

#[derive(Default)]
struct BranchInfo {
branch_spans: Vec<BranchSpan>,
}

#[derive(Clone, Copy)]
struct NotInfo {
/// When visiting the associated expression as a branch condition, treat this
Expand Down Expand Up @@ -62,20 +71,20 @@ impl BlockMarkerGen {
}
}

impl BranchInfoBuilder {
/// Creates a new branch info builder, but only if branch coverage instrumentation
impl CoverageInfoBuilder {
/// Creates a new coverage info builder, but only if coverage instrumentation
/// is enabled and `def_id` represents a function that is eligible for coverage.
pub(crate) fn new_if_enabled(tcx: TyCtxt<'_>, def_id: LocalDefId) -> Option<Self> {
if tcx.sess.instrument_coverage_branch() && tcx.is_eligible_for_coverage(def_id) {
Some(Self {
nots: FxHashMap::default(),
markers: BlockMarkerGen::default(),
branch_spans: vec![],
mcdc_info: tcx.sess.instrument_coverage_mcdc().then(MCDCInfoBuilder::new),
})
} else {
None
if !tcx.sess.instrument_coverage() || !tcx.is_eligible_for_coverage(def_id) {
return None;
}

Some(Self {
nots: FxHashMap::default(),
markers: BlockMarkerGen::default(),
branch_info: tcx.sess.instrument_coverage_branch().then(BranchInfo::default),
mcdc_info: tcx.sess.instrument_coverage_mcdc().then(MCDCInfoBuilder::new),
})
}

/// Unary `!` expressions inside an `if` condition are lowered by lowering
Expand All @@ -88,6 +97,12 @@ impl BranchInfoBuilder {
pub(crate) fn visit_unary_not(&mut self, thir: &Thir<'_>, unary_not: ExprId) {
assert_matches!(thir[unary_not].kind, ExprKind::Unary { op: UnOp::Not, .. });

// The information collected by this visitor is only needed when branch
// coverage or higher is enabled.
if self.branch_info.is_none() {
return;
}

self.visit_with_not_info(
thir,
unary_not,
Expand Down Expand Up @@ -137,40 +152,40 @@ impl BranchInfoBuilder {
false_block,
inject_block_marker,
);
} else {
let true_marker = self.markers.inject_block_marker(cfg, source_info, true_block);
let false_marker = self.markers.inject_block_marker(cfg, source_info, false_block);

self.branch_spans.push(BranchSpan {
span: source_info.span,
true_marker,
false_marker,
});
return;
}

// Bail out if branch coverage is not enabled.
let Some(branch_info) = self.branch_info.as_mut() else { return };

let true_marker = self.markers.inject_block_marker(cfg, source_info, true_block);
let false_marker = self.markers.inject_block_marker(cfg, source_info, false_block);

branch_info.branch_spans.push(BranchSpan {
span: source_info.span,
true_marker,
false_marker,
});
}

pub(crate) fn into_done(self) -> Option<Box<mir::coverage::BranchInfo>> {
let Self {
nots: _,
markers: BlockMarkerGen { num_block_markers },
branch_spans,
mcdc_info,
} = self;
pub(crate) fn into_done(self) -> Box<CoverageInfoHi> {
let Self { nots: _, markers: BlockMarkerGen { num_block_markers }, branch_info, mcdc_info } =
self;

if num_block_markers == 0 {
assert!(branch_spans.is_empty());
return None;
}
let branch_spans =
branch_info.map(|branch_info| branch_info.branch_spans).unwrap_or_default();

let (mcdc_decision_spans, mcdc_branch_spans) =
mcdc_info.map(MCDCInfoBuilder::into_done).unwrap_or_default();

Some(Box::new(mir::coverage::BranchInfo {
// For simplicity, always return an info struct (without Option), even
// if there's nothing interesting in it.
Box::new(CoverageInfoHi {
num_block_markers,
branch_spans,
mcdc_branch_spans,
mcdc_decision_spans,
}))
})
}
}

Expand All @@ -184,7 +199,7 @@ impl<'tcx> Builder<'_, 'tcx> {
block: &mut BasicBlock,
) {
// Bail out if condition coverage is not enabled for this function.
let Some(branch_info) = self.coverage_branch_info.as_mut() else { return };
let Some(coverage_info) = self.coverage_info.as_mut() else { return };
if !self.tcx.sess.instrument_coverage_condition() {
return;
};
Expand Down Expand Up @@ -224,7 +239,7 @@ impl<'tcx> Builder<'_, 'tcx> {
);

// Separate path for handling branches when MC/DC is enabled.
branch_info.register_two_way_branch(
coverage_info.register_two_way_branch(
self.tcx,
&mut self.cfg,
source_info,
Expand All @@ -247,12 +262,12 @@ impl<'tcx> Builder<'_, 'tcx> {
mut then_block: BasicBlock,
mut else_block: BasicBlock,
) {
// Bail out if branch coverage is not enabled for this function.
let Some(branch_info) = self.coverage_branch_info.as_mut() else { return };
// Bail out if coverage is not enabled for this function.
let Some(coverage_info) = self.coverage_info.as_mut() else { return };

// If this condition expression is nested within one or more `!` expressions,
// replace it with the enclosing `!` collected by `visit_unary_not`.
if let Some(&NotInfo { enclosing_not, is_flipped }) = branch_info.nots.get(&expr_id) {
if let Some(&NotInfo { enclosing_not, is_flipped }) = coverage_info.nots.get(&expr_id) {
expr_id = enclosing_not;
if is_flipped {
std::mem::swap(&mut then_block, &mut else_block);
Expand All @@ -261,7 +276,7 @@ impl<'tcx> Builder<'_, 'tcx> {

let source_info = SourceInfo { span: self.thir[expr_id].span, scope: self.source_scope };

branch_info.register_two_way_branch(
coverage_info.register_two_way_branch(
self.tcx,
&mut self.cfg,
source_info,
Expand All @@ -280,13 +295,11 @@ impl<'tcx> Builder<'_, 'tcx> {
true_block: BasicBlock,
false_block: BasicBlock,
) {
// Bail out if branch coverage is not enabled for this function.
let Some(branch_info) = self.coverage_branch_info.as_mut() else { return };

// FIXME(#124144) This may need special handling when MC/DC is enabled.
// Bail out if coverage is not enabled for this function.
let Some(coverage_info) = self.coverage_info.as_mut() else { return };

let source_info = SourceInfo { span: pattern.span, scope: self.source_scope };
branch_info.register_two_way_branch(
coverage_info.register_two_way_branch(
self.tcx,
&mut self.cfg,
source_info,
Expand Down
Loading

0 comments on commit 11dd90f

Please sign in to comment.