Skip to content

Commit

Permalink
Auto merge of #85328 - GuillaumeGomez:rollup-exe9nbj, r=GuillaumeGomez
Browse files Browse the repository at this point in the history
Rollup of 12 pull requests

Successful merges:

 - #84461 (rustdoc: Remove unnecessary `StripItem` wrapper)
 - #85067 (Minimize amount of fake `DefId`s used in rustdoc)
 - #85207 (Fix typo in comment)
 - #85215 (coverage bug fixes and some refactoring)
 - #85221 (dbg macro: Discuss use in tests, and slightly clarify)
 - #85246 (Miner code formatting)
 - #85253 (swap function order for better read flow)
 - #85256 (Fix display for "implementors" section)
 - #85268 (Use my real name)
 - #85278 (Improve match statements)
 - #85289 (Fix toggle position on mobile)
 - #85323 (Fix eslint errors)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
  • Loading branch information
bors committed May 15, 2021
2 parents b439be0 + 46bc552 commit 2a245f4
Show file tree
Hide file tree
Showing 38 changed files with 427 additions and 447 deletions.
2 changes: 1 addition & 1 deletion .mailmap
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ Brian Anderson <[email protected]> <[email protected]>
Brian Anderson <[email protected]> <[email protected]>
Brian Dawn <[email protected]>
Brian Leibig <[email protected]> Brian Leibig <[email protected]>
Camelid <[email protected]> <[email protected]>
Noah Lev <[email protected]> <[email protected]>
Carl-Anton Ingmarsson <[email protected]> <[email protected]>
Carol (Nichols || Goulding) <[email protected]> <[email protected]>
Carol (Nichols || Goulding) <[email protected]> <[email protected]>
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_codegen_ssa/src/coverageinfo/map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,9 @@ impl<'tcx> FunctionCoverage<'tcx> {
}

fn create(tcx: TyCtxt<'tcx>, instance: Instance<'tcx>, is_used: bool) -> Self {
let coverageinfo = tcx.coverageinfo(instance.def_id());
let coverageinfo = tcx.coverageinfo(instance.def);
debug!(
"FunctionCoverage::new(instance={:?}) has coverageinfo={:?}. is_used={}",
"FunctionCoverage::create(instance={:?}) has coverageinfo={:?}. is_used={}",
instance, coverageinfo, is_used
);
Self {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_ssa/src/mir/coverageinfo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
bx.add_coverage_counter(instance, id, code_region);
}

let coverageinfo = bx.tcx().coverageinfo(instance.def_id());
let coverageinfo = bx.tcx().coverageinfo(instance.def);

let fn_name = bx.get_pgo_func_name_var(instance);
let hash = bx.const_u64(function_source_hash);
Expand Down
10 changes: 5 additions & 5 deletions compiler/rustc_error_codes/src/error_codes/E0277.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,16 +29,16 @@ trait Foo {
fn bar(&self);
}
fn some_func<T: Foo>(foo: T) {
foo.bar(); // we can now use this method since i32 implements the
// Foo trait
}
// we implement the trait on the i32 type
impl Foo for i32 {
fn bar(&self) {}
}
fn some_func<T: Foo>(foo: T) {
foo.bar(); // we can now use this method since i32 implements the
// Foo trait
}
fn main() {
some_func(5i32); // ok!
}
Expand Down
5 changes: 2 additions & 3 deletions compiler/rustc_middle/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -335,10 +335,9 @@ rustc_queries! {

/// Returns coverage summary info for a function, after executing the `InstrumentCoverage`
/// MIR pass (assuming the -Zinstrument-coverage option is enabled).
query coverageinfo(key: DefId) -> mir::CoverageInfo {
desc { |tcx| "retrieving coverage info from MIR for `{}`", tcx.def_path_str(key) }
query coverageinfo(key: ty::InstanceDef<'tcx>) -> mir::CoverageInfo {
desc { |tcx| "retrieving coverage info from MIR for `{}`", tcx.def_path_str(key.def_id()) }
storage(ArenaCacheSelector<'tcx>)
cache_on_disk_if { key.is_local() }
}

/// Returns the name of the file that contains the function body, if instrumented for coverage.
Expand Down
5 changes: 1 addition & 4 deletions compiler/rustc_mir/src/dataflow/move_paths/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,10 +137,7 @@ impl<'b, 'a, 'tcx> Gatherer<'b, 'a, 'tcx> {
self.loc,
InteriorOfSliceOrArray {
ty: place_ty,
is_index: match elem {
ProjectionElem::Index(..) => true,
_ => false,
},
is_index: matches!(elem, ProjectionElem::Index(..)),
},
));
}
Expand Down
4 changes: 3 additions & 1 deletion compiler/rustc_mir/src/transform/coverage/debug.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@ use rustc_index::vec::Idx;
use rustc_middle::mir::coverage::*;
use rustc_middle::mir::{self, BasicBlock, TerminatorKind};
use rustc_middle::ty::TyCtxt;
use rustc_span::Span;

use std::iter;
use std::lazy::SyncOnceCell;
Expand Down Expand Up @@ -636,6 +637,7 @@ pub(super) fn dump_coverage_spanview(
mir_body: &mir::Body<'tcx>,
basic_coverage_blocks: &CoverageGraph,
pass_name: &str,
body_span: Span,
coverage_spans: &Vec<CoverageSpan>,
) {
let mir_source = mir_body.source;
Expand All @@ -647,7 +649,7 @@ pub(super) fn dump_coverage_spanview(
let crate_name = tcx.crate_name(def_id.krate);
let item_name = tcx.def_path(def_id).to_filename_friendly_no_crate();
let title = format!("{}.{} - Coverage Spans", crate_name, item_name);
spanview::write_document(tcx, def_id, span_viewables, &title, &mut file)
spanview::write_document(tcx, body_span, span_viewables, &title, &mut file)
.expect("Unexpected IO error dumping coverage spans as HTML");
}

Expand Down
69 changes: 41 additions & 28 deletions compiler/rustc_mir/src/transform/coverage/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ impl<'tcx> MirPass<'tcx> for InstrumentCoverage {

trace!("InstrumentCoverage starting for {:?}", mir_source.def_id());
Instrumentor::new(&self.name(), tcx, mir_body).inject_counters();
trace!("InstrumentCoverage starting for {:?}", mir_source.def_id());
trace!("InstrumentCoverage done for {:?}", mir_source.def_id());
}
}

Expand All @@ -116,25 +116,7 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> {
let def_id = mir_body.source.def_id();
let (some_fn_sig, hir_body) = fn_sig_and_body(tcx, def_id);

let mut body_span = hir_body.value.span;

if tcx.is_closure(def_id) {
// If the MIR function is a closure, and if the closure body span
// starts from a macro, but it's content is not in that macro, try
// to find a non-macro callsite, and instrument the spans there
// instead.
loop {
let expn_data = body_span.ctxt().outer_expn_data();
if expn_data.is_root() {
break;
}
if let ExpnKind::Macro { .. } = expn_data.kind {
body_span = expn_data.call_site;
} else {
break;
}
}
}
let body_span = get_body_span(tcx, hir_body, mir_body);

let source_file = source_map.lookup_source_file(body_span.lo());
let fn_sig_span = match some_fn_sig.filter(|fn_sig| {
Expand All @@ -144,6 +126,15 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> {
Some(fn_sig) => fn_sig.span.with_hi(body_span.lo()),
None => body_span.shrink_to_lo(),
};

debug!(
"instrumenting {}: {:?}, fn sig span: {:?}, body span: {:?}",
if tcx.is_closure(def_id) { "closure" } else { "function" },
def_id,
fn_sig_span,
body_span
);

let function_source_hash = hash_mir_source(tcx, hir_body);
let basic_coverage_blocks = CoverageGraph::from_mir(mir_body);
Self {
Expand All @@ -160,19 +151,11 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> {

fn inject_counters(&'a mut self) {
let tcx = self.tcx;
let source_map = tcx.sess.source_map();
let mir_source = self.mir_body.source;
let def_id = mir_source.def_id();
let fn_sig_span = self.fn_sig_span;
let body_span = self.body_span;

debug!(
"instrumenting {:?}, fn sig span: {}, body span: {}",
def_id,
source_map.span_to_diagnostic_string(fn_sig_span),
source_map.span_to_diagnostic_string(body_span)
);

let mut graphviz_data = debug::GraphvizData::new();
let mut debug_used_expressions = debug::UsedExpressions::new();

Expand Down Expand Up @@ -204,6 +187,7 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> {
self.mir_body,
&self.basic_coverage_blocks,
self.pass_name,
body_span,
&coverage_spans,
);
}
Expand Down Expand Up @@ -560,6 +544,35 @@ fn fn_sig_and_body<'tcx>(
(hir::map::fn_sig(hir_node), tcx.hir().body(fn_body_id))
}

fn get_body_span<'tcx>(
tcx: TyCtxt<'tcx>,
hir_body: &rustc_hir::Body<'tcx>,
mir_body: &mut mir::Body<'tcx>,
) -> Span {
let mut body_span = hir_body.value.span;
let def_id = mir_body.source.def_id();

if tcx.is_closure(def_id) {
// If the MIR function is a closure, and if the closure body span
// starts from a macro, but it's content is not in that macro, try
// to find a non-macro callsite, and instrument the spans there
// instead.
loop {
let expn_data = body_span.ctxt().outer_expn_data();
if expn_data.is_root() {
break;
}
if let ExpnKind::Macro { .. } = expn_data.kind {
body_span = expn_data.call_site;
} else {
break;
}
}
}

body_span
}

fn hash_mir_source<'tcx>(tcx: TyCtxt<'tcx>, hir_body: &'tcx rustc_hir::Body<'tcx>) -> u64 {
let mut hcx = tcx.create_no_span_stable_hashing_context();
hash(&mut hcx, &hir_body.value).to_smaller_hash()
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_mir/src/transform/coverage/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,8 @@ impl CoverageVisitor {
}
}

fn coverageinfo<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId) -> CoverageInfo {
let mir_body = mir_body(tcx, def_id);
fn coverageinfo<'tcx>(tcx: TyCtxt<'tcx>, instance_def: ty::InstanceDef<'tcx>) -> CoverageInfo {
let mir_body = tcx.instance_mir(instance_def);

let mut coverage_visitor = CoverageVisitor {
// num_counters always has at least the `ZERO` counter.
Expand Down
78 changes: 35 additions & 43 deletions compiler/rustc_mir/src/transform/coverage/spans.rs
Original file line number Diff line number Diff line change
Expand Up @@ -530,17 +530,25 @@ impl<'a, 'tcx> CoverageSpans<'a, 'tcx> {
.iter()
.enumerate()
.filter_map(move |(index, statement)| {
filtered_statement_span(statement, self.body_span).map(
|(span, expn_span)| {
CoverageSpan::for_statement(
statement, span, expn_span, bcb, bb, index,
)
},
)
filtered_statement_span(statement).map(|span| {
CoverageSpan::for_statement(
statement,
function_source_span(span, self.body_span),
span,
bcb,
bb,
index,
)
})
})
.chain(filtered_terminator_span(data.terminator(), self.body_span).map(
|(span, expn_span)| CoverageSpan::for_terminator(span, expn_span, bcb, bb),
))
.chain(filtered_terminator_span(data.terminator()).map(|span| {
CoverageSpan::for_terminator(
function_source_span(span, self.body_span),
span,
bcb,
bb,
)
}))
})
.collect()
}
Expand Down Expand Up @@ -795,13 +803,9 @@ impl<'a, 'tcx> CoverageSpans<'a, 'tcx> {
}
}

/// See `function_source_span()` for a description of the two returned spans.
/// If the MIR `Statement` is not contributive to computing coverage spans,
/// returns `None`.
pub(super) fn filtered_statement_span(
statement: &'a Statement<'tcx>,
body_span: Span,
) -> Option<(Span, Span)> {
/// If the MIR `Statement` has a span contributive to computing coverage spans,
/// return it; otherwise return `None`.
pub(super) fn filtered_statement_span(statement: &'a Statement<'tcx>) -> Option<Span> {
match statement.kind {
// These statements have spans that are often outside the scope of the executed source code
// for their parent `BasicBlock`.
Expand Down Expand Up @@ -838,18 +842,14 @@ pub(super) fn filtered_statement_span(
| StatementKind::LlvmInlineAsm(_)
| StatementKind::Retag(_, _)
| StatementKind::AscribeUserType(_, _) => {
Some(function_source_span(statement.source_info.span, body_span))
Some(statement.source_info.span)
}
}
}

/// See `function_source_span()` for a description of the two returned spans.
/// If the MIR `Terminator` is not contributive to computing coverage spans,
/// returns `None`.
pub(super) fn filtered_terminator_span(
terminator: &'a Terminator<'tcx>,
body_span: Span,
) -> Option<(Span, Span)> {
/// If the MIR `Terminator` has a span contributive to computing coverage spans,
/// return it; otherwise return `None`.
pub(super) fn filtered_terminator_span(terminator: &'a Terminator<'tcx>) -> Option<Span> {
match terminator.kind {
// These terminators have spans that don't positively contribute to computing a reasonable
// span of actually executed source code. (For example, SwitchInt terminators extracted from
Expand All @@ -873,7 +873,7 @@ pub(super) fn filtered_terminator_span(
span = span.with_lo(constant.span.lo());
}
}
Some(function_source_span(span, body_span))
Some(span)
}

// Retain spans from all other terminators
Expand All @@ -884,28 +884,20 @@ pub(super) fn filtered_terminator_span(
| TerminatorKind::GeneratorDrop
| TerminatorKind::FalseUnwind { .. }
| TerminatorKind::InlineAsm { .. } => {
Some(function_source_span(terminator.source_info.span, body_span))
Some(terminator.source_info.span)
}
}
}

/// Returns two spans from the given span (the span associated with a
/// `Statement` or `Terminator`):
///
/// 1. An extrapolated span (pre-expansion[^1]) corresponding to a range within
/// the function's body source. This span is guaranteed to be contained
/// within, or equal to, the `body_span`. If the extrapolated span is not
/// contained within the `body_span`, the `body_span` is returned.
/// 2. The actual `span` value from the `Statement`, before expansion.
///
/// Only the first span is used when computing coverage code regions. The second
/// span is useful if additional expansion data is needed (such as to look up
/// the macro name for a composed span within that macro).)
/// Returns an extrapolated span (pre-expansion[^1]) corresponding to a range
/// within the function's body source. This span is guaranteed to be contained
/// within, or equal to, the `body_span`. If the extrapolated span is not
/// contained within the `body_span`, the `body_span` is returned.
///
/// [^1]Expansions result from Rust syntax including macros, syntactic
/// sugar, etc.).
/// [^1]Expansions result from Rust syntax including macros, syntactic sugar,
/// etc.).
#[inline]
fn function_source_span(span: Span, body_span: Span) -> (Span, Span) {
pub(super) fn function_source_span(span: Span, body_span: Span) -> Span {
let original_span = original_sp(span, body_span).with_ctxt(body_span.ctxt());
(if body_span.contains(original_span) { original_span } else { body_span }, span)
if body_span.contains(original_span) { original_span } else { body_span }
}
6 changes: 2 additions & 4 deletions compiler/rustc_mir/src/transform/coverage/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -683,12 +683,10 @@ fn test_make_bcb_counters() {
let mut basic_coverage_blocks = graph::CoverageGraph::from_mir(&mir_body);
let mut coverage_spans = Vec::new();
for (bcb, data) in basic_coverage_blocks.iter_enumerated() {
if let Some((span, expn_span)) =
spans::filtered_terminator_span(data.terminator(&mir_body), body_span)
{
if let Some(span) = spans::filtered_terminator_span(data.terminator(&mir_body)) {
coverage_spans.push(spans::CoverageSpan::for_terminator(
spans::function_source_span(span, body_span),
span,
expn_span,
bcb,
data.last_bb(),
));
Expand Down
5 changes: 1 addition & 4 deletions compiler/rustc_mir/src/transform/early_otherwise_branch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -170,10 +170,7 @@ impl<'tcx> MirPass<'tcx> for EarlyOtherwiseBranch {
}

fn is_switch<'tcx>(terminator: &Terminator<'tcx>) -> bool {
match terminator.kind {
TerminatorKind::SwitchInt { .. } => true,
_ => false,
}
matches!(terminator.kind, TerminatorKind::SwitchInt { .. })
}

struct Helper<'a, 'tcx> {
Expand Down
5 changes: 1 addition & 4 deletions compiler/rustc_mir/src/transform/simplify_try.rs
Original file line number Diff line number Diff line change
Expand Up @@ -628,10 +628,7 @@ impl<'a, 'tcx> SimplifyBranchSameOptimizationFinder<'a, 'tcx> {
// But `asm!(...)` could abort the program,
// so we cannot assume that the `unreachable` terminator itself is reachable.
// FIXME(Centril): use a normalization pass instead of a check.
|| bb.statements.iter().any(|stmt| match stmt.kind {
StatementKind::LlvmInlineAsm(..) => true,
_ => false,
})
|| bb.statements.iter().any(|stmt| matches!(stmt.kind, StatementKind::LlvmInlineAsm(..)))
})
.peekable();

Expand Down
Loading

0 comments on commit 2a245f4

Please sign in to comment.