Skip to content

Commit

Permalink
Auto merge of rust-lang#118771 - workingjubilee:rollup-q1p3riz, r=wor…
Browse files Browse the repository at this point in the history
…kingjubilee

Rollup of 7 pull requests

Successful merges:

 - rust-lang#118198 (coverage: Use `SpanMarker` to improve coverage spans for `if !` expressions)
 - rust-lang#118512 (Add tests related to normalization in implied bounds)
 - rust-lang#118610 (update target feature following LLVM API change)
 - rust-lang#118666 (coverage: Simplify the heuristic for ignoring `async fn` return spans)
 - rust-lang#118737 (Extend tidy alphabetical checking to `tests/`.)
 - rust-lang#118762 (Some more minor `async gen`-related nits)
 - rust-lang#118764 (Make async generators fused by default)

r? `@ghost`
`@rustbot` modify labels: rollup
  • Loading branch information
bors committed Dec 9, 2023
2 parents c416699 + 61dfb1f commit ce67033
Show file tree
Hide file tree
Showing 38 changed files with 543 additions and 111 deletions.
8 changes: 8 additions & 0 deletions compiler/rustc_ast/src/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2450,6 +2450,14 @@ impl CoroutineKind {
matches!(self, CoroutineKind::Gen { .. })
}

pub fn closure_id(self) -> NodeId {
match self {
CoroutineKind::Async { closure_id, .. }
| CoroutineKind::Gen { closure_id, .. }
| CoroutineKind::AsyncGen { closure_id, .. } => closure_id,
}
}

/// In this case this is an `async` or `gen` return, the `NodeId` for the generated `impl Trait`
/// item.
pub fn return_id(self) -> (NodeId, Span) {
Expand Down
5 changes: 1 addition & 4 deletions compiler/rustc_ast_lowering/src/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1035,10 +1035,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
let (Some(coroutine_kind), Some(body)) = (coroutine_kind, body) else {
return self.lower_fn_body_block(span, decl, body);
};
// FIXME(gen_blocks): Introduce `closure_id` method and remove ALL destructuring.
let (CoroutineKind::Async { closure_id, .. }
| CoroutineKind::Gen { closure_id, .. }
| CoroutineKind::AsyncGen { closure_id, .. }) = coroutine_kind;
let closure_id = coroutine_kind.closure_id();

self.lower_body(|this| {
let mut parameters: Vec<hir::Param<'_>> = Vec::new();
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_ast_lowering/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,8 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
} else {
[sym::gen_future].into()
},
// FIXME(gen_blocks): how does `closure_track_caller`
// FIXME(gen_blocks): how does `closure_track_caller`/`async_fn_track_caller`
// interact with `gen`/`async gen` blocks
allow_async_iterator: [sym::gen_future, sym::async_iterator].into(),
generics_def_id_map: Default::default(),
host_param_id: None,
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_ast_passes/src/ast_validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1271,11 +1271,11 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
// Functions cannot both be `const async` or `const gen`
if let Some(&FnHeader {
constness: Const::Yes(cspan),
coroutine_kind: Some(coro_kind),
coroutine_kind: Some(coroutine_kind),
..
}) = fk.header()
{
let aspan = match coro_kind {
let aspan = match coroutine_kind {
CoroutineKind::Async { span: aspan, .. }
| CoroutineKind::Gen { span: aspan, .. }
| CoroutineKind::AsyncGen { span: aspan, .. } => aspan,
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_builtin_macros/src/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -541,8 +541,8 @@ fn check_test_signature(
return Err(sd.emit_err(errors::TestBadFn { span: i.span, cause: span, kind: "unsafe" }));
}

if let Some(coro_kind) = f.sig.header.coroutine_kind {
match coro_kind {
if let Some(coroutine_kind) = f.sig.header.coroutine_kind {
match coroutine_kind {
ast::CoroutineKind::Async { span, .. } => {
return Err(sd.emit_err(errors::TestBadFn {
span: i.span,
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,9 @@ impl<'tcx> CoverageInfoBuilderMethods<'tcx> for Builder<'_, '_, 'tcx> {

let Coverage { kind } = coverage;
match *kind {
// Span markers are only meaningful during MIR instrumentation,
// and have no effect during codegen.
CoverageKind::SpanMarker => {}
CoverageKind::CounterIncrement { id } => {
func_coverage.mark_counter_id_seen(id);
// We need to explicitly drop the `RefMut` before calling into `instrprof_increment`,
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_codegen_llvm/src/llvm_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,10 @@ pub fn to_llvm_features<'a>(sess: &Session, s: &'a str) -> LLVMFeature<'a> {
"sve2-bitperm",
TargetFeatureFoldStrength::EnableOnly("neon"),
),
// The unaligned-scalar-mem feature was renamed to fast-unaligned-access.
("riscv32" | "riscv64", "fast-unaligned-access") if get_version().0 <= 17 => {
LLVMFeature::new("unaligned-scalar-mem")
}
(_, s) => LLVMFeature::new(s),
}
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_ssa/src/target_features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -291,9 +291,9 @@ const RISCV_ALLOWED_FEATURES: &[(&str, Stability)] = &[
("d", Unstable(sym::riscv_target_feature)),
("e", Unstable(sym::riscv_target_feature)),
("f", Unstable(sym::riscv_target_feature)),
("fast-unaligned-access", Unstable(sym::riscv_target_feature)),
("m", Stable),
("relax", Unstable(sym::riscv_target_feature)),
("unaligned-scalar-mem", Unstable(sym::riscv_target_feature)),
("v", Unstable(sym::riscv_target_feature)),
("zba", Stable),
("zbb", Stable),
Expand Down
15 changes: 5 additions & 10 deletions compiler/rustc_lint/src/early.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,11 +162,8 @@ impl<'a, T: EarlyLintPass> ast_visit::Visitor<'a> for EarlyContextAndPass<'a, T>
// Explicitly check for lints associated with 'closure_id', since
// it does not have a corresponding AST node
if let ast_visit::FnKind::Fn(_, _, sig, _, _, _) = fk {
if let Some(coro_kind) = sig.header.coroutine_kind {
let (ast::CoroutineKind::Async { closure_id, .. }
| ast::CoroutineKind::Gen { closure_id, .. }
| ast::CoroutineKind::AsyncGen { closure_id, .. }) = coro_kind;
self.check_id(closure_id);
if let Some(coroutine_kind) = sig.header.coroutine_kind {
self.check_id(coroutine_kind.closure_id());
}
}
}
Expand Down Expand Up @@ -226,12 +223,10 @@ impl<'a, T: EarlyLintPass> ast_visit::Visitor<'a> for EarlyContextAndPass<'a, T>
// it does not have a corresponding AST node
match e.kind {
ast::ExprKind::Closure(box ast::Closure {
coroutine_kind: Some(coro_kind), ..
coroutine_kind: Some(coroutine_kind),
..
}) => {
let (ast::CoroutineKind::Async { closure_id, .. }
| ast::CoroutineKind::Gen { closure_id, .. }
| ast::CoroutineKind::AsyncGen { closure_id, .. }) = coro_kind;
self.check_id(closure_id);
self.check_id(coroutine_kind.closure_id());
}
_ => {}
}
Expand Down
8 changes: 8 additions & 0 deletions compiler/rustc_middle/src/mir/coverage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,13 @@ impl Debug for CovTerm {

#[derive(Clone, PartialEq, TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)]
pub enum CoverageKind {
/// Marks a span that might otherwise not be represented in MIR, so that
/// coverage instrumentation can associate it with its enclosing block/BCB.
///
/// Only used by the `InstrumentCoverage` pass, and has no effect during
/// codegen.
SpanMarker,

/// Marks the point in MIR control flow represented by a coverage counter.
///
/// This is eventually lowered to `llvm.instrprof.increment` in LLVM IR.
Expand All @@ -99,6 +106,7 @@ impl Debug for CoverageKind {
fn fmt(&self, fmt: &mut Formatter<'_>) -> fmt::Result {
use CoverageKind::*;
match self {
SpanMarker => write!(fmt, "SpanMarker"),
CounterIncrement { id } => write!(fmt, "CounterIncrement({:?})", id.index()),
ExpressionUsed { id } => write!(fmt, "ExpressionUsed({:?})", id.index()),
}
Expand Down
13 changes: 13 additions & 0 deletions compiler/rustc_mir_build/src/build/cfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,19 @@ impl<'tcx> CFG<'tcx> {
self.push(block, stmt);
}

/// Adds a dummy statement whose only role is to associate a span with its
/// enclosing block for the purposes of coverage instrumentation.
///
/// This results in more accurate coverage reports for certain kinds of
/// syntax (e.g. `continue` or `if !`) that would otherwise not appear in MIR.
pub(crate) fn push_coverage_span_marker(&mut self, block: BasicBlock, source_info: SourceInfo) {
let kind = StatementKind::Coverage(Box::new(Coverage {
kind: coverage::CoverageKind::SpanMarker,
}));
let stmt = Statement { source_info, kind };
self.push(block, stmt);
}

pub(crate) fn terminate(
&mut self,
block: BasicBlock,
Expand Down
5 changes: 5 additions & 0 deletions compiler/rustc_mir_build/src/build/matches/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,11 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
let local_scope = this.local_scope();
let (success_block, failure_block) =
this.in_if_then_scope(local_scope, expr_span, |this| {
// Help out coverage instrumentation by injecting a dummy statement with
// the original condition's span (including `!`). This fixes #115468.
if this.tcx.sess.instrument_coverage() {
this.cfg.push_coverage_span_marker(block, this.source_info(expr_span));
}
this.then_else_break(
block,
&this.thir[arg],
Expand Down
24 changes: 8 additions & 16 deletions compiler/rustc_mir_build/src/build/scope.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,6 @@ use rustc_index::{IndexSlice, IndexVec};
use rustc_middle::middle::region;
use rustc_middle::mir::*;
use rustc_middle::thir::{Expr, LintLevel};
use rustc_middle::ty::Ty;
use rustc_session::lint::Level;
use rustc_span::{Span, DUMMY_SP};

Expand Down Expand Up @@ -660,14 +659,15 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
(None, Some(_)) => {
panic!("`return`, `become` and `break` with value and must have a destination")
}
(None, None) if self.tcx.sess.instrument_coverage() => {
// Unlike `break` and `return`, which push an `Assign` statement to MIR, from which
// a Coverage code region can be generated, `continue` needs no `Assign`; but
// without one, the `InstrumentCoverage` MIR pass cannot generate a code region for
// `continue`. Coverage will be missing unless we add a dummy `Assign` to MIR.
self.add_dummy_assignment(span, block, source_info);
(None, None) => {
if self.tcx.sess.instrument_coverage() {
// Normally we wouldn't build any MIR in this case, but that makes it
// harder for coverage instrumentation to extract a relevant span for
// `continue` expressions. So here we inject a dummy statement with the
// desired span.
self.cfg.push_coverage_span_marker(block, source_info);
}
}
(None, None) => {}
}

let region_scope = self.scopes.breakable_scopes[break_index].region_scope;
Expand Down Expand Up @@ -723,14 +723,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
self.cfg.terminate(block, source_info, TerminatorKind::UnwindResume);
}

// Add a dummy `Assign` statement to the CFG, with the span for the source code's `continue`
// statement.
fn add_dummy_assignment(&mut self, span: Span, block: BasicBlock, source_info: SourceInfo) {
let local_decl = LocalDecl::new(Ty::new_unit(self.tcx), span);
let temp_place = Place::from(self.local_decls.push(local_decl));
self.cfg.push_assign_unit(block, source_info, temp_place, self.tcx);
}

fn leave_top_scope(&mut self, block: BasicBlock) -> BasicBlock {
// If we are emitting a `drop` statement, we need to have the cached
// diverge cleanup pads ready in case that drop panics.
Expand Down
44 changes: 33 additions & 11 deletions compiler/rustc_mir_transform/src/coroutine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -252,15 +252,15 @@ struct TransformVisitor<'tcx> {

impl<'tcx> TransformVisitor<'tcx> {
fn insert_none_ret_block(&self, body: &mut Body<'tcx>) -> BasicBlock {
assert!(matches!(self.coroutine_kind, CoroutineKind::Gen(_)));

let block = BasicBlock::new(body.basic_blocks.len());
let source_info = SourceInfo::outermost(body.span);
let option_def_id = self.tcx.require_lang_item(LangItem::Option, None);

let statements = vec![Statement {
kind: StatementKind::Assign(Box::new((
Place::return_place(),
let none_value = match self.coroutine_kind {
CoroutineKind::Async(_) => span_bug!(body.span, "`Future`s are not fused inherently"),
CoroutineKind::Coroutine => span_bug!(body.span, "`Coroutine`s cannot be fused"),
// `gen` continues return `None`
CoroutineKind::Gen(_) => {
let option_def_id = self.tcx.require_lang_item(LangItem::Option, None);
Rvalue::Aggregate(
Box::new(AggregateKind::Adt(
option_def_id,
Expand All @@ -270,8 +270,29 @@ impl<'tcx> TransformVisitor<'tcx> {
None,
)),
IndexVec::new(),
),
))),
)
}
// `async gen` continues to return `Poll::Ready(None)`
CoroutineKind::AsyncGen(_) => {
let ty::Adt(_poll_adt, args) = *self.old_yield_ty.kind() else { bug!() };
let ty::Adt(_option_adt, args) = *args.type_at(0).kind() else { bug!() };
let yield_ty = args.type_at(0);
Rvalue::Use(Operand::Constant(Box::new(ConstOperand {
span: source_info.span,
const_: Const::Unevaluated(
UnevaluatedConst::new(
self.tcx.require_lang_item(LangItem::AsyncGenFinished, None),
self.tcx.mk_args(&[yield_ty.into()]),
),
self.old_yield_ty,
),
user_ty: None,
})))
}
};

let statements = vec![Statement {
kind: StatementKind::Assign(Box::new((Place::return_place(), none_value))),
source_info,
}];

Expand Down Expand Up @@ -1393,11 +1414,12 @@ fn create_coroutine_resume_function<'tcx>(

if can_return {
let block = match coroutine_kind {
// FIXME(gen_blocks): Should `async gen` yield `None` when resumed once again?
CoroutineKind::Async(_) | CoroutineKind::AsyncGen(_) | CoroutineKind::Coroutine => {
CoroutineKind::Async(_) | CoroutineKind::Coroutine => {
insert_panic_block(tcx, body, ResumedAfterReturn(coroutine_kind))
}
CoroutineKind::Gen(_) => transform.insert_none_ret_block(body),
CoroutineKind::AsyncGen(_) | CoroutineKind::Gen(_) => {
transform.insert_none_ret_block(body)
}
};
cases.insert(1, (RETURNED, block));
}
Expand Down
47 changes: 16 additions & 31 deletions compiler/rustc_mir_transform/src/coverage/spans.rs
Original file line number Diff line number Diff line change
Expand Up @@ -319,29 +319,16 @@ impl<'a> CoverageSpansGenerator<'a> {
}
}

let prev = self.take_prev();
debug!(" AT END, adding last prev={prev:?}");

// Drain any remaining dups into the output.
for dup in self.pending_dups.drain(..) {
debug!(" ...adding at least one pending dup={:?}", dup);
self.refined_spans.push(dup);
}

// Async functions wrap a closure that implements the body to be executed. The enclosing
// function is called and returns an `impl Future` without initially executing any of the
// body. To avoid showing the return from the enclosing function as a "covered" return from
// the closure, the enclosing function's `TerminatorKind::Return`s `CoverageSpan` is
// excluded. The closure's `Return` is the only one that will be counted. This provides
// adequate coverage, and more intuitive counts. (Avoids double-counting the closing brace
// of the function body.)
let body_ends_with_closure = if let Some(last_covspan) = self.refined_spans.last() {
last_covspan.is_closure && last_covspan.span.hi() == self.body_span.hi()
} else {
false
};

if !body_ends_with_closure {
// There is usually a final span remaining in `prev` after the loop ends,
// so add it to the output as well.
if let Some(prev) = self.some_prev.take() {
debug!(" AT END, adding last prev={prev:?}");
self.refined_spans.push(prev);
}

Expand Down Expand Up @@ -398,38 +385,36 @@ impl<'a> CoverageSpansGenerator<'a> {
self.refined_spans.push(macro_name_cov);
}

#[track_caller]
fn curr(&self) -> &CoverageSpan {
self.some_curr
.as_ref()
.unwrap_or_else(|| bug!("invalid attempt to unwrap a None some_curr"))
self.some_curr.as_ref().unwrap_or_else(|| bug!("some_curr is None (curr)"))
}

#[track_caller]
fn curr_mut(&mut self) -> &mut CoverageSpan {
self.some_curr
.as_mut()
.unwrap_or_else(|| bug!("invalid attempt to unwrap a None some_curr"))
self.some_curr.as_mut().unwrap_or_else(|| bug!("some_curr is None (curr_mut)"))
}

/// If called, then the next call to `next_coverage_span()` will *not* update `prev` with the
/// `curr` coverage span.
#[track_caller]
fn take_curr(&mut self) -> CoverageSpan {
self.some_curr.take().unwrap_or_else(|| bug!("invalid attempt to unwrap a None some_curr"))
self.some_curr.take().unwrap_or_else(|| bug!("some_curr is None (take_curr)"))
}

#[track_caller]
fn prev(&self) -> &CoverageSpan {
self.some_prev
.as_ref()
.unwrap_or_else(|| bug!("invalid attempt to unwrap a None some_prev"))
self.some_prev.as_ref().unwrap_or_else(|| bug!("some_prev is None (prev)"))
}

#[track_caller]
fn prev_mut(&mut self) -> &mut CoverageSpan {
self.some_prev
.as_mut()
.unwrap_or_else(|| bug!("invalid attempt to unwrap a None some_prev"))
self.some_prev.as_mut().unwrap_or_else(|| bug!("some_prev is None (prev_mut)"))
}

#[track_caller]
fn take_prev(&mut self) -> CoverageSpan {
self.some_prev.take().unwrap_or_else(|| bug!("invalid attempt to unwrap a None some_prev"))
self.some_prev.take().unwrap_or_else(|| bug!("some_prev is None (take_prev)"))
}

/// If there are `pending_dups` but `prev` is not a matching dup (`prev.span` doesn't match the
Expand Down
Loading

0 comments on commit ce67033

Please sign in to comment.