diff options
Diffstat (limited to 'compiler')
19 files changed, 387 insertions, 574 deletions
diff --git a/compiler/rustc_ast/src/mut_visit.rs b/compiler/rustc_ast/src/mut_visit.rs index 196fcc1af30..622c260868e 100644 --- a/compiler/rustc_ast/src/mut_visit.rs +++ b/compiler/rustc_ast/src/mut_visit.rs @@ -1625,9 +1625,10 @@ pub fn walk_expr<T: MutVisitor>(vis: &mut T, Expr { kind, id, span, attrs, token visit_thin_exprs(vis, call_args); vis.visit_span(span); } - ExprKind::Binary(_binop, lhs, rhs) => { + ExprKind::Binary(binop, lhs, rhs) => { vis.visit_expr(lhs); vis.visit_expr(rhs); + vis.visit_span(&mut binop.span); } ExprKind::Unary(_unop, ohs) => vis.visit_expr(ohs), ExprKind::Cast(expr, ty) => { @@ -1785,20 +1786,21 @@ pub fn noop_filter_map_expr<T: MutVisitor>(vis: &mut T, mut e: P<Expr>) -> Optio pub fn walk_flat_map_stmt<T: MutVisitor>( vis: &mut T, - Stmt { kind, mut span, mut id }: Stmt, + Stmt { kind, span, mut id }: Stmt, ) -> SmallVec<[Stmt; 1]> { vis.visit_id(&mut id); - let stmts: SmallVec<_> = walk_flat_map_stmt_kind(vis, kind) + let mut stmts: SmallVec<[Stmt; 1]> = walk_flat_map_stmt_kind(vis, kind) .into_iter() .map(|kind| Stmt { id, kind, span }) .collect(); - if stmts.len() > 1 { - panic!( + match stmts.len() { + 0 => {} + 1 => vis.visit_span(&mut stmts[0].span), + 2.. => panic!( "cloning statement `NodeId`s is prohibited by default, \ the visitor should implement custom statement visiting" - ); + ), } - vis.visit_span(&mut span); stmts } diff --git a/compiler/rustc_ast_passes/messages.ftl b/compiler/rustc_ast_passes/messages.ftl index d81fd53938b..5a0ec865f9d 100644 --- a/compiler/rustc_ast_passes/messages.ftl +++ b/compiler/rustc_ast_passes/messages.ftl @@ -207,8 +207,6 @@ ast_passes_precise_capturing_duplicated = duplicate `use<...>` precise capturing ast_passes_precise_capturing_not_allowed_here = `use<...>` precise capturing syntax not allowed in {$loc} -ast_passes_show_span = {$msg} - ast_passes_stability_outside_std = stability attributes may not be used outside of the standard library ast_passes_static_without_body = diff --git a/compiler/rustc_ast_passes/src/errors.rs b/compiler/rustc_ast_passes/src/errors.rs index f65056a494b..9b600e3ee92 100644 --- a/compiler/rustc_ast_passes/src/errors.rs +++ b/compiler/rustc_ast_passes/src/errors.rs @@ -780,14 +780,6 @@ pub(crate) struct IncompatibleFeatures { } #[derive(Diagnostic)] -#[diag(ast_passes_show_span)] -pub(crate) struct ShowSpan { - #[primary_span] - pub span: Span, - pub msg: &'static str, -} - -#[derive(Diagnostic)] #[diag(ast_passes_negative_bound_not_supported)] pub(crate) struct NegativeBoundUnsupported { #[primary_span] diff --git a/compiler/rustc_ast_passes/src/lib.rs b/compiler/rustc_ast_passes/src/lib.rs index 86752da79ae..b4ed70d83e5 100644 --- a/compiler/rustc_ast_passes/src/lib.rs +++ b/compiler/rustc_ast_passes/src/lib.rs @@ -1,8 +1,6 @@ //! The `rustc_ast_passes` crate contains passes which validate the AST in `syntax` //! parsed by `rustc_parse` and then lowered, after the passes in this crate, //! by `rustc_ast_lowering`. -//! -//! The crate also contains other misc AST visitors, e.g. `node_count` and `show_span`. // tidy-alphabetical-start #![allow(internal_features)] @@ -18,6 +16,5 @@ pub mod ast_validation; mod errors; pub mod feature_gate; -pub mod show_span; rustc_fluent_macro::fluent_messages! { "../messages.ftl" } diff --git a/compiler/rustc_ast_passes/src/show_span.rs b/compiler/rustc_ast_passes/src/show_span.rs deleted file mode 100644 index e7ba2e7fc30..00000000000 --- a/compiler/rustc_ast_passes/src/show_span.rs +++ /dev/null @@ -1,68 +0,0 @@ -//! Span debugger -//! -//! This module shows spans for all expressions in the crate -//! to help with compiler debugging. - -use std::str::FromStr; - -use rustc_ast as ast; -use rustc_ast::visit; -use rustc_ast::visit::Visitor; -use rustc_errors::DiagCtxtHandle; - -use crate::errors; - -enum Mode { - Expression, - Pattern, - Type, -} - -impl FromStr for Mode { - type Err = (); - fn from_str(s: &str) -> Result<Mode, ()> { - let mode = match s { - "expr" => Mode::Expression, - "pat" => Mode::Pattern, - "ty" => Mode::Type, - _ => return Err(()), - }; - Ok(mode) - } -} - -struct ShowSpanVisitor<'a> { - dcx: DiagCtxtHandle<'a>, - mode: Mode, -} - -impl<'a> Visitor<'a> for ShowSpanVisitor<'a> { - fn visit_expr(&mut self, e: &'a ast::Expr) { - if let Mode::Expression = self.mode { - self.dcx.emit_warn(errors::ShowSpan { span: e.span, msg: "expression" }); - } - visit::walk_expr(self, e); - } - - fn visit_pat(&mut self, p: &'a ast::Pat) { - if let Mode::Pattern = self.mode { - self.dcx.emit_warn(errors::ShowSpan { span: p.span, msg: "pattern" }); - } - visit::walk_pat(self, p); - } - - fn visit_ty(&mut self, t: &'a ast::Ty) { - if let Mode::Type = self.mode { - self.dcx.emit_warn(errors::ShowSpan { span: t.span, msg: "type" }); - } - visit::walk_ty(self, t); - } -} - -pub fn run(dcx: DiagCtxtHandle<'_>, mode: &str, krate: &ast::Crate) { - let Ok(mode) = mode.parse() else { - return; - }; - let mut v = ShowSpanVisitor { dcx, mode }; - visit::walk_crate(&mut v, krate); -} diff --git a/compiler/rustc_borrowck/src/region_infer/mod.rs b/compiler/rustc_borrowck/src/region_infer/mod.rs index 99af5500ac6..0eecf98a6ed 100644 --- a/compiler/rustc_borrowck/src/region_infer/mod.rs +++ b/compiler/rustc_borrowck/src/region_infer/mod.rs @@ -973,25 +973,20 @@ impl<'tcx> RegionInferenceContext<'tcx> { propagated_outlives_requirements: &mut Vec<ClosureOutlivesRequirement<'tcx>>, ) -> bool { let tcx = infcx.tcx; - - let TypeTest { generic_kind, lower_bound, span: _, verify_bound: _ } = type_test; + let TypeTest { generic_kind, lower_bound, span: blame_span, ref verify_bound } = *type_test; let generic_ty = generic_kind.to_ty(tcx); let Some(subject) = self.try_promote_type_test_subject(infcx, generic_ty) else { return false; }; - debug!("subject = {:?}", subject); - - let r_scc = self.constraint_sccs.scc(*lower_bound); - + let r_scc = self.constraint_sccs.scc(lower_bound); debug!( "lower_bound = {:?} r_scc={:?} universe={:?}", lower_bound, r_scc, self.constraint_sccs.annotation(r_scc).min_universe() ); - // If the type test requires that `T: 'a` where `'a` is a // placeholder from another universe, that effectively requires // `T: 'static`, so we have to propagate that requirement. @@ -1004,7 +999,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { propagated_outlives_requirements.push(ClosureOutlivesRequirement { subject, outlived_free_region: static_r, - blame_span: type_test.span, + blame_span, category: ConstraintCategory::Boring, }); @@ -1031,12 +1026,12 @@ impl<'tcx> RegionInferenceContext<'tcx> { // where `ur` is a local bound -- we are sometimes in a // position to prove things that our caller cannot. See // #53570 for an example. - if self.eval_verify_bound(infcx, generic_ty, ur, &type_test.verify_bound) { + if self.eval_verify_bound(infcx, generic_ty, ur, &verify_bound) { continue; } let non_local_ub = self.universal_region_relations.non_local_upper_bounds(ur); - debug!("try_promote_type_test: non_local_ub={:?}", non_local_ub); + debug!(?non_local_ub); // This is slightly too conservative. To show T: '1, given `'2: '1` // and `'3: '1` we only need to prove that T: '2 *or* T: '3, but to @@ -1049,10 +1044,10 @@ impl<'tcx> RegionInferenceContext<'tcx> { let requirement = ClosureOutlivesRequirement { subject, outlived_free_region: upper_bound, - blame_span: type_test.span, + blame_span, category: ConstraintCategory::Boring, }; - debug!("try_promote_type_test: pushing {:#?}", requirement); + debug!(?requirement, "adding closure requirement"); propagated_outlives_requirements.push(requirement); } } @@ -1063,44 +1058,14 @@ impl<'tcx> RegionInferenceContext<'tcx> { /// variables in the type `T` with an equal universal region from the /// closure signature. /// This is not always possible, so this is a fallible process. - #[instrument(level = "debug", skip(self, infcx))] + #[instrument(level = "debug", skip(self, infcx), ret)] fn try_promote_type_test_subject( &self, infcx: &InferCtxt<'tcx>, ty: Ty<'tcx>, ) -> Option<ClosureOutlivesSubject<'tcx>> { let tcx = infcx.tcx; - - // Opaque types' args may include useless lifetimes. - // We will replace them with ReStatic. - struct OpaqueFolder<'tcx> { - tcx: TyCtxt<'tcx>, - } - impl<'tcx> ty::TypeFolder<TyCtxt<'tcx>> for OpaqueFolder<'tcx> { - fn cx(&self) -> TyCtxt<'tcx> { - self.tcx - } - fn fold_ty(&mut self, t: Ty<'tcx>) -> Ty<'tcx> { - use ty::TypeSuperFoldable as _; - let tcx = self.tcx; - let &ty::Alias(ty::Opaque, ty::AliasTy { args, def_id, .. }) = t.kind() else { - return t.super_fold_with(self); - }; - let args = std::iter::zip(args, tcx.variances_of(def_id)).map(|(arg, v)| { - match (arg.unpack(), v) { - (ty::GenericArgKind::Lifetime(_), ty::Bivariant) => { - tcx.lifetimes.re_static.into() - } - _ => arg.fold_with(self), - } - }); - Ty::new_opaque(tcx, def_id, tcx.mk_args_from_iter(args)) - } - } - - let ty = ty.fold_with(&mut OpaqueFolder { tcx }); let mut failed = false; - let ty = fold_regions(tcx, ty, |r, _depth| { let r_vid = self.to_region_vid(r); let r_scc = self.constraint_sccs.scc(r_vid); diff --git a/compiler/rustc_builtin_macros/messages.ftl b/compiler/rustc_builtin_macros/messages.ftl index 6ebc2fd870c..c05d44cb452 100644 --- a/compiler/rustc_builtin_macros/messages.ftl +++ b/compiler/rustc_builtin_macros/messages.ftl @@ -94,6 +94,21 @@ builtin_macros_cfg_accessible_indeterminate = cannot determine whether the path builtin_macros_cfg_accessible_literal_path = `cfg_accessible` path cannot be a literal builtin_macros_cfg_accessible_multiple_paths = multiple `cfg_accessible` paths are specified builtin_macros_cfg_accessible_unspecified_path = `cfg_accessible` path is not specified + +builtin_macros_coerce_pointee_requires_maybe_sized = `derive(CoercePointee)` requires `{$name}` to be marked `?Sized` + +builtin_macros_coerce_pointee_requires_one_field = `CoercePointee` can only be derived on `struct`s with at least one field + +builtin_macros_coerce_pointee_requires_one_generic = `CoercePointee` can only be derived on `struct`s that are generic over at least one type + +builtin_macros_coerce_pointee_requires_one_pointee = exactly one generic type parameter must be marked as `#[pointee]` to derive `CoercePointee` traits + +builtin_macros_coerce_pointee_requires_transparent = `CoercePointee` can only be derived on `struct`s with `#[repr(transparent)]` + +builtin_macros_coerce_pointee_too_many_pointees = only one type parameter can be marked as `#[pointee]` when deriving `CoercePointee` traits + .label = here another type parameter is marked as `#[pointee]` + + builtin_macros_concat_bytes_array = cannot concatenate doubly nested array .note = byte strings are treated as arrays of bytes .help = try flattening the array diff --git a/compiler/rustc_builtin_macros/src/deriving/coerce_pointee.rs b/compiler/rustc_builtin_macros/src/deriving/coerce_pointee.rs index 8adb9a3f4b0..3bd8f899a4a 100644 --- a/compiler/rustc_builtin_macros/src/deriving/coerce_pointee.rs +++ b/compiler/rustc_builtin_macros/src/deriving/coerce_pointee.rs @@ -9,6 +9,7 @@ use rustc_ast::{ use rustc_attr as attr; use rustc_data_structures::flat_map_in_place::FlatMapInPlace; use rustc_expand::base::{Annotatable, ExtCtxt}; +use rustc_macros::Diagnostic; use rustc_span::symbol::{Ident, sym}; use rustc_span::{Span, Symbol}; use thin_vec::{ThinVec, thin_vec}; @@ -38,12 +39,7 @@ pub(crate) fn expand_deriving_coerce_pointee( .any(|r| matches!(r, attr::ReprTransparent)) }); if !is_transparent { - cx.dcx() - .struct_span_err( - span, - "`CoercePointee` can only be derived on `struct`s with `#[repr(transparent)]`", - ) - .emit(); + cx.dcx().emit_err(RequireTransparent { span }); return; } if !matches!( @@ -51,22 +47,12 @@ pub(crate) fn expand_deriving_coerce_pointee( VariantData::Struct { fields, recovered: _ } | VariantData::Tuple(fields, _) if !fields.is_empty()) { - cx.dcx() - .struct_span_err( - span, - "`CoercePointee` can only be derived on `struct`s with at least one field", - ) - .emit(); + cx.dcx().emit_err(RequireOneField { span }); return; } (aitem.ident, g) } else { - cx.dcx() - .struct_span_err( - span, - "`CoercePointee` can only be derived on `struct`s with `#[repr(transparent)]`", - ) - .emit(); + cx.dcx().emit_err(RequireTransparent { span }); return; }; @@ -95,10 +81,7 @@ pub(crate) fn expand_deriving_coerce_pointee( let pointee_param_idx = if type_params.is_empty() { // `#[derive(CoercePointee)]` requires at least one generic type on the target `struct` - cx.dcx().struct_span_err( - span, - "`CoercePointee` can only be derived on `struct`s that are generic over at least one type", - ).emit(); + cx.dcx().emit_err(RequireOneGeneric { span }); return; } else if type_params.len() == 1 { // Regardless of the only type param being designed as `#[pointee]` or not, we can just use it as such @@ -111,19 +94,11 @@ pub(crate) fn expand_deriving_coerce_pointee( match (pointees.next(), pointees.next()) { (Some((idx, _span)), None) => idx, (None, _) => { - cx.dcx().struct_span_err( - span, - "exactly one generic type parameter must be marked as #[pointee] to derive CoercePointee traits", - ).emit(); + cx.dcx().emit_err(RequireOnePointee { span }); return; } (Some((_, one)), Some((_, another))) => { - cx.dcx() - .struct_span_err( - vec![one, another], - "only one type parameter can be marked as `#[pointee]` when deriving CoercePointee traits", - ) - .emit(); + cx.dcx().emit_err(TooManyPointees { one, another }); return; } } @@ -181,15 +156,10 @@ pub(crate) fn expand_deriving_coerce_pointee( pointee_ty_ident.name, ) { - cx.dcx() - .struct_span_err( - pointee_ty_ident.span, - format!( - "`derive(CoercePointee)` requires {} to be marked `?Sized`", - pointee_ty_ident.name - ), - ) - .emit(); + cx.dcx().emit_err(RequiresMaybeSized { + span: pointee_ty_ident.span, + name: pointee_ty_ident.name.to_ident_string(), + }); return; } let arg = GenericArg::Type(s_ty.clone()); @@ -459,3 +429,48 @@ impl<'a, 'b> rustc_ast::visit::Visitor<'a> for AlwaysErrorOnGenericParam<'a, 'b> } } } + +#[derive(Diagnostic)] +#[diag(builtin_macros_coerce_pointee_requires_transparent)] +struct RequireTransparent { + #[primary_span] + span: Span, +} + +#[derive(Diagnostic)] +#[diag(builtin_macros_coerce_pointee_requires_one_field)] +struct RequireOneField { + #[primary_span] + span: Span, +} + +#[derive(Diagnostic)] +#[diag(builtin_macros_coerce_pointee_requires_one_generic)] +struct RequireOneGeneric { + #[primary_span] + span: Span, +} + +#[derive(Diagnostic)] +#[diag(builtin_macros_coerce_pointee_requires_one_pointee)] +struct RequireOnePointee { + #[primary_span] + span: Span, +} + +#[derive(Diagnostic)] +#[diag(builtin_macros_coerce_pointee_too_many_pointees)] +struct TooManyPointees { + #[primary_span] + one: Span, + #[label] + another: Span, +} + +#[derive(Diagnostic)] +#[diag(builtin_macros_coerce_pointee_requires_maybe_sized)] +struct RequiresMaybeSized { + #[primary_span] + span: Span, + name: String, +} diff --git a/compiler/rustc_driver_impl/src/lib.rs b/compiler/rustc_driver_impl/src/lib.rs index 5b472bb9b81..b7d64f75bf3 100644 --- a/compiler/rustc_driver_impl/src/lib.rs +++ b/compiler/rustc_driver_impl/src/lib.rs @@ -418,9 +418,7 @@ fn run_compiler( return early_exit(); } - if sess.opts.unstable_opts.parse_crate_root_only - || sess.opts.unstable_opts.show_span.is_some() - { + if sess.opts.unstable_opts.parse_crate_root_only { return early_exit(); } diff --git a/compiler/rustc_hir_analysis/src/collect.rs b/compiler/rustc_hir_analysis/src/collect.rs index 8d5824130d8..a4636da3f62 100644 --- a/compiler/rustc_hir_analysis/src/collect.rs +++ b/compiler/rustc_hir_analysis/src/collect.rs @@ -309,10 +309,10 @@ impl<'tcx> Visitor<'tcx> for CollectItemTypesVisitor<'tcx> { self.tcx.ensure().type_of(param.def_id); if let Some(default) = default { // need to store default and type of default + self.tcx.ensure().const_param_default(param.def_id); if let hir::ConstArgKind::Anon(ac) = default.kind { self.tcx.ensure().type_of(ac.def_id); } - self.tcx.ensure().const_param_default(param.def_id); } } } @@ -1817,7 +1817,6 @@ fn const_param_default<'tcx>( ), }; let icx = ItemCtxt::new(tcx, def_id); - // FIXME(const_generics): investigate which places do and don't need const ty feeding - let ct = icx.lowerer().lower_const_arg(default_ct, FeedConstTy::No); + let ct = icx.lowerer().lower_const_arg(default_ct, FeedConstTy::Param(def_id.to_def_id())); ty::EarlyBinder::bind(ct) } diff --git a/compiler/rustc_hir_analysis/src/collect/type_of.rs b/compiler/rustc_hir_analysis/src/collect/type_of.rs index 7cf99bebd36..72d5b3ac4f5 100644 --- a/compiler/rustc_hir_analysis/src/collect/type_of.rs +++ b/compiler/rustc_hir_analysis/src/collect/type_of.rs @@ -12,7 +12,6 @@ use rustc_middle::ty::{self, Article, IsSuggestable, Ty, TyCtxt, TypeVisitableEx use rustc_middle::{bug, span_bug}; use rustc_span::symbol::Ident; use rustc_span::{DUMMY_SP, Span}; -use tracing::debug; use super::{ItemCtxt, bad_placeholder}; use crate::errors::TypeofReservedKeywordUsed; @@ -138,252 +137,26 @@ fn const_arg_anon_type_of<'tcx>(tcx: TyCtxt<'tcx>, arg_hir_id: HirId, span: Span use hir::*; use rustc_middle::ty::Ty; - let parent_node_id = tcx.parent_hir_id(arg_hir_id); - let parent_node = tcx.hir_node(parent_node_id); - - let (generics, arg_idx) = match parent_node { - // Easy case: arrays repeat expressions. + match tcx.parent_hir_node(arg_hir_id) { + // Array length const arguments do not have `type_of` fed as there is never a corresponding + // generic parameter definition. Node::Ty(&hir::Ty { kind: TyKind::Array(_, ref constant), .. }) | Node::Expr(&Expr { kind: ExprKind::Repeat(_, ref constant), .. }) if constant.hir_id == arg_hir_id => { return tcx.types.usize; } - Node::GenericParam(&GenericParam { - def_id: param_def_id, - kind: GenericParamKind::Const { default: Some(ct), .. }, - .. - }) if ct.hir_id == arg_hir_id => { - return tcx - .type_of(param_def_id) - .no_bound_vars() - .expect("const parameter types cannot be generic"); - } - - // This match arm is for when the def_id appears in a GAT whose - // path can't be resolved without typechecking e.g. - // - // trait Foo { - // type Assoc<const N: usize>; - // fn foo() -> Self::Assoc<3>; - // } - // - // In the above code we would call this query with the def_id of 3 and - // the parent_node we match on would be the hir node for Self::Assoc<3> - // - // `Self::Assoc<3>` cant be resolved without typechecking here as we - // didnt write <Self as Foo>::Assoc<3>. If we did then another match - // arm would handle this. - // - // I believe this match arm is only needed for GAT but I am not 100% sure - BoxyUwU - Node::Ty(hir_ty @ hir::Ty { kind: TyKind::Path(QPath::TypeRelative(ty, segment)), .. }) => { - // Find the Item containing the associated type so we can create an ItemCtxt. - // Using the ItemCtxt lower the HIR for the unresolved assoc type into a - // ty which is a fully resolved projection. - // For the code example above, this would mean lowering `Self::Assoc<3>` - // to a ty::Alias(ty::Projection, `<Self as Foo>::Assoc<3>`). - let item_def_id = tcx.hir().get_parent_item(ty.hir_id).def_id; - let ty = ItemCtxt::new(tcx, item_def_id).lower_ty(hir_ty); - - // Iterate through the generics of the projection to find the one that corresponds to - // the def_id that this query was called with. We filter to only type and const args here - // as a precaution for if it's ever allowed to elide lifetimes in GAT's. It currently isn't - // but it can't hurt to be safe ^^ - if let ty::Alias(ty::Projection | ty::Inherent, projection) = ty.kind() { - let generics = tcx.generics_of(projection.def_id); - - let arg_index = segment - .args - .and_then(|args| { - args.args - .iter() - .filter(|arg| arg.is_ty_or_const()) - .position(|arg| arg.hir_id() == arg_hir_id) - }) - .unwrap_or_else(|| { - bug!("no arg matching AnonConst in segment"); - }); - - (generics, arg_index) - } else { - // I dont think it's possible to reach this but I'm not 100% sure - BoxyUwU - return Ty::new_error_with_message( - tcx, - span, - "unexpected non-GAT usage of an anon const", - ); - } - } - Node::Expr(&Expr { - kind: - ExprKind::MethodCall(segment, ..) | ExprKind::Path(QPath::TypeRelative(_, segment)), - .. - }) => { - let body_owner = tcx.hir().enclosing_body_owner(arg_hir_id); - let tables = tcx.typeck(body_owner); - // This may fail in case the method/path does not actually exist. - // As there is no relevant param for `def_id`, we simply return - // `None` here. - let Some(type_dependent_def) = tables.type_dependent_def_id(parent_node_id) else { - return Ty::new_error_with_message( - tcx, - span, - format!("unable to find type-dependent def for {parent_node_id:?}"), - ); - }; - let idx = segment - .args - .and_then(|args| { - args.args - .iter() - .filter(|arg| arg.is_ty_or_const()) - .position(|arg| arg.hir_id() == arg_hir_id) - }) - .unwrap_or_else(|| { - bug!("no arg matching ConstArg in segment"); - }); - - (tcx.generics_of(type_dependent_def), idx) - } - - Node::Ty(&hir::Ty { kind: TyKind::Path(_), .. }) - | Node::Expr(&Expr { kind: ExprKind::Path(_) | ExprKind::Struct(..), .. }) - | Node::TraitRef(..) - | Node::Pat(_) => { - let path = match parent_node { - Node::Ty(&hir::Ty { kind: TyKind::Path(QPath::Resolved(_, path)), .. }) - | Node::TraitRef(&TraitRef { path, .. }) => &*path, - Node::Expr(&Expr { - kind: - ExprKind::Path(QPath::Resolved(_, path)) - | ExprKind::Struct(&QPath::Resolved(_, path), ..), - .. - }) => { - let body_owner = tcx.hir().enclosing_body_owner(arg_hir_id); - let _tables = tcx.typeck(body_owner); - &*path - } - Node::Pat(pat) => { - if let Some(path) = get_path_containing_arg_in_pat(pat, arg_hir_id) { - path - } else { - return Ty::new_error_with_message( - tcx, - span, - format!("unable to find const parent for {arg_hir_id} in pat {pat:?}"), - ); - } - } - _ => { - return Ty::new_error_with_message( - tcx, - span, - format!("unexpected const parent path {parent_node:?}"), - ); - } - }; - - // We've encountered an `AnonConst` in some path, so we need to - // figure out which generic parameter it corresponds to and return - // the relevant type. - let Some((arg_index, segment)) = path.segments.iter().find_map(|seg| { - let args = seg.args?; - args.args - .iter() - .filter(|arg| arg.is_ty_or_const()) - .position(|arg| arg.hir_id() == arg_hir_id) - .map(|index| (index, seg)) - .or_else(|| { - args.constraints - .iter() - .copied() - .filter_map(AssocItemConstraint::ct) - .position(|ct| ct.hir_id == arg_hir_id) - .map(|idx| (idx, seg)) - }) - }) else { - return Ty::new_error_with_message(tcx, span, "no arg matching AnonConst in path"); - }; - - let generics = match tcx.res_generics_def_id(segment.res) { - Some(def_id) => tcx.generics_of(def_id), - None => { - return Ty::new_error_with_message( - tcx, - span, - format!("unexpected anon const res {:?} in path: {:?}", segment.res, path), - ); - } - }; - - (generics, arg_index) - } - _ => { - return Ty::new_error_with_message( - tcx, - span, - format!("unexpected const arg parent in type_of(): {parent_node:?}"), - ); - } - }; - - debug!(?parent_node); - debug!(?generics, ?arg_idx); - if let Some(param_def_id) = generics - .own_params - .iter() - .filter(|param| param.kind.is_ty_or_const()) - .nth(match generics.has_self && generics.parent.is_none() { - true => arg_idx + 1, - false => arg_idx, - }) - .and_then(|param| match param.kind { - ty::GenericParamDefKind::Const { .. } => { - debug!(?param); - Some(param.def_id) - } - _ => None, - }) - { - tcx.type_of(param_def_id).no_bound_vars().expect("const parameter types cannot be generic") - } else { - return Ty::new_error_with_message( + // This is not a `bug!` as const arguments in path segments that did not resolve to anything + // will result in `type_of` never being fed. + _ => Ty::new_error_with_message( tcx, span, - format!("const generic parameter not found in {generics:?} at position {arg_idx:?}"), - ); + "`type_of` called on const argument's anon const before the const argument was lowered", + ), } } -fn get_path_containing_arg_in_pat<'hir>( - pat: &'hir hir::Pat<'hir>, - arg_id: HirId, -) -> Option<&'hir hir::Path<'hir>> { - use hir::*; - - let is_arg_in_path = |p: &hir::Path<'_>| { - p.segments - .iter() - .filter_map(|seg| seg.args) - .flat_map(|args| args.args) - .any(|arg| arg.hir_id() == arg_id) - }; - let mut arg_path = None; - pat.walk(|pat| match pat.kind { - PatKind::Struct(QPath::Resolved(_, path), _, _) - | PatKind::TupleStruct(QPath::Resolved(_, path), _, _) - | PatKind::Path(QPath::Resolved(_, path)) - if is_arg_in_path(path) => - { - arg_path = Some(path); - false - } - _ => true, - }); - arg_path -} - pub(super) fn type_of(tcx: TyCtxt<'_>, def_id: LocalDefId) -> ty::EarlyBinder<'_, Ty<'_>> { use rustc_hir::*; use rustc_middle::ty::Ty; diff --git a/compiler/rustc_hir_analysis/src/collect/type_of/opaque.rs b/compiler/rustc_hir_analysis/src/collect/type_of/opaque.rs index fcea0052546..66255829dcf 100644 --- a/compiler/rustc_hir_analysis/src/collect/type_of/opaque.rs +++ b/compiler/rustc_hir_analysis/src/collect/type_of/opaque.rs @@ -226,13 +226,18 @@ impl TaitConstraintLocator<'_> { constrained = true; if !opaque_types_defined_by.contains(&self.def_id) { - self.tcx.dcx().emit_err(TaitForwardCompat { + let guar = self.tcx.dcx().emit_err(TaitForwardCompat { span: hidden_type.span, item_span: self .tcx .def_ident_span(item_def_id) .unwrap_or_else(|| self.tcx.def_span(item_def_id)), }); + // Avoid "opaque type not constrained" errors on the opaque itself. + self.found = Some(ty::OpaqueHiddenType { + span: DUMMY_SP, + ty: Ty::new_error(self.tcx, guar), + }); } let concrete_type = self.tcx.erase_regions(hidden_type.remap_generic_params_to_declaration_params( @@ -248,7 +253,7 @@ impl TaitConstraintLocator<'_> { if !constrained { debug!("no constraints in typeck results"); if opaque_types_defined_by.contains(&self.def_id) { - self.tcx.dcx().emit_err(TaitForwardCompat2 { + let guar = self.tcx.dcx().emit_err(TaitForwardCompat2 { span: self .tcx .def_ident_span(item_def_id) @@ -256,6 +261,11 @@ impl TaitConstraintLocator<'_> { opaque_type_span: self.tcx.def_span(self.def_id), opaque_type: self.tcx.def_path_str(self.def_id), }); + // Avoid "opaque type not constrained" errors on the opaque itself. + self.found = Some(ty::OpaqueHiddenType { + span: DUMMY_SP, + ty: Ty::new_error(self.tcx, guar), + }); } return; }; diff --git a/compiler/rustc_interface/src/passes.rs b/compiler/rustc_interface/src/passes.rs index 06e27ed8060..d42915f4110 100644 --- a/compiler/rustc_interface/src/passes.rs +++ b/compiler/rustc_interface/src/passes.rs @@ -54,10 +54,6 @@ pub(crate) fn parse<'a>(sess: &'a Session) -> Result<ast::Crate> { }) .map_err(|parse_error| parse_error.emit())?; - if let Some(ref s) = sess.opts.unstable_opts.show_span { - rustc_ast_passes::show_span::run(sess.dcx(), s, &krate); - } - if sess.opts.unstable_opts.input_stats { input_stats::print_ast_stats(&krate, "PRE EXPANSION AST STATS", "ast-stats-1"); } diff --git a/compiler/rustc_interface/src/tests.rs b/compiler/rustc_interface/src/tests.rs index 9384ed925c7..3c4d9c2e928 100644 --- a/compiler/rustc_interface/src/tests.rs +++ b/compiler/rustc_interface/src/tests.rs @@ -844,7 +844,6 @@ fn test_unstable_options_tracking_hash() { tracked!(sanitizer_recover, SanitizerSet::ADDRESS); tracked!(saturating_float_casts, Some(true)); tracked!(share_generics, Some(true)); - tracked!(show_span, Some(String::from("abc"))); tracked!(simulate_remapped_rust_src_base, Some(PathBuf::from("/rustc/abc"))); tracked!(small_data_threshold, Some(16)); tracked!(split_lto_unit, Some(true)); diff --git a/compiler/rustc_metadata/src/rmeta/table.rs b/compiler/rustc_metadata/src/rmeta/table.rs index 9438350ca09..28f406fbc96 100644 --- a/compiler/rustc_metadata/src/rmeta/table.rs +++ b/compiler/rustc_metadata/src/rmeta/table.rs @@ -1,6 +1,5 @@ use rustc_hir::def::CtorOf; use rustc_index::Idx; -use tracing::trace; use crate::rmeta::*; @@ -530,8 +529,6 @@ where { /// Given the metadata, extract out the value at a particular index (if any). pub(super) fn get<'a, 'tcx, M: Metadata<'a, 'tcx>>(&self, metadata: M, i: I) -> T::Value<'tcx> { - trace!("LazyTable::lookup: index={:?} len={:?}", i, self.len); - // Access past the end of the table returns a Default if i.index() >= self.len { return Default::default(); diff --git a/compiler/rustc_mir_transform/src/coverage/counters.rs b/compiler/rustc_mir_transform/src/coverage/counters.rs index cf9f6981c5a..46efdd16ee8 100644 --- a/compiler/rustc_mir_transform/src/coverage/counters.rs +++ b/compiler/rustc_mir_transform/src/coverage/counters.rs @@ -1,3 +1,4 @@ +use std::cmp::Ordering; use std::fmt::{self, Debug}; use rustc_data_structures::captures::Captures; @@ -10,9 +11,12 @@ use tracing::{debug, debug_span, instrument}; use crate::coverage::graph::{BasicCoverageBlock, CoverageGraph, TraverseCoverageGraphWithLoops}; +#[cfg(test)] +mod tests; + /// The coverage counter or counter expression associated with a particular /// BCB node or BCB edge. -#[derive(Clone, Copy, PartialEq, Eq, Hash)] +#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] enum BcbCounter { Counter { id: CounterId }, Expression { id: ExpressionId }, @@ -43,8 +47,9 @@ struct BcbExpression { rhs: BcbCounter, } -#[derive(Debug)] -pub(super) enum CounterIncrementSite { +/// Enum representing either a node or an edge in the coverage graph. +#[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] +pub(super) enum Site { Node { bcb: BasicCoverageBlock }, Edge { from_bcb: BasicCoverageBlock, to_bcb: BasicCoverageBlock }, } @@ -54,16 +59,10 @@ pub(super) enum CounterIncrementSite { pub(super) struct CoverageCounters { /// List of places where a counter-increment statement should be injected /// into MIR, each with its corresponding counter ID. - counter_increment_sites: IndexVec<CounterId, CounterIncrementSite>, + counter_increment_sites: IndexVec<CounterId, Site>, /// Coverage counters/expressions that are associated with individual BCBs. node_counters: IndexVec<BasicCoverageBlock, Option<BcbCounter>>, - /// Coverage counters/expressions that are associated with the control-flow - /// edge between two BCBs. - /// - /// We currently don't iterate over this map, but if we do in the future, - /// switch it back to `FxIndexMap` to avoid query stability hazards. - edge_counters: FxHashMap<(BasicCoverageBlock, BasicCoverageBlock), BcbCounter>, /// Table of expression data, associating each expression ID with its /// corresponding operator (+ or -) and its LHS/RHS operands. @@ -83,93 +82,30 @@ impl CoverageCounters { let mut builder = CountersBuilder::new(graph, bcb_needs_counter); builder.make_bcb_counters(); - builder.counters + builder.into_coverage_counters() } fn with_num_bcbs(num_bcbs: usize) -> Self { Self { counter_increment_sites: IndexVec::new(), node_counters: IndexVec::from_elem_n(None, num_bcbs), - edge_counters: FxHashMap::default(), expressions: IndexVec::new(), expressions_memo: FxHashMap::default(), } } - /// Shared helper used by [`Self::make_phys_node_counter`] and - /// [`Self::make_phys_edge_counter`]. Don't call this directly. - fn make_counter_inner(&mut self, site: CounterIncrementSite) -> BcbCounter { + /// Creates a new physical counter for a BCB node or edge. + fn make_phys_counter(&mut self, site: Site) -> BcbCounter { let id = self.counter_increment_sites.push(site); BcbCounter::Counter { id } } - /// Creates a new physical counter for a BCB node. - fn make_phys_node_counter(&mut self, bcb: BasicCoverageBlock) -> BcbCounter { - self.make_counter_inner(CounterIncrementSite::Node { bcb }) - } - - /// Creates a new physical counter for a BCB edge. - fn make_phys_edge_counter( - &mut self, - from_bcb: BasicCoverageBlock, - to_bcb: BasicCoverageBlock, - ) -> BcbCounter { - self.make_counter_inner(CounterIncrementSite::Edge { from_bcb, to_bcb }) - } - fn make_expression(&mut self, lhs: BcbCounter, op: Op, rhs: BcbCounter) -> BcbCounter { let new_expr = BcbExpression { lhs, op, rhs }; - *self - .expressions_memo - .entry(new_expr) - .or_insert_with(|| Self::make_expression_inner(&mut self.expressions, new_expr)) - } - - /// This is an associated function so that we can call it while borrowing - /// `&mut self.expressions_memo`. - fn make_expression_inner( - expressions: &mut IndexVec<ExpressionId, BcbExpression>, - new_expr: BcbExpression, - ) -> BcbCounter { - // Simplify expressions using basic algebra. - // - // Some of these cases might not actually occur in practice, depending - // on the details of how the instrumentor builds expressions. - let BcbExpression { lhs, op, rhs } = new_expr; - - if let BcbCounter::Expression { id } = lhs { - let lhs_expr = &expressions[id]; - - // Simplify `(a - b) + b` to `a`. - if lhs_expr.op == Op::Subtract && op == Op::Add && lhs_expr.rhs == rhs { - return lhs_expr.lhs; - } - // Simplify `(a + b) - b` to `a`. - if lhs_expr.op == Op::Add && op == Op::Subtract && lhs_expr.rhs == rhs { - return lhs_expr.lhs; - } - // Simplify `(a + b) - a` to `b`. - if lhs_expr.op == Op::Add && op == Op::Subtract && lhs_expr.lhs == rhs { - return lhs_expr.rhs; - } - } - - if let BcbCounter::Expression { id } = rhs { - let rhs_expr = &expressions[id]; - - // Simplify `a + (b - a)` to `b`. - if op == Op::Add && rhs_expr.op == Op::Subtract && lhs == rhs_expr.rhs { - return rhs_expr.lhs; - } - // Simplify `a - (a - b)` to `b`. - if op == Op::Subtract && rhs_expr.op == Op::Subtract && lhs == rhs_expr.lhs { - return rhs_expr.rhs; - } - } - - // Simplification failed, so actually create the new expression. - let id = expressions.push(new_expr); - BcbCounter::Expression { id } + *self.expressions_memo.entry(new_expr).or_insert_with(|| { + let id = self.expressions.push(new_expr); + BcbCounter::Expression { id } + }) } /// Creates a counter that is the sum of the given counters. @@ -182,6 +118,12 @@ impl CoverageCounters { .reduce(|accum, counter| self.make_expression(accum, Op::Add, counter)) } + /// Creates a counter whose value is `lhs - SUM(rhs)`. + fn make_subtracted_sum(&mut self, lhs: BcbCounter, rhs: &[BcbCounter]) -> BcbCounter { + let Some(rhs_sum) = self.make_sum(rhs) else { return lhs }; + self.make_expression(lhs, Op::Subtract, rhs_sum) + } + pub(super) fn num_counters(&self) -> usize { self.counter_increment_sites.len() } @@ -195,20 +137,6 @@ impl CoverageCounters { counter } - fn set_edge_counter( - &mut self, - from_bcb: BasicCoverageBlock, - to_bcb: BasicCoverageBlock, - counter: BcbCounter, - ) -> BcbCounter { - let existing = self.edge_counters.insert((from_bcb, to_bcb), counter); - assert!( - existing.is_none(), - "edge ({from_bcb:?} -> {to_bcb:?}) already has a counter: {existing:?} => {counter:?}" - ); - counter - } - pub(super) fn term_for_bcb(&self, bcb: BasicCoverageBlock) -> Option<CovTerm> { self.node_counters[bcb].map(|counter| counter.as_term()) } @@ -218,8 +146,8 @@ impl CoverageCounters { /// each site's corresponding counter ID. pub(super) fn counter_increment_sites( &self, - ) -> impl Iterator<Item = (CounterId, &CounterIncrementSite)> { - self.counter_increment_sites.iter_enumerated() + ) -> impl Iterator<Item = (CounterId, Site)> + Captures<'_> { + self.counter_increment_sites.iter_enumerated().map(|(id, &site)| (id, site)) } /// Returns an iterator over the subset of BCB nodes that have been associated @@ -254,22 +182,53 @@ impl CoverageCounters { } } +/// Symbolic representation of the coverage counter to be used for a particular +/// node or edge in the coverage graph. The same site counter can be used for +/// multiple sites, if they have been determined to have the same count. +#[derive(Clone, Copy, Debug)] +enum SiteCounter { + /// A physical counter at some node/edge. + Phys { site: Site }, + /// A counter expression for a node that takes the sum of all its in-edge + /// counters. + NodeSumExpr { bcb: BasicCoverageBlock }, + /// A counter expression for an edge that takes the counter of its source + /// node, and subtracts the counters of all its sibling out-edges. + EdgeDiffExpr { from_bcb: BasicCoverageBlock, to_bcb: BasicCoverageBlock }, +} + +/// Yields the graph successors of `from_bcb` that aren't `to_bcb`. This is +/// used when creating a counter expression for [`SiteCounter::EdgeDiffExpr`]. +/// +/// For example, in this diagram the sibling out-edge targets of edge `AC` are +/// the nodes `B` and `D`. +/// +/// ```text +/// A +/// / | \ +/// B C D +/// ``` +fn sibling_out_edge_targets( + graph: &CoverageGraph, + from_bcb: BasicCoverageBlock, + to_bcb: BasicCoverageBlock, +) -> impl Iterator<Item = BasicCoverageBlock> + Captures<'_> { + graph.successors[from_bcb].iter().copied().filter(move |&t| t != to_bcb) +} + /// Helper struct that allows counter creation to inspect the BCB graph, and /// the set of nodes that need counters. struct CountersBuilder<'a> { - counters: CoverageCounters, graph: &'a CoverageGraph, bcb_needs_counter: &'a BitSet<BasicCoverageBlock>, + + site_counters: FxHashMap<Site, SiteCounter>, } impl<'a> CountersBuilder<'a> { fn new(graph: &'a CoverageGraph, bcb_needs_counter: &'a BitSet<BasicCoverageBlock>) -> Self { assert_eq!(graph.num_nodes(), bcb_needs_counter.domain_size()); - Self { - counters: CoverageCounters::with_num_bcbs(graph.num_nodes()), - graph, - bcb_needs_counter, - } + Self { graph, bcb_needs_counter, site_counters: FxHashMap::default() } } fn make_bcb_counters(&mut self) { @@ -302,9 +261,7 @@ impl<'a> CountersBuilder<'a> { fn make_node_counter_and_out_edge_counters(&mut self, from_bcb: BasicCoverageBlock) { // First, ensure that this node has a counter of some kind. // We might also use that counter to compute one of the out-edge counters. - let node_counter = self.get_or_make_node_counter(from_bcb); - - let successors = self.graph.successors[from_bcb].as_slice(); + self.get_or_make_node_counter(from_bcb); // If this node's out-edges won't sum to the node's counter, // then there's no reason to create edge counters here. @@ -315,11 +272,11 @@ impl<'a> CountersBuilder<'a> { // When choosing which out-edge should be given a counter expression, ignore edges that // already have counters, or could use the existing counter of their target node. let out_edge_has_counter = |to_bcb| { - if self.counters.edge_counters.contains_key(&(from_bcb, to_bcb)) { + if self.site_counters.contains_key(&Site::Edge { from_bcb, to_bcb }) { return true; } self.graph.sole_predecessor(to_bcb) == Some(from_bcb) - && self.counters.node_counters[to_bcb].is_some() + && self.site_counters.contains_key(&Site::Node { bcb: to_bcb }) }; // Determine the set of out-edges that could benefit from being given an expression. @@ -332,51 +289,41 @@ impl<'a> CountersBuilder<'a> { // If there are out-edges without counters, choose one to be given an expression // (computed from this node and the other out-edges) instead of a physical counter. - let Some(target_bcb) = self.choose_out_edge_for_expression(from_bcb, &candidate_successors) + let Some(to_bcb) = self.choose_out_edge_for_expression(from_bcb, &candidate_successors) else { return; }; // For each out-edge other than the one that was chosen to get an expression, - // ensure that it has a counter (existing counter/expression or a new counter), - // and accumulate the corresponding counters into a single sum expression. - let other_out_edge_counters = successors - .iter() - .copied() - // Skip the chosen edge, since we'll calculate its count from this sum. - .filter(|&edge_target_bcb| edge_target_bcb != target_bcb) - .map(|to_bcb| self.get_or_make_edge_counter(from_bcb, to_bcb)) - .collect::<Vec<_>>(); - let Some(sum_of_all_other_out_edges) = self.counters.make_sum(&other_out_edge_counters) - else { - return; - }; + // ensure that it has a counter (existing counter/expression or a new counter). + for target in sibling_out_edge_targets(self.graph, from_bcb, to_bcb) { + self.get_or_make_edge_counter(from_bcb, target); + } // Now create an expression for the chosen edge, by taking the counter // for its source node and subtracting the sum of its sibling out-edges. - let expression = - self.counters.make_expression(node_counter, Op::Subtract, sum_of_all_other_out_edges); - - debug!("{target_bcb:?} gets an expression: {expression:?}"); - self.counters.set_edge_counter(from_bcb, target_bcb, expression); + let counter = SiteCounter::EdgeDiffExpr { from_bcb, to_bcb }; + self.site_counters.insert(Site::Edge { from_bcb, to_bcb }, counter); } #[instrument(level = "debug", skip(self))] - fn get_or_make_node_counter(&mut self, bcb: BasicCoverageBlock) -> BcbCounter { + fn get_or_make_node_counter(&mut self, bcb: BasicCoverageBlock) -> SiteCounter { // If the BCB already has a counter, return it. - if let Some(counter) = self.counters.node_counters[bcb] { + if let Some(&counter) = self.site_counters.get(&Site::Node { bcb }) { debug!("{bcb:?} already has a counter: {counter:?}"); return counter; } let counter = self.make_node_counter_inner(bcb); - self.counters.set_node_counter(bcb, counter) + self.site_counters.insert(Site::Node { bcb }, counter); + counter } - fn make_node_counter_inner(&mut self, bcb: BasicCoverageBlock) -> BcbCounter { + fn make_node_counter_inner(&mut self, bcb: BasicCoverageBlock) -> SiteCounter { // If the node's sole in-edge already has a counter, use that. if let Some(sole_pred) = self.graph.sole_predecessor(bcb) - && let Some(&edge_counter) = self.counters.edge_counters.get(&(sole_pred, bcb)) + && let Some(&edge_counter) = + self.site_counters.get(&Site::Edge { from_bcb: sole_pred, to_bcb: bcb }) { return edge_counter; } @@ -390,20 +337,17 @@ impl<'a> CountersBuilder<'a> { // leading to infinite recursion. if predecessors.len() <= 1 || predecessors.contains(&bcb) { debug!(?bcb, ?predecessors, "node has <=1 predecessors or is its own predecessor"); - let counter = self.counters.make_phys_node_counter(bcb); + let counter = SiteCounter::Phys { site: Site::Node { bcb } }; debug!(?bcb, ?counter, "node gets a physical counter"); return counter; } // A BCB with multiple incoming edges can compute its count by ensuring that counters // exist for each of those edges, and then adding them up to get a total count. - let in_edge_counters = predecessors - .iter() - .copied() - .map(|from_bcb| self.get_or_make_edge_counter(from_bcb, bcb)) - .collect::<Vec<_>>(); - let sum_of_in_edges: BcbCounter = - self.counters.make_sum(&in_edge_counters).expect("there must be at least one in-edge"); + for &from_bcb in predecessors { + self.get_or_make_edge_counter(from_bcb, bcb); + } + let sum_of_in_edges = SiteCounter::NodeSumExpr { bcb }; debug!("{bcb:?} gets a new counter (sum of predecessor counters): {sum_of_in_edges:?}"); sum_of_in_edges @@ -414,22 +358,23 @@ impl<'a> CountersBuilder<'a> { &mut self, from_bcb: BasicCoverageBlock, to_bcb: BasicCoverageBlock, - ) -> BcbCounter { + ) -> SiteCounter { // If the edge already has a counter, return it. - if let Some(&counter) = self.counters.edge_counters.get(&(from_bcb, to_bcb)) { + if let Some(&counter) = self.site_counters.get(&Site::Edge { from_bcb, to_bcb }) { debug!("Edge {from_bcb:?}->{to_bcb:?} already has a counter: {counter:?}"); return counter; } let counter = self.make_edge_counter_inner(from_bcb, to_bcb); - self.counters.set_edge_counter(from_bcb, to_bcb, counter) + self.site_counters.insert(Site::Edge { from_bcb, to_bcb }, counter); + counter } fn make_edge_counter_inner( &mut self, from_bcb: BasicCoverageBlock, to_bcb: BasicCoverageBlock, - ) -> BcbCounter { + ) -> SiteCounter { // If the target node has exactly one in-edge (i.e. this one), then just // use the node's counter, since it will have the same value. if let Some(sole_pred) = self.graph.sole_predecessor(to_bcb) { @@ -447,7 +392,7 @@ impl<'a> CountersBuilder<'a> { } // Make a new counter to count this edge. - let counter = self.counters.make_phys_edge_counter(from_bcb, to_bcb); + let counter = SiteCounter::Phys { site: Site::Edge { from_bcb, to_bcb } }; debug!(?from_bcb, ?to_bcb, ?counter, "edge gets a physical counter"); counter } @@ -510,4 +455,145 @@ impl<'a> CountersBuilder<'a> { None } + + fn into_coverage_counters(self) -> CoverageCounters { + Transcriber::new(&self).transcribe_counters() + } +} + +/// Helper struct for converting `CountersBuilder` into a final `CoverageCounters`. +struct Transcriber<'a> { + old: &'a CountersBuilder<'a>, + new: CoverageCounters, + phys_counter_for_site: FxHashMap<Site, BcbCounter>, +} + +impl<'a> Transcriber<'a> { + fn new(old: &'a CountersBuilder<'a>) -> Self { + Self { + old, + new: CoverageCounters::with_num_bcbs(old.graph.num_nodes()), + phys_counter_for_site: FxHashMap::default(), + } + } + + fn transcribe_counters(mut self) -> CoverageCounters { + for bcb in self.old.bcb_needs_counter.iter() { + let site = Site::Node { bcb }; + let site_counter = self.site_counter(site); + + // Resolve the site counter into flat lists of nodes/edges whose + // physical counts contribute to the counter for this node. + // Distinguish between counts that will be added vs subtracted. + let mut pos = vec![]; + let mut neg = vec![]; + self.push_resolved_sites(site_counter, &mut pos, &mut neg); + + // Simplify by cancelling out sites that appear on both sides. + let (mut pos, mut neg) = sort_and_cancel(pos, neg); + + if pos.is_empty() { + // If we somehow end up with no positive terms after cancellation, + // fall back to creating a physical counter. There's no known way + // for this to happen, but it's hard to confidently rule it out. + debug_assert!(false, "{site:?} has no positive counter terms"); + pos = vec![Some(site)]; + neg = vec![]; + } + + let mut new_counters_for_sites = |sites: Vec<Option<Site>>| { + sites + .into_iter() + .filter_map(|id| try { self.ensure_phys_counter(id?) }) + .collect::<Vec<_>>() + }; + let mut pos = new_counters_for_sites(pos); + let mut neg = new_counters_for_sites(neg); + + pos.sort(); + neg.sort(); + + let pos_counter = self.new.make_sum(&pos).expect("`pos` should not be empty"); + let new_counter = self.new.make_subtracted_sum(pos_counter, &neg); + self.new.set_node_counter(bcb, new_counter); + } + + self.new + } + + fn site_counter(&self, site: Site) -> SiteCounter { + self.old.site_counters.get(&site).copied().unwrap_or_else(|| { + // We should have already created all necessary site counters. + // But if we somehow didn't, avoid crashing in release builds, + // and just use an extra physical counter instead. + debug_assert!(false, "{site:?} should have a counter"); + SiteCounter::Phys { site } + }) + } + + fn ensure_phys_counter(&mut self, site: Site) -> BcbCounter { + *self.phys_counter_for_site.entry(site).or_insert_with(|| self.new.make_phys_counter(site)) + } + + /// Resolves the given counter into flat lists of nodes/edges, whose counters + /// will then be added and subtracted to form a counter expression. + fn push_resolved_sites(&self, counter: SiteCounter, pos: &mut Vec<Site>, neg: &mut Vec<Site>) { + match counter { + SiteCounter::Phys { site } => pos.push(site), + SiteCounter::NodeSumExpr { bcb } => { + for &from_bcb in &self.old.graph.predecessors[bcb] { + let edge_counter = self.site_counter(Site::Edge { from_bcb, to_bcb: bcb }); + self.push_resolved_sites(edge_counter, pos, neg); + } + } + SiteCounter::EdgeDiffExpr { from_bcb, to_bcb } => { + // First, add the count for `from_bcb`. + let node_counter = self.site_counter(Site::Node { bcb: from_bcb }); + self.push_resolved_sites(node_counter, pos, neg); + + // Then subtract the counts for the other out-edges. + for target in sibling_out_edge_targets(self.old.graph, from_bcb, to_bcb) { + let edge_counter = self.site_counter(Site::Edge { from_bcb, to_bcb: target }); + // Swap `neg` and `pos` so that the counter is subtracted. + self.push_resolved_sites(edge_counter, neg, pos); + } + } + } + } +} + +/// Given two lists: +/// - Sorts each list. +/// - Converts each list to `Vec<Option<T>>`. +/// - Scans for values that appear in both lists, and cancels them out by +/// replacing matching pairs of values with `None`. +fn sort_and_cancel<T: Ord>(mut pos: Vec<T>, mut neg: Vec<T>) -> (Vec<Option<T>>, Vec<Option<T>>) { + pos.sort(); + neg.sort(); + + // Convert to `Vec<Option<T>>`. If `T` has a niche, this should be zero-cost. + let mut pos = pos.into_iter().map(Some).collect::<Vec<_>>(); + let mut neg = neg.into_iter().map(Some).collect::<Vec<_>>(); + + // Scan through the lists using two cursors. When either cursor reaches the + // end of its list, there can be no more equal pairs, so stop. + let mut p = 0; + let mut n = 0; + while p < pos.len() && n < neg.len() { + // If the values are equal, remove them and advance both cursors. + // Otherwise, advance whichever cursor points to the lesser value. + // (Choosing which cursor to advance relies on both lists being sorted.) + match pos[p].cmp(&neg[n]) { + Ordering::Less => p += 1, + Ordering::Equal => { + pos[p] = None; + neg[n] = None; + p += 1; + n += 1; + } + Ordering::Greater => n += 1, + } + } + + (pos, neg) } diff --git a/compiler/rustc_mir_transform/src/coverage/counters/tests.rs b/compiler/rustc_mir_transform/src/coverage/counters/tests.rs new file mode 100644 index 00000000000..794d4358f82 --- /dev/null +++ b/compiler/rustc_mir_transform/src/coverage/counters/tests.rs @@ -0,0 +1,41 @@ +use std::fmt::Debug; + +use super::sort_and_cancel; + +fn flatten<T>(input: Vec<Option<T>>) -> Vec<T> { + input.into_iter().flatten().collect() +} + +fn sort_and_cancel_and_flatten<T: Clone + Ord>(pos: Vec<T>, neg: Vec<T>) -> (Vec<T>, Vec<T>) { + let (pos_actual, neg_actual) = sort_and_cancel(pos, neg); + (flatten(pos_actual), flatten(neg_actual)) +} + +#[track_caller] +fn check_test_case<T: Clone + Debug + Ord>( + pos: Vec<T>, + neg: Vec<T>, + pos_expected: Vec<T>, + neg_expected: Vec<T>, +) { + eprintln!("pos = {pos:?}; neg = {neg:?}"); + let output = sort_and_cancel_and_flatten(pos, neg); + assert_eq!(output, (pos_expected, neg_expected)); +} + +#[test] +fn cancellation() { + let cases: &[(Vec<u32>, Vec<u32>, Vec<u32>, Vec<u32>)] = &[ + (vec![], vec![], vec![], vec![]), + (vec![4, 2, 1, 5, 3], vec![], vec![1, 2, 3, 4, 5], vec![]), + (vec![5, 5, 5, 5, 5], vec![5], vec![5, 5, 5, 5], vec![]), + (vec![1, 1, 2, 2, 3, 3], vec![1, 2, 3], vec![1, 2, 3], vec![]), + (vec![1, 1, 2, 2, 3, 3], vec![2, 4, 2], vec![1, 1, 3, 3], vec![4]), + ]; + + for (pos, neg, pos_expected, neg_expected) in cases { + check_test_case(pos.to_vec(), neg.to_vec(), pos_expected.to_vec(), neg_expected.to_vec()); + // Same test case, but with its inputs flipped and its outputs flipped. + check_test_case(neg.to_vec(), pos.to_vec(), neg_expected.to_vec(), pos_expected.to_vec()); + } +} diff --git a/compiler/rustc_mir_transform/src/coverage/mod.rs b/compiler/rustc_mir_transform/src/coverage/mod.rs index e8b3d80be02..241c3c79114 100644 --- a/compiler/rustc_mir_transform/src/coverage/mod.rs +++ b/compiler/rustc_mir_transform/src/coverage/mod.rs @@ -25,7 +25,7 @@ use rustc_span::source_map::SourceMap; use rustc_span::{BytePos, Pos, SourceFile, Span}; use tracing::{debug, debug_span, trace}; -use crate::coverage::counters::{CounterIncrementSite, CoverageCounters}; +use crate::coverage::counters::{CoverageCounters, Site}; use crate::coverage::graph::CoverageGraph; use crate::coverage::mappings::ExtractedMappings; @@ -265,13 +265,13 @@ fn inject_coverage_statements<'tcx>( coverage_counters: &CoverageCounters, ) { // Inject counter-increment statements into MIR. - for (id, counter_increment_site) in coverage_counters.counter_increment_sites() { + for (id, site) in coverage_counters.counter_increment_sites() { // Determine the block to inject a counter-increment statement into. // For BCB nodes this is just their first block, but for edges we need // to create a new block between the two BCBs, and inject into that. - let target_bb = match *counter_increment_site { - CounterIncrementSite::Node { bcb } => basic_coverage_blocks[bcb].leader_bb(), - CounterIncrementSite::Edge { from_bcb, to_bcb } => { + let target_bb = match site { + Site::Node { bcb } => basic_coverage_blocks[bcb].leader_bb(), + Site::Edge { from_bcb, to_bcb } => { // Create a new block between the last block of `from_bcb` and // the first block of `to_bcb`. let from_bb = basic_coverage_blocks[from_bcb].last_bb(); diff --git a/compiler/rustc_session/src/options.rs b/compiler/rustc_session/src/options.rs index 9e95e278325..2c0302bbb2b 100644 --- a/compiler/rustc_session/src/options.rs +++ b/compiler/rustc_session/src/options.rs @@ -2038,8 +2038,6 @@ written to standard error output)"), "make the current crate share its generic instantiations"), shell_argfiles: bool = (false, parse_bool, [UNTRACKED], "allow argument files to be specified with POSIX \"shell-style\" argument quoting"), - show_span: Option<String> = (None, parse_opt_string, [TRACKED], - "show spans in the crate root file, for compiler debugging (expr|pat|ty)"), simulate_remapped_rust_src_base: Option<PathBuf> = (None, parse_opt_pathbuf, [TRACKED], "simulate the effect of remap-debuginfo = true at bootstrapping by remapping path \ to rust's source base directory. only meant for testing purposes"), |