Skip to content

Commit ce67033

Browse files
borsbors
committed
Auto merge of #118771 - workingjubilee:rollup-q1p3riz, r=workingjubilee
Rollup of 7 pull requests Successful merges: - #118198 (coverage: Use `SpanMarker` to improve coverage spans for `if !` expressions) - #118512 (Add tests related to normalization in implied bounds) - #118610 (update target feature following LLVM API change) - #118666 (coverage: Simplify the heuristic for ignoring `async fn` return spans) - #118737 (Extend tidy alphabetical checking to `tests/`.) - #118762 (Some more minor `async gen`-related nits) - #118764 (Make async generators fused by default) r? `@ghost` `@rustbot` modify labels: rollup
2 parents c416699 + 61dfb1f commit ce67033

File tree

38 files changed

+543
-111
lines changed

38 files changed

+543
-111
lines changed

‎compiler/rustc_ast/src/ast.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2450,6 +2450,14 @@ impl CoroutineKind {
24502450
matches!(self, CoroutineKind::Gen { .. })
24512451
}
24522452

2453+
pub fn closure_id(self) -> NodeId {
2454+
match self {
2455+
CoroutineKind::Async { closure_id, .. }
2456+
| CoroutineKind::Gen { closure_id, .. }
2457+
| CoroutineKind::AsyncGen { closure_id, .. } => closure_id,
2458+
}
2459+
}
2460+
24532461
/// In this case this is an `async` or `gen` return, the `NodeId` for the generated `impl Trait`
24542462
/// item.
24552463
pub fn return_id(self) -> (NodeId, Span) {

‎compiler/rustc_ast_lowering/src/item.rs

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1035,10 +1035,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
10351035
let (Some(coroutine_kind), Some(body)) = (coroutine_kind, body) else {
10361036
return self.lower_fn_body_block(span, decl, body);
10371037
};
1038-
// FIXME(gen_blocks): Introduce `closure_id` method and remove ALL destructuring.
1039-
let (CoroutineKind::Async { closure_id, .. }
1040-
| CoroutineKind::Gen { closure_id, .. }
1041-
| CoroutineKind::AsyncGen { closure_id, .. }) = coroutine_kind;
1038+
let closure_id = coroutine_kind.closure_id();
10421039

10431040
self.lower_body(|this| {
10441041
let mut parameters: Vec<hir::Param<'_>> = Vec::new();

‎compiler/rustc_ast_lowering/src/lib.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,8 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
177177
} else {
178178
[sym::gen_future].into()
179179
},
180-
// FIXME(gen_blocks): how does `closure_track_caller`
180+
// FIXME(gen_blocks): how does `closure_track_caller`/`async_fn_track_caller`
181+
// interact with `gen`/`async gen` blocks
181182
allow_async_iterator: [sym::gen_future, sym::async_iterator].into(),
182183
generics_def_id_map: Default::default(),
183184
host_param_id: None,

‎compiler/rustc_ast_passes/src/ast_validation.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1271,11 +1271,11 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
12711271
// Functions cannot both be `const async` or `const gen`
12721272
if let Some(&FnHeader {
12731273
constness: Const::Yes(cspan),
1274-
coroutine_kind: Some(coro_kind),
1274+
coroutine_kind: Some(coroutine_kind),
12751275
..
12761276
}) = fk.header()
12771277
{
1278-
let aspan = match coro_kind {
1278+
let aspan = match coroutine_kind {
12791279
CoroutineKind::Async { span: aspan, .. }
12801280
| CoroutineKind::Gen { span: aspan, .. }
12811281
| CoroutineKind::AsyncGen { span: aspan, .. } => aspan,

‎compiler/rustc_builtin_macros/src/test.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -541,8 +541,8 @@ fn check_test_signature(
541541
return Err(sd.emit_err(errors::TestBadFn { span: i.span, cause: span, kind: "unsafe" }));
542542
}
543543

544-
if let Some(coro_kind) = f.sig.header.coroutine_kind {
545-
match coro_kind {
544+
if let Some(coroutine_kind) = f.sig.header.coroutine_kind {
545+
match coroutine_kind {
546546
ast::CoroutineKind::Async { span, .. } => {
547547
return Err(sd.emit_err(errors::TestBadFn {
548548
span: i.span,

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,9 @@ impl<'tcx> CoverageInfoBuilderMethods<'tcx> for Builder<'_, '_, 'tcx> {
100100

101101
let Coverage { kind } = coverage;
102102
match *kind {
103+
// Span markers are only meaningful during MIR instrumentation,
104+
// and have no effect during codegen.
105+
CoverageKind::SpanMarker => {}
103106
CoverageKind::CounterIncrement { id } => {
104107
func_coverage.mark_counter_id_seen(id);
105108
// We need to explicitly drop the `RefMut` before calling into `instrprof_increment`,

‎compiler/rustc_codegen_llvm/src/llvm_util.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -263,6 +263,10 @@ pub fn to_llvm_features<'a>(sess: &Session, s: &'a str) -> LLVMFeature<'a> {
263263
"sve2-bitperm",
264264
TargetFeatureFoldStrength::EnableOnly("neon"),
265265
),
266+
// The unaligned-scalar-mem feature was renamed to fast-unaligned-access.
267+
("riscv32" | "riscv64", "fast-unaligned-access") if get_version().0 <= 17 => {
268+
LLVMFeature::new("unaligned-scalar-mem")
269+
}
266270
(_, s) => LLVMFeature::new(s),
267271
}
268272
}

‎compiler/rustc_codegen_ssa/src/target_features.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -291,9 +291,9 @@ const RISCV_ALLOWED_FEATURES: &[(&str, Stability)] = &[
291291
("d", Unstable(sym::riscv_target_feature)),
292292
("e", Unstable(sym::riscv_target_feature)),
293293
("f", Unstable(sym::riscv_target_feature)),
294+
("fast-unaligned-access", Unstable(sym::riscv_target_feature)),
294295
("m", Stable),
295296
("relax", Unstable(sym::riscv_target_feature)),
296-
("unaligned-scalar-mem", Unstable(sym::riscv_target_feature)),
297297
("v", Unstable(sym::riscv_target_feature)),
298298
("zba", Stable),
299299
("zbb", Stable),

‎compiler/rustc_lint/src/early.rs

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -162,11 +162,8 @@ impl<'a, T: EarlyLintPass> ast_visit::Visitor<'a> for EarlyContextAndPass<'a, T>
162162
// Explicitly check for lints associated with 'closure_id', since
163163
// it does not have a corresponding AST node
164164
if let ast_visit::FnKind::Fn(_, _, sig, _, _, _) = fk {
165-
if let Some(coro_kind) = sig.header.coroutine_kind {
166-
let (ast::CoroutineKind::Async { closure_id, .. }
167-
| ast::CoroutineKind::Gen { closure_id, .. }
168-
| ast::CoroutineKind::AsyncGen { closure_id, .. }) = coro_kind;
169-
self.check_id(closure_id);
165+
if let Some(coroutine_kind) = sig.header.coroutine_kind {
166+
self.check_id(coroutine_kind.closure_id());
170167
}
171168
}
172169
}
@@ -226,12 +223,10 @@ impl<'a, T: EarlyLintPass> ast_visit::Visitor<'a> for EarlyContextAndPass<'a, T>
226223
// it does not have a corresponding AST node
227224
match e.kind {
228225
ast::ExprKind::Closure(box ast::Closure {
229-
coroutine_kind: Some(coro_kind), ..
226+
coroutine_kind: Some(coroutine_kind),
227+
..
230228
}) => {
231-
let (ast::CoroutineKind::Async { closure_id, .. }
232-
| ast::CoroutineKind::Gen { closure_id, .. }
233-
| ast::CoroutineKind::AsyncGen { closure_id, .. }) = coro_kind;
234-
self.check_id(closure_id);
229+
self.check_id(coroutine_kind.closure_id());
235230
}
236231
_ => {}
237232
}

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,13 @@ impl Debug for CovTerm {
7676

7777
#[derive(Clone, PartialEq, TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)]
7878
pub enum CoverageKind {
79+
/// Marks a span that might otherwise not be represented in MIR, so that
80+
/// coverage instrumentation can associate it with its enclosing block/BCB.
81+
///
82+
/// Only used by the `InstrumentCoverage` pass, and has no effect during
83+
/// codegen.
84+
SpanMarker,
85+
7986
/// Marks the point in MIR control flow represented by a coverage counter.
8087
///
8188
/// This is eventually lowered to `llvm.instrprof.increment` in LLVM IR.
@@ -99,6 +106,7 @@ impl Debug for CoverageKind {
99106
fn fmt(&self, fmt: &mut Formatter<'_>) -> fmt::Result {
100107
use CoverageKind::*;
101108
match self {
109+
SpanMarker => write!(fmt, "SpanMarker"),
102110
CounterIncrement { id } => write!(fmt, "CounterIncrement({:?})", id.index()),
103111
ExpressionUsed { id } => write!(fmt, "ExpressionUsed({:?})", id.index()),
104112
}

‎compiler/rustc_mir_build/src/build/cfg.rs

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,19 @@ impl<'tcx> CFG<'tcx> {
101101
self.push(block, stmt);
102102
}
103103

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

‎compiler/rustc_mir_build/src/build/matches/mod.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,11 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
9090
let local_scope = this.local_scope();
9191
let (success_block, failure_block) =
9292
this.in_if_then_scope(local_scope, expr_span, |this| {
93+
// Help out coverage instrumentation by injecting a dummy statement with
94+
// the original condition's span (including `!`). This fixes #115468.
95+
if this.tcx.sess.instrument_coverage() {
96+
this.cfg.push_coverage_span_marker(block, this.source_info(expr_span));
97+
}
9398
this.then_else_break(
9499
block,
95100
&this.thir[arg],

‎compiler/rustc_mir_build/src/build/scope.rs

Lines changed: 8 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,6 @@ use rustc_index::{IndexSlice, IndexVec};
9090
use rustc_middle::middle::region;
9191
use rustc_middle::mir::*;
9292
use rustc_middle::thir::{Expr, LintLevel};
93-
use rustc_middle::ty::Ty;
9493
use rustc_session::lint::Level;
9594
use rustc_span::{Span, DUMMY_SP};
9695

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

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

726-
// Add a dummy `Assign` statement to the CFG, with the span for the source code's `continue`
727-
// statement.
728-
fn add_dummy_assignment(&mut self, span: Span, block: BasicBlock, source_info: SourceInfo) {
729-
let local_decl = LocalDecl::new(Ty::new_unit(self.tcx), span);
730-
let temp_place = Place::from(self.local_decls.push(local_decl));
731-
self.cfg.push_assign_unit(block, source_info, temp_place, self.tcx);
732-
}
733-
734726
fn leave_top_scope(&mut self, block: BasicBlock) -> BasicBlock {
735727
// If we are emitting a `drop` statement, we need to have the cached
736728
// diverge cleanup pads ready in case that drop panics.

‎compiler/rustc_mir_transform/src/coroutine.rs

Lines changed: 33 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -252,15 +252,15 @@ struct TransformVisitor<'tcx> {
252252

253253
impl<'tcx> TransformVisitor<'tcx> {
254254
fn insert_none_ret_block(&self, body: &mut Body<'tcx>) -> BasicBlock {
255-
assert!(matches!(self.coroutine_kind, CoroutineKind::Gen(_)));
256-
257255
let block = BasicBlock::new(body.basic_blocks.len());
258256
let source_info = SourceInfo::outermost(body.span);
259-
let option_def_id = self.tcx.require_lang_item(LangItem::Option, None);
260257

261-
let statements = vec![Statement {
262-
kind: StatementKind::Assign(Box::new((
263-
Place::return_place(),
258+
let none_value = match self.coroutine_kind {
259+
CoroutineKind::Async(_) => span_bug!(body.span, "`Future`s are not fused inherently"),
260+
CoroutineKind::Coroutine => span_bug!(body.span, "`Coroutine`s cannot be fused"),
261+
// `gen` continues return `None`
262+
CoroutineKind::Gen(_) => {
263+
let option_def_id = self.tcx.require_lang_item(LangItem::Option, None);
264264
Rvalue::Aggregate(
265265
Box::new(AggregateKind::Adt(
266266
option_def_id,
@@ -270,8 +270,29 @@ impl<'tcx> TransformVisitor<'tcx> {
270270
None,
271271
)),
272272
IndexVec::new(),
273-
),
274-
))),
273+
)
274+
}
275+
// `async gen` continues to return `Poll::Ready(None)`
276+
CoroutineKind::AsyncGen(_) => {
277+
let ty::Adt(_poll_adt, args) = *self.old_yield_ty.kind() else { bug!() };
278+
let ty::Adt(_option_adt, args) = *args.type_at(0).kind() else { bug!() };
279+
let yield_ty = args.type_at(0);
280+
Rvalue::Use(Operand::Constant(Box::new(ConstOperand {
281+
span: source_info.span,
282+
const_: Const::Unevaluated(
283+
UnevaluatedConst::new(
284+
self.tcx.require_lang_item(LangItem::AsyncGenFinished, None),
285+
self.tcx.mk_args(&[yield_ty.into()]),
286+
),
287+
self.old_yield_ty,
288+
),
289+
user_ty: None,
290+
})))
291+
}
292+
};
293+
294+
let statements = vec![Statement {
295+
kind: StatementKind::Assign(Box::new((Place::return_place(), none_value))),
275296
source_info,
276297
}];
277298

@@ -1393,11 +1414,12 @@ fn create_coroutine_resume_function<'tcx>(
13931414

13941415
if can_return {
13951416
let block = match coroutine_kind {
1396-
// FIXME(gen_blocks): Should `async gen` yield `None` when resumed once again?
1397-
CoroutineKind::Async(_) | CoroutineKind::AsyncGen(_) | CoroutineKind::Coroutine => {
1417+
CoroutineKind::Async(_) | CoroutineKind::Coroutine => {
13981418
insert_panic_block(tcx, body, ResumedAfterReturn(coroutine_kind))
13991419
}
1400-
CoroutineKind::Gen(_) => transform.insert_none_ret_block(body),
1420+
CoroutineKind::AsyncGen(_) | CoroutineKind::Gen(_) => {
1421+
transform.insert_none_ret_block(body)
1422+
}
14011423
};
14021424
cases.insert(1, (RETURNED, block));
14031425
}

‎compiler/rustc_mir_transform/src/coverage/spans.rs

Lines changed: 16 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -319,29 +319,16 @@ impl<'a> CoverageSpansGenerator<'a> {
319319
}
320320
}
321321

322-
let prev = self.take_prev();
323-
debug!(" AT END, adding last prev={prev:?}");
324-
325322
// Drain any remaining dups into the output.
326323
for dup in self.pending_dups.drain(..) {
327324
debug!(" ...adding at least one pending dup={:?}", dup);
328325
self.refined_spans.push(dup);
329326
}
330327

331-
// Async functions wrap a closure that implements the body to be executed. The enclosing
332-
// function is called and returns an `impl Future` without initially executing any of the
333-
// body. To avoid showing the return from the enclosing function as a "covered" return from
334-
// the closure, the enclosing function's `TerminatorKind::Return`s `CoverageSpan` is
335-
// excluded. The closure's `Return` is the only one that will be counted. This provides
336-
// adequate coverage, and more intuitive counts. (Avoids double-counting the closing brace
337-
// of the function body.)
338-
let body_ends_with_closure = if let Some(last_covspan) = self.refined_spans.last() {
339-
last_covspan.is_closure && last_covspan.span.hi() == self.body_span.hi()
340-
} else {
341-
false
342-
};
343-
344-
if !body_ends_with_closure {
328+
// There is usually a final span remaining in `prev` after the loop ends,
329+
// so add it to the output as well.
330+
if let Some(prev) = self.some_prev.take() {
331+
debug!(" AT END, adding last prev={prev:?}");
345332
self.refined_spans.push(prev);
346333
}
347334

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

388+
#[track_caller]
401389
fn curr(&self) -> &CoverageSpan {
402-
self.some_curr
403-
.as_ref()
404-
.unwrap_or_else(|| bug!("invalid attempt to unwrap a None some_curr"))
390+
self.some_curr.as_ref().unwrap_or_else(|| bug!("some_curr is None (curr)"))
405391
}
406392

393+
#[track_caller]
407394
fn curr_mut(&mut self) -> &mut CoverageSpan {
408-
self.some_curr
409-
.as_mut()
410-
.unwrap_or_else(|| bug!("invalid attempt to unwrap a None some_curr"))
395+
self.some_curr.as_mut().unwrap_or_else(|| bug!("some_curr is None (curr_mut)"))
411396
}
412397

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

405+
#[track_caller]
419406
fn prev(&self) -> &CoverageSpan {
420-
self.some_prev
421-
.as_ref()
422-
.unwrap_or_else(|| bug!("invalid attempt to unwrap a None some_prev"))
407+
self.some_prev.as_ref().unwrap_or_else(|| bug!("some_prev is None (prev)"))
423408
}
424409

410+
#[track_caller]
425411
fn prev_mut(&mut self) -> &mut CoverageSpan {
426-
self.some_prev
427-
.as_mut()
428-
.unwrap_or_else(|| bug!("invalid attempt to unwrap a None some_prev"))
412+
self.some_prev.as_mut().unwrap_or_else(|| bug!("some_prev is None (prev_mut)"))
429413
}
430414

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

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

0 commit comments

Comments
 (0)