Skip to content

Commit

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

Rollup of 8 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#118756 (use bold magenta instead of bold white for highlighting)
 - 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 608f324 + 8230fd5 commit 3deea35
Show file tree
Hide file tree
Showing 44 changed files with 611 additions and 133 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
22 changes: 11 additions & 11 deletions compiler/rustc_errors/src/emitter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2674,6 +2674,14 @@ fn from_stderr(color: ColorConfig) -> Destination {
}
}

/// On Windows, BRIGHT_BLUE is hard to read on black. Use cyan instead.
///
/// See #36178.
#[cfg(windows)]
const BRIGHT_BLUE: Color = Color::Cyan;
#[cfg(not(windows))]
const BRIGHT_BLUE: Color = Color::Blue;

impl Style {
fn color_spec(&self, lvl: Level) -> ColorSpec {
let mut spec = ColorSpec::new();
Expand All @@ -2688,11 +2696,7 @@ impl Style {
Style::LineNumber => {
spec.set_bold(true);
spec.set_intense(true);
if cfg!(windows) {
spec.set_fg(Some(Color::Cyan));
} else {
spec.set_fg(Some(Color::Blue));
}
spec.set_fg(Some(BRIGHT_BLUE));
}
Style::Quotation => {}
Style::MainHeaderMsg => {
Expand All @@ -2707,19 +2711,15 @@ impl Style {
}
Style::UnderlineSecondary | Style::LabelSecondary => {
spec.set_bold(true).set_intense(true);
if cfg!(windows) {
spec.set_fg(Some(Color::Cyan));
} else {
spec.set_fg(Some(Color::Blue));
}
spec.set_fg(Some(BRIGHT_BLUE));
}
Style::HeaderMsg | Style::NoStyle => {}
Style::Level(lvl) => {
spec = lvl.color();
spec.set_bold(true);
}
Style::Highlight => {
spec.set_bold(true);
spec.set_bold(true).set_fg(Some(Color::Magenta));
}
}
spec
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
Loading

0 comments on commit 3deea35

Please sign in to comment.