Skip to content

Commit

Permalink
Auto merge of #117013 - matthiaskrgr:rollup-mvgp54x, r=matthiaskrgr
Browse files Browse the repository at this point in the history 
Rollup of 8 pull requests

Successful merges:

 - #114521 (std: freebsd build update.)
 - #116911 (Suggest relaxing implicit `type Assoc: Sized;` bound)
 - #116917 (coverage: Simplify the injection of coverage statements)
 - #116961 (Typo suggestion to change bindings with leading underscore)
 - #116964 (Add stable Instance::body() and RustcInternal trait)
 - #116974 (coverage: Fix inconsistent handling of function signature spans)
 - #116990 (Mention `into_iter` on borrow errors suggestions when appropriate)
 - #116995 (Point at assoc fn definition on type param divergence)

r? `@ghost`
`@rustbot` modify labels: rollup
  • Loading branch information
@bors
bors committed Oct 21, 2023
2 parents 45a45c6 + e9d18f5 commit 6f97d83
Show file tree
Hide file tree
Showing 36 changed files with 682 additions and 257 deletions.
22 changes: 22 additions & 0 deletions compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2263,6 +2263,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
current: usize,
found: usize,
prop_expr: Option<&'tcx hir::Expr<'tcx>>,
call: Option<&'tcx hir::Expr<'tcx>>,
}

impl<'tcx> Visitor<'tcx> for NestedStatementVisitor<'tcx> {
Expand All @@ -2272,6 +2273,11 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
self.current -= 1;
}
fn visit_expr(&mut self, expr: &'tcx hir::Expr<'tcx>) {
if let hir::ExprKind::MethodCall(_, rcvr, _, _) = expr.kind {
if self.span == rcvr.span.source_callsite() {
self.call = Some(expr);
}
}
if self.span == expr.span.source_callsite() {
self.found = self.current;
if self.prop_expr.is_none() {
Expand All @@ -2295,6 +2301,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
current: 0,
found: 0,
prop_expr: None,
call: None,
};
visitor.visit_stmt(stmt);

Expand All @@ -2316,6 +2323,21 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
&& let Some(p) = sm.span_to_margin(stmt.span)
&& let Ok(s) = sm.span_to_snippet(proper_span)
{
if let Some(call) = visitor.call
&& let hir::ExprKind::MethodCall(path, _, [], _) = call.kind
&& path.ident.name == sym::iter
&& let Some(ty) = expr_ty
{
err.span_suggestion_verbose(
path.ident.span,
format!(
"consider consuming the `{ty}` when turning it into an \
`Iterator`",
),
"into_iter".to_string(),
Applicability::MaybeIncorrect,
);
}
if !is_format_arguments_item {
let addition = format!("let binding = {};\n{}", s, " ".repeat(p));
err.multipart_suggestion_verbose(
Expand Down
24 changes: 5 additions & 19 deletions compiler/rustc_hir_analysis/src/check/compare_impl_item.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1557,38 +1557,24 @@ fn compare_number_of_generics<'tcx>(
DiagnosticId::Error("E0049".into()),
);

let mut suffix = None;

let msg =
format!("expected {trait_count} {kind} parameter{}", pluralize!(trait_count),);
if let Some(spans) = trait_spans {
let mut spans = spans.iter();
if let Some(span) = spans.next() {
err.span_label(
*span,
format!(
"expected {} {} parameter{}",
trait_count,
kind,
pluralize!(trait_count),
),
);
err.span_label(*span, msg);
}
for span in spans {
err.span_label(*span, "");
}
} else {
suffix = Some(format!(", expected {trait_count}"));
err.span_label(tcx.def_span(trait_.def_id), msg);
}

if let Some(span) = span {
err.span_label(
span,
format!(
"found {} {} parameter{}{}",
impl_count,
kind,
pluralize!(impl_count),
suffix.unwrap_or_default(),
),
format!("found {} {} parameter{}", impl_count, kind, pluralize!(impl_count),),
);
}

Expand Down
26 changes: 13 additions & 13 deletions compiler/rustc_mir_transform/src/coverage/counters.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -169,22 +169,22 @@ impl CoverageCounters {
self.bcb_counters[bcb].as_ref()
}

pub(super) fn take_bcb_counter(&mut self, bcb: BasicCoverageBlock) -> Option<BcbCounter> {
self.bcb_counters[bcb].take()
}

pub(super) fn drain_bcb_counters(
&mut self,
) -> impl Iterator<Item = (BasicCoverageBlock, BcbCounter)> + '_ {
pub(super) fn bcb_node_counters(
&self,
) -> impl Iterator<Item = (BasicCoverageBlock, &BcbCounter)> {
self.bcb_counters
.iter_enumerated_mut()
.filter_map(|(bcb, counter)| Some((bcb, counter.take()?)))
.iter_enumerated()
.filter_map(|(bcb, counter_kind)| Some((bcb, counter_kind.as_ref()?)))
}

pub(super) fn drain_bcb_edge_counters(
&mut self,
) -> impl Iterator<Item = ((BasicCoverageBlock, BasicCoverageBlock), BcbCounter)> + '_ {
self.bcb_edge_counters.drain()
/// For each edge in the BCB graph that has an associated counter, yields
/// that edge's *from* and *to* nodes, and its counter.
pub(super) fn bcb_edge_counters(
&self,
) -> impl Iterator<Item = (BasicCoverageBlock, BasicCoverageBlock, &BcbCounter)> {
self.bcb_edge_counters
.iter()
.map(|(&(from_bcb, to_bcb), counter_kind)| (from_bcb, to_bcb, counter_kind))
}

pub(super) fn take_expressions(&mut self) -> IndexVec<ExpressionId, Expression> {
Expand Down
195 changes: 72 additions & 123 deletions compiler/rustc_mir_transform/src/coverage/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ mod spans;
mod tests;

use self::counters::{BcbCounter, CoverageCounters};
use self::graph::{BasicCoverageBlock, BasicCoverageBlockData, CoverageGraph};
use self::graph::CoverageGraph;
use self::spans::CoverageSpans;

use crate::MirPass;
Expand Down Expand Up @@ -104,7 +104,6 @@ struct Instrumentor<'a, 'tcx> {
function_source_hash: u64,
basic_coverage_blocks: CoverageGraph,
coverage_counters: CoverageCounters,
mappings: Vec<Mapping>,
}

impl<'a, 'tcx> Instrumentor<'a, 'tcx> {
Expand Down Expand Up @@ -145,7 +144,6 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> {
function_source_hash,
basic_coverage_blocks,
coverage_counters,
mappings: Vec::new(),
}
}

Expand All @@ -168,148 +166,99 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> {
// and all `Expression` dependencies (operands) are also generated, for any other
// `BasicCoverageBlock`s not already associated with a coverage span.
let bcb_has_coverage_spans = |bcb| coverage_spans.bcb_has_coverage_spans(bcb);
let result = self
.coverage_counters
.make_bcb_counters(&mut self.basic_coverage_blocks, bcb_has_coverage_spans);

if let Ok(()) = result {
////////////////////////////////////////////////////
// Remove the counter or edge counter from of each coverage cpan's associated
// `BasicCoverageBlock`, and inject a `Coverage` statement into the MIR.
//
// `Coverage` statements injected from coverage spans will include the code regions
// (source code start and end positions) to be counted by the associated counter.
//
// These coverage-span-associated counters are removed from their associated
// `BasicCoverageBlock`s so that the only remaining counters in the `CoverageGraph`
// are indirect counters (to be injected next, without associated code regions).
self.inject_coverage_span_counters(&coverage_spans);

////////////////////////////////////////////////////
// For any remaining `BasicCoverageBlock` counters (that were not associated with
// any coverage span), inject `Coverage` statements (_without_ code region spans)
// to ensure `BasicCoverageBlock` counters that other `Expression`s may depend on
// are in fact counted, even though they don't directly contribute to counting
// their own independent code region's coverage.
self.inject_indirect_counters();
};
self.coverage_counters
.make_bcb_counters(&mut self.basic_coverage_blocks, bcb_has_coverage_spans)
.unwrap_or_else(|e| {
bug!("Error processing: {:?}: {:?}", self.mir_body.source.def_id(), e.message)
});

if let Err(e) = result {
bug!("Error processing: {:?}: {:?}", self.mir_body.source.def_id(), e.message)
};
let mappings = self.create_mappings_and_inject_coverage_statements(&coverage_spans);

self.mir_body.function_coverage_info = Some(Box::new(FunctionCoverageInfo {
function_source_hash: self.function_source_hash,
num_counters: self.coverage_counters.num_counters(),
expressions: self.coverage_counters.take_expressions(),
mappings: std::mem::take(&mut self.mappings),
mappings,
}));
}

/// Injects a single [`StatementKind::Coverage`] for each BCB that has one
/// or more coverage spans.
fn inject_coverage_span_counters(&mut self, coverage_spans: &CoverageSpans) {
let tcx = self.tcx;
let source_map = tcx.sess.source_map();
/// For each [`BcbCounter`] associated with a BCB node or BCB edge, create
/// any corresponding mappings (for BCB nodes only), and inject any necessary
/// coverage statements into MIR.
fn create_mappings_and_inject_coverage_statements(
&mut self,
coverage_spans: &CoverageSpans,
) -> Vec<Mapping> {
let source_map = self.tcx.sess.source_map();
let body_span = self.body_span;

use rustc_session::RemapFileNameExt;
let file_name =
Symbol::intern(&self.source_file.name.for_codegen(self.tcx.sess).to_string_lossy());

for (bcb, spans) in coverage_spans.bcbs_with_coverage_spans() {
let counter_kind = self.coverage_counters.take_bcb_counter(bcb).unwrap_or_else(|| {
bug!("Every BasicCoverageBlock should have a Counter or Expression");
});

let term = counter_kind.as_term();
self.mappings.extend(spans.iter().map(|&span| {
let code_region = make_code_region(source_map, file_name, span, body_span);
Mapping { code_region, term }
}));

inject_statement(
self.mir_body,
self.make_mir_coverage_kind(&counter_kind),
self.bcb_leader_bb(bcb),
);
}
}
let mut mappings = Vec::new();

// Process the counters and spans associated with BCB nodes.
for (bcb, counter_kind) in self.coverage_counters.bcb_node_counters() {
let spans = coverage_spans.spans_for_bcb(bcb);
let has_mappings = !spans.is_empty();

// If this BCB has any coverage spans, add corresponding mappings to
// the mappings table.
if has_mappings {
let term = counter_kind.as_term();
mappings.extend(spans.iter().map(|&span| {
let code_region = make_code_region(source_map, file_name, span, body_span);
Mapping { code_region, term }
}));
}

/// At this point, any BCB with coverage counters has already had its counter injected
/// into MIR, and had its counter removed from `coverage_counters` (via `take_counter()`).
///
/// Any other counter associated with a `BasicCoverageBlock`, or its incoming edge, but not
/// associated with a coverage span, should only exist if the counter is an `Expression`
/// dependency (one of the expression operands). Collect them, and inject the additional
/// counters into the MIR, without a reportable coverage span.
fn inject_indirect_counters(&mut self) {
let mut bcb_counters_without_direct_coverage_spans = Vec::new();
for (target_bcb, counter_kind) in self.coverage_counters.drain_bcb_counters() {
bcb_counters_without_direct_coverage_spans.push((None, target_bcb, counter_kind));
}
for ((from_bcb, target_bcb), counter_kind) in
self.coverage_counters.drain_bcb_edge_counters()
{
bcb_counters_without_direct_coverage_spans.push((
Some(from_bcb),
target_bcb,
counter_kind,
));
let do_inject = match counter_kind {
// Counter-increment statements always need to be injected.
BcbCounter::Counter { .. } => true,
// The only purpose of expression-used statements is to detect
// when a mapping is unreachable, so we only inject them for
// expressions with one or more mappings.
BcbCounter::Expression { .. } => has_mappings,
};
if do_inject {
inject_statement(
self.mir_body,
self.make_mir_coverage_kind(counter_kind),
self.basic_coverage_blocks[bcb].leader_bb(),
);
}
}

for (edge_from_bcb, target_bcb, counter_kind) in bcb_counters_without_direct_coverage_spans
{
match counter_kind {
BcbCounter::Counter { .. } => {
let inject_to_bb = if let Some(from_bcb) = edge_from_bcb {
// The MIR edge starts `from_bb` (the outgoing / last BasicBlock in
// `from_bcb`) and ends at `to_bb` (the incoming / first BasicBlock in the
// `target_bcb`; also called the `leader_bb`).
let from_bb = self.bcb_last_bb(from_bcb);
let to_bb = self.bcb_leader_bb(target_bcb);

let new_bb = inject_edge_counter_basic_block(self.mir_body, from_bb, to_bb);
debug!(
"Edge {:?} (last {:?}) -> {:?} (leader {:?}) requires a new MIR \
BasicBlock {:?}, for unclaimed edge counter {:?}",
edge_from_bcb, from_bb, target_bcb, to_bb, new_bb, counter_kind,
);
new_bb
} else {
let target_bb = self.bcb_last_bb(target_bcb);
debug!(
"{:?} ({:?}) gets a new Coverage statement for unclaimed counter {:?}",
target_bcb, target_bb, counter_kind,
);
target_bb
};

inject_statement(
self.mir_body,
self.make_mir_coverage_kind(&counter_kind),
inject_to_bb,
);
}
// Experessions with no associated spans don't need to inject a statement.
BcbCounter::Expression { .. } => {}
// Process the counters associated with BCB edges.
for (from_bcb, to_bcb, counter_kind) in self.coverage_counters.bcb_edge_counters() {
let do_inject = match counter_kind {
// Counter-increment statements always need to be injected.
BcbCounter::Counter { .. } => true,
// BCB-edge expressions never have mappings, so they never need
// a corresponding statement.
BcbCounter::Expression { .. } => false,
};
if !do_inject {
continue;
}
}
}

#[inline]
fn bcb_leader_bb(&self, bcb: BasicCoverageBlock) -> BasicBlock {
self.bcb_data(bcb).leader_bb()
}
// We need to inject a coverage statement into a new BB between the
// last BB of `from_bcb` and the first BB of `to_bcb`.
let from_bb = self.basic_coverage_blocks[from_bcb].last_bb();
let to_bb = self.basic_coverage_blocks[to_bcb].leader_bb();

#[inline]
fn bcb_last_bb(&self, bcb: BasicCoverageBlock) -> BasicBlock {
self.bcb_data(bcb).last_bb()
}
let new_bb = inject_edge_counter_basic_block(self.mir_body, from_bb, to_bb);
debug!(
"Edge {from_bcb:?} (last {from_bb:?}) -> {to_bcb:?} (leader {to_bb:?}) \
requires a new MIR BasicBlock {new_bb:?} for edge counter {counter_kind:?}",
);

// Inject a counter into the newly-created BB.
inject_statement(self.mir_body, self.make_mir_coverage_kind(&counter_kind), new_bb);
}

#[inline]
fn bcb_data(&self, bcb: BasicCoverageBlock) -> &BasicCoverageBlockData {
&self.basic_coverage_blocks[bcb]
mappings
}

fn make_mir_coverage_kind(&self, counter_kind: &BcbCounter) -> CoverageKind {
Expand Down
Loading

0 comments on commit 6f97d83

Please sign in to comment.