From fb65ca14b21cb3210251915864007d7c0d64ae21 Mon Sep 17 00:00:00 2001 From: Giacomo Stevanato Date: Sun, 24 Mar 2024 11:20:10 +0100 Subject: [PATCH 01/10] Clarify transmute example --- library/core/src/intrinsics.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/core/src/intrinsics.rs b/library/core/src/intrinsics.rs index dec31548fc8c3..bb1debfc77dec 100644 --- a/library/core/src/intrinsics.rs +++ b/library/core/src/intrinsics.rs @@ -1356,7 +1356,7 @@ extern "rust-intrinsic" { /// let v_clone = v_orig.clone(); /// /// // This is the suggested, safe way. - /// // It does copy the entire vector, though, into a new array. + /// // It may copy the entire vector into a new one though, but also may not. /// let v_collected = v_clone.into_iter() /// .map(Some) /// .collect::>>(); From b5ee20f714456c027b25539f8b145152b3f0e2b1 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Sun, 24 Mar 2024 22:00:08 +1100 Subject: [PATCH 02/10] Clean up unnecessary headers/flags in coverage mir-opt tests These headers and flags were historically needed, but are now unnecessary due to various changes in how coverage information is stored in MIR. --- ...ument_coverage.bar.InstrumentCoverage.diff | 2 +- ...ment_coverage.main.InstrumentCoverage.diff | 10 +++---- tests/mir-opt/instrument_coverage.rs | 30 +++++++------------ ...rage_cleanup.main.CleanupPostBorrowck.diff | 12 ++++---- ...erage_cleanup.main.InstrumentCoverage.diff | 12 ++++---- tests/mir-opt/instrument_coverage_cleanup.rs | 1 - 6 files changed, 28 insertions(+), 39 deletions(-) diff --git a/tests/mir-opt/instrument_coverage.bar.InstrumentCoverage.diff b/tests/mir-opt/instrument_coverage.bar.InstrumentCoverage.diff index dec99ff4601a0..01b01ea5c1756 100644 --- a/tests/mir-opt/instrument_coverage.bar.InstrumentCoverage.diff +++ b/tests/mir-opt/instrument_coverage.bar.InstrumentCoverage.diff @@ -4,7 +4,7 @@ fn bar() -> bool { let mut _0: bool; -+ coverage Code(Counter(0)) => /the/src/instrument_coverage.rs:21:1 - 23:2; ++ coverage Code(Counter(0)) => $DIR/instrument_coverage.rs:19:1 - 21:2; + bb0: { + Coverage::CounterIncrement(0); diff --git a/tests/mir-opt/instrument_coverage.main.InstrumentCoverage.diff b/tests/mir-opt/instrument_coverage.main.InstrumentCoverage.diff index 368088d12961a..3606a9e393231 100644 --- a/tests/mir-opt/instrument_coverage.main.InstrumentCoverage.diff +++ b/tests/mir-opt/instrument_coverage.main.InstrumentCoverage.diff @@ -9,11 +9,11 @@ + coverage ExpressionId(0) => Expression { lhs: Counter(0), op: Add, rhs: Counter(1) }; + coverage ExpressionId(1) => Expression { lhs: Expression(0), op: Subtract, rhs: Counter(1) }; -+ coverage Code(Counter(0)) => /the/src/instrument_coverage.rs:12:1 - 12:11; -+ coverage Code(Expression(0)) => /the/src/instrument_coverage.rs:13:5 - 14:17; -+ coverage Code(Expression(1)) => /the/src/instrument_coverage.rs:15:13 - 15:18; -+ coverage Code(Counter(1)) => /the/src/instrument_coverage.rs:16:10 - 16:11; -+ coverage Code(Expression(1)) => /the/src/instrument_coverage.rs:18:1 - 18:2; ++ coverage Code(Counter(0)) => $DIR/instrument_coverage.rs:10:1 - 10:11; ++ coverage Code(Expression(0)) => $DIR/instrument_coverage.rs:11:5 - 12:17; ++ coverage Code(Expression(1)) => $DIR/instrument_coverage.rs:13:13 - 13:18; ++ coverage Code(Counter(1)) => $DIR/instrument_coverage.rs:14:10 - 14:11; ++ coverage Code(Expression(1)) => $DIR/instrument_coverage.rs:16:1 - 16:2; + bb0: { + Coverage::CounterIncrement(0); diff --git a/tests/mir-opt/instrument_coverage.rs b/tests/mir-opt/instrument_coverage.rs index 010f7bf4d80b2..ae63990f253df 100644 --- a/tests/mir-opt/instrument_coverage.rs +++ b/tests/mir-opt/instrument_coverage.rs @@ -1,11 +1,9 @@ -// skip-filecheck -// Test that `-C instrument-coverage` injects Coverage statements. The Coverage Counter statements -// are later converted into LLVM instrprof.increment intrinsics, during codegen. +// Test that `-C instrument-coverage` injects Coverage statements. +// The Coverage::CounterIncrement statements are later converted into LLVM +// instrprof.increment intrinsics, during codegen. //@ unit-test: InstrumentCoverage -//@ needs-profiler-support -//@ ignore-windows -//@ compile-flags: -C instrument-coverage --remap-path-prefix={{src-base}}=/the/src +//@ compile-flags: -Cinstrument-coverage -Zno-profiler-runtime // EMIT_MIR instrument_coverage.main.InstrumentCoverage.diff // EMIT_MIR instrument_coverage.bar.InstrumentCoverage.diff @@ -22,17 +20,9 @@ fn bar() -> bool { true } -// Note that the MIR with injected coverage intrinsics includes references to source locations, -// including the source file absolute path. Typically, MIR pretty print output with file -// references are safe because the file prefixes are substituted with `$DIR`, but in this case -// the file references are encoded as function arguments, with an `Operand` type representation -// (`Slice` `Allocation` interned byte array) that cannot be normalized by simple substitution. -// -// The first workaround is to use the `SourceMap`-supported `--remap-path-prefix` option; however, -// the implementation of the `--remap-path-prefix` option currently joins the new prefix and the -// remaining source path with an OS-specific path separator (`\` on Windows). This difference still -// shows up in the byte array representation of the path, causing Windows tests to fail to match -// blessed results baselined with a `/` path separator. -// -// Since this `mir-opt` test does not have any significant platform dependencies, other than the -// path separator differences, the final workaround is to disable testing on Windows. +// CHECK: coverage ExpressionId({{[0-9]+}}) => +// CHECK-DAG: coverage Code(Counter({{[0-9]+}})) => +// CHECK-DAG: coverage Code(Expression({{[0-9]+}})) => +// CHECK: bb0: +// CHECK-DAG: Coverage::ExpressionUsed({{[0-9]+}}) +// CHECK-DAG: Coverage::CounterIncrement({{[0-9]+}}) diff --git a/tests/mir-opt/instrument_coverage_cleanup.main.CleanupPostBorrowck.diff b/tests/mir-opt/instrument_coverage_cleanup.main.CleanupPostBorrowck.diff index ff65ca7703928..34d011540b9c1 100644 --- a/tests/mir-opt/instrument_coverage_cleanup.main.CleanupPostBorrowck.diff +++ b/tests/mir-opt/instrument_coverage_cleanup.main.CleanupPostBorrowck.diff @@ -5,15 +5,15 @@ let mut _0: (); let mut _1: bool; - coverage branch { true: BlockMarkerId(0), false: BlockMarkerId(1) } => /the/src/instrument_coverage_cleanup.rs:15:8: 15:36 (#0) + coverage branch { true: BlockMarkerId(0), false: BlockMarkerId(1) } => $DIR/instrument_coverage_cleanup.rs:14:8: 14:36 (#0) coverage ExpressionId(0) => Expression { lhs: Counter(0), op: Subtract, rhs: Counter(1) }; coverage ExpressionId(1) => Expression { lhs: Counter(1), op: Add, rhs: Expression(0) }; - coverage Code(Counter(0)) => /the/src/instrument_coverage_cleanup.rs:14:1 - 15:36; - coverage Code(Expression(0)) => /the/src/instrument_coverage_cleanup.rs:15:37 - 15:39; - coverage Code(Counter(1)) => /the/src/instrument_coverage_cleanup.rs:15:39 - 15:40; - coverage Code(Expression(1)) => /the/src/instrument_coverage_cleanup.rs:16:1 - 16:2; - coverage Branch { true_term: Expression(0), false_term: Counter(1) } => /the/src/instrument_coverage_cleanup.rs:15:8 - 15:36; + coverage Code(Counter(0)) => $DIR/instrument_coverage_cleanup.rs:13:1 - 14:36; + coverage Code(Expression(0)) => $DIR/instrument_coverage_cleanup.rs:14:37 - 14:39; + coverage Code(Counter(1)) => $DIR/instrument_coverage_cleanup.rs:14:39 - 14:40; + coverage Code(Expression(1)) => $DIR/instrument_coverage_cleanup.rs:15:1 - 15:2; + coverage Branch { true_term: Expression(0), false_term: Counter(1) } => $DIR/instrument_coverage_cleanup.rs:14:8 - 14:36; bb0: { Coverage::CounterIncrement(0); diff --git a/tests/mir-opt/instrument_coverage_cleanup.main.InstrumentCoverage.diff b/tests/mir-opt/instrument_coverage_cleanup.main.InstrumentCoverage.diff index 8757559149a43..6756d5c1e1b2c 100644 --- a/tests/mir-opt/instrument_coverage_cleanup.main.InstrumentCoverage.diff +++ b/tests/mir-opt/instrument_coverage_cleanup.main.InstrumentCoverage.diff @@ -5,15 +5,15 @@ let mut _0: (); let mut _1: bool; - coverage branch { true: BlockMarkerId(0), false: BlockMarkerId(1) } => /the/src/instrument_coverage_cleanup.rs:15:8: 15:36 (#0) + coverage branch { true: BlockMarkerId(0), false: BlockMarkerId(1) } => $DIR/instrument_coverage_cleanup.rs:14:8: 14:36 (#0) + coverage ExpressionId(0) => Expression { lhs: Counter(0), op: Subtract, rhs: Counter(1) }; + coverage ExpressionId(1) => Expression { lhs: Counter(1), op: Add, rhs: Expression(0) }; -+ coverage Code(Counter(0)) => /the/src/instrument_coverage_cleanup.rs:14:1 - 15:36; -+ coverage Code(Expression(0)) => /the/src/instrument_coverage_cleanup.rs:15:37 - 15:39; -+ coverage Code(Counter(1)) => /the/src/instrument_coverage_cleanup.rs:15:39 - 15:40; -+ coverage Code(Expression(1)) => /the/src/instrument_coverage_cleanup.rs:16:1 - 16:2; -+ coverage Branch { true_term: Expression(0), false_term: Counter(1) } => /the/src/instrument_coverage_cleanup.rs:15:8 - 15:36; ++ coverage Code(Counter(0)) => $DIR/instrument_coverage_cleanup.rs:13:1 - 14:36; ++ coverage Code(Expression(0)) => $DIR/instrument_coverage_cleanup.rs:14:37 - 14:39; ++ coverage Code(Counter(1)) => $DIR/instrument_coverage_cleanup.rs:14:39 - 14:40; ++ coverage Code(Expression(1)) => $DIR/instrument_coverage_cleanup.rs:15:1 - 15:2; ++ coverage Branch { true_term: Expression(0), false_term: Counter(1) } => $DIR/instrument_coverage_cleanup.rs:14:8 - 14:36; + bb0: { + Coverage::CounterIncrement(0); diff --git a/tests/mir-opt/instrument_coverage_cleanup.rs b/tests/mir-opt/instrument_coverage_cleanup.rs index 8a2fd67139bd9..7db76339e551d 100644 --- a/tests/mir-opt/instrument_coverage_cleanup.rs +++ b/tests/mir-opt/instrument_coverage_cleanup.rs @@ -7,7 +7,6 @@ //@ unit-test: InstrumentCoverage //@ compile-flags: -Cinstrument-coverage -Zcoverage-options=branch -Zno-profiler-runtime -//@ compile-flags: --remap-path-prefix={{src-base}}=/the/src // EMIT_MIR instrument_coverage_cleanup.main.InstrumentCoverage.diff // EMIT_MIR instrument_coverage_cleanup.main.CleanupPostBorrowck.diff From ea4518522f65480eb507a255dc215d39231e659f Mon Sep 17 00:00:00 2001 From: Matthew Maurer Date: Mon, 19 Feb 2024 18:39:35 +0000 Subject: [PATCH 03/10] CFI: Handle dyn with no principal In user-facing Rust, `dyn` always has at least one predicate following it. Unfortunately, because we filter out marker traits from receivers at callsites and `dyn Sync` is, for example, legal, this results in us having `dyn` types with no predicates on occasion in our alias set encoding. This patch handles cases where there are no predicates in a `dyn` type which are relevant to its alias set. Fixes #122998 --- .../src/typeid/typeid_itanium_cxx_abi.rs | 27 ++++++++++--------- tests/ui/sanitizer/cfi-drop-no-principal.rs | 21 +++++++++++++++ 2 files changed, 35 insertions(+), 13 deletions(-) create mode 100644 tests/ui/sanitizer/cfi-drop-no-principal.rs diff --git a/compiler/rustc_symbol_mangling/src/typeid/typeid_itanium_cxx_abi.rs b/compiler/rustc_symbol_mangling/src/typeid/typeid_itanium_cxx_abi.rs index 367fec0e8fcb7..eda7d396d7240 100644 --- a/compiler/rustc_symbol_mangling/src/typeid/typeid_itanium_cxx_abi.rs +++ b/compiler/rustc_symbol_mangling/src/typeid/typeid_itanium_cxx_abi.rs @@ -747,9 +747,8 @@ fn transform_predicates<'tcx>( tcx: TyCtxt<'tcx>, predicates: &List>, ) -> &'tcx List> { - let predicates: Vec> = predicates - .iter() - .filter_map(|predicate| match predicate.skip_binder() { + tcx.mk_poly_existential_predicates_from_iter(predicates.iter().filter_map(|predicate| { + match predicate.skip_binder() { ty::ExistentialPredicate::Trait(trait_ref) => { let trait_ref = ty::TraitRef::identity(tcx, trait_ref.def_id); Some(ty::Binder::dummy(ty::ExistentialPredicate::Trait( @@ -758,9 +757,8 @@ fn transform_predicates<'tcx>( } ty::ExistentialPredicate::Projection(..) => None, ty::ExistentialPredicate::AutoTrait(..) => Some(predicate), - }) - .collect(); - tcx.mk_poly_existential_predicates(&predicates) + } + })) } /// Transforms args for being encoded and used in the substitution dictionary. @@ -1171,14 +1169,17 @@ fn strip_receiver_auto<'tcx>( let ty::Dynamic(preds, lifetime, kind) = ty.kind() else { bug!("Tried to strip auto traits from non-dynamic type {ty}"); }; - let filtered_preds = - if preds.principal().is_some() { + let new_rcvr = if preds.principal().is_some() { + let filtered_preds = tcx.mk_poly_existential_predicates_from_iter(preds.into_iter().filter(|pred| { !matches!(pred.skip_binder(), ty::ExistentialPredicate::AutoTrait(..)) - })) - } else { - ty::List::empty() - }; - let new_rcvr = Ty::new_dynamic(tcx, filtered_preds, *lifetime, *kind); + })); + Ty::new_dynamic(tcx, filtered_preds, *lifetime, *kind) + } else { + // If there's no principal type, re-encode it as a unit, since we don't know anything + // about it. This technically discards the knowledge that it was a type that was made + // into a trait object at some point, but that's not a lot. + tcx.types.unit + }; tcx.mk_args_trait(new_rcvr, args.into_iter().skip(1)) } diff --git a/tests/ui/sanitizer/cfi-drop-no-principal.rs b/tests/ui/sanitizer/cfi-drop-no-principal.rs new file mode 100644 index 0000000000000..c1c88c8c71c73 --- /dev/null +++ b/tests/ui/sanitizer/cfi-drop-no-principal.rs @@ -0,0 +1,21 @@ +// Check that dropping a trait object without a principal trait succeeds + +//@ needs-sanitizer-cfi +// FIXME(#122848) Remove only-linux once OSX CFI binaries works +//@ only-linux +//@ compile-flags: --crate-type=bin -Cprefer-dynamic=off -Clto -Zsanitizer=cfi +//@ compile-flags: -C target-feature=-crt-static -C codegen-units=1 -C opt-level=0 +// FIXME(#118761) Should be run-pass once the labels on drop are compatible. +// This test is being landed ahead of that to test that the compiler doesn't ICE while labeling the +// callsite for a drop, but the vtable doesn't have the correct label yet. +//@ build-pass + +struct CustomDrop; + +impl Drop for CustomDrop { + fn drop(&mut self) {} +} + +fn main() { + let _ = Box::new(CustomDrop) as Box; +} From 40f41e7e896e7d68534d05e955c20330b13f6d0b Mon Sep 17 00:00:00 2001 From: Matthew Maurer Date: Sat, 23 Mar 2024 01:19:43 +0000 Subject: [PATCH 04/10] CFI: Support arbitrary receivers Previously, we only rewrote `&self` and `&mut self` receivers. By instantiating the method from the trait definition, we can make this work work with arbitrary legal receivers instead. --- Cargo.lock | 1 + compiler/rustc_symbol_mangling/Cargo.toml | 1 + compiler/rustc_symbol_mangling/src/lib.rs | 1 + .../src/typeid/typeid_itanium_cxx_abi.rs | 109 +++++++++++------- tests/ui/sanitizer/cfi-complex-receiver.rs | 42 +++++++ 5 files changed, 115 insertions(+), 39 deletions(-) create mode 100644 tests/ui/sanitizer/cfi-complex-receiver.rs diff --git a/Cargo.lock b/Cargo.lock index b8fe1ebaf8019..ad48620bd907a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4634,6 +4634,7 @@ dependencies = [ "rustc_session", "rustc_span", "rustc_target", + "rustc_trait_selection", "tracing", "twox-hash", ] diff --git a/compiler/rustc_symbol_mangling/Cargo.toml b/compiler/rustc_symbol_mangling/Cargo.toml index 0ce522c9cabca..1c8f1d0367039 100644 --- a/compiler/rustc_symbol_mangling/Cargo.toml +++ b/compiler/rustc_symbol_mangling/Cargo.toml @@ -15,6 +15,7 @@ rustc_middle = { path = "../rustc_middle" } rustc_session = { path = "../rustc_session" } rustc_span = { path = "../rustc_span" } rustc_target = { path = "../rustc_target" } +rustc_trait_selection = { path = "../rustc_trait_selection" } tracing = "0.1" twox-hash = "1.6.3" # tidy-alphabetical-end diff --git a/compiler/rustc_symbol_mangling/src/lib.rs b/compiler/rustc_symbol_mangling/src/lib.rs index 02bb1fde75c1b..0588af9bda72a 100644 --- a/compiler/rustc_symbol_mangling/src/lib.rs +++ b/compiler/rustc_symbol_mangling/src/lib.rs @@ -90,6 +90,7 @@ #![doc(html_root_url = "https://doc.rust-lang.org/nightly/nightly-rustc/")] #![doc(rust_logo)] #![feature(rustdoc_internals)] +#![feature(let_chains)] #![allow(internal_features)] #[macro_use] diff --git a/compiler/rustc_symbol_mangling/src/typeid/typeid_itanium_cxx_abi.rs b/compiler/rustc_symbol_mangling/src/typeid/typeid_itanium_cxx_abi.rs index 367fec0e8fcb7..9406038232d1f 100644 --- a/compiler/rustc_symbol_mangling/src/typeid/typeid_itanium_cxx_abi.rs +++ b/compiler/rustc_symbol_mangling/src/typeid/typeid_itanium_cxx_abi.rs @@ -11,6 +11,7 @@ use rustc_data_structures::base_n; use rustc_data_structures::fx::FxHashMap; use rustc_hir as hir; use rustc_middle::ty::layout::IntegerExt; +use rustc_middle::ty::TypeVisitableExt; use rustc_middle::ty::{ self, Const, ExistentialPredicate, FloatTy, FnSig, Instance, IntTy, List, Region, RegionKind, TermKind, Ty, TyCtxt, UintTy, @@ -21,7 +22,9 @@ use rustc_span::sym; use rustc_target::abi::call::{Conv, FnAbi, PassMode}; use rustc_target::abi::Integer; use rustc_target::spec::abi::Abi; +use rustc_trait_selection::traits; use std::fmt::Write as _; +use std::iter; use crate::typeid::TypeIdOptions; @@ -1115,51 +1118,46 @@ pub fn typeid_for_instance<'tcx>( instance.args = strip_receiver_auto(tcx, instance.args) } + if let Some(impl_id) = tcx.impl_of_method(instance.def_id()) + && let Some(trait_ref) = tcx.impl_trait_ref(impl_id) + { + let impl_method = tcx.associated_item(instance.def_id()); + let method_id = impl_method + .trait_item_def_id + .expect("Part of a trait implementation, but not linked to the def_id?"); + let trait_method = tcx.associated_item(method_id); + if traits::is_vtable_safe_method(tcx, trait_ref.skip_binder().def_id, trait_method) { + // Trait methods will have a Self polymorphic parameter, where the concreteized + // implementatation will not. We need to walk back to the more general trait method + let trait_ref = tcx.instantiate_and_normalize_erasing_regions( + instance.args, + ty::ParamEnv::reveal_all(), + trait_ref, + ); + let invoke_ty = trait_object_ty(tcx, ty::Binder::dummy(trait_ref)); + + // At the call site, any call to this concrete function through a vtable will be + // `Virtual(method_id, idx)` with appropriate arguments for the method. Since we have the + // original method id, and we've recovered the trait arguments, we can make the callee + // instance we're computing the alias set for match the caller instance. + // + // Right now, our code ignores the vtable index everywhere, so we use 0 as a placeholder. + // If we ever *do* start encoding the vtable index, we will need to generate an alias set + // based on which vtables we are putting this method into, as there will be more than one + // index value when supertraits are involved. + instance.def = ty::InstanceDef::Virtual(method_id, 0); + let abstract_trait_args = + tcx.mk_args_trait(invoke_ty, trait_ref.args.into_iter().skip(1)); + instance.args = instance.args.rebase_onto(tcx, impl_id, abstract_trait_args); + } + } + let fn_abi = tcx .fn_abi_of_instance(tcx.param_env(instance.def_id()).and((instance, ty::List::empty()))) .unwrap_or_else(|instance| { bug!("typeid_for_instance: couldn't get fn_abi of instance {:?}", instance) }); - // If this instance is a method and self is a reference, get the impl it belongs to - let impl_def_id = tcx.impl_of_method(instance.def_id()); - if impl_def_id.is_some() && !fn_abi.args.is_empty() && fn_abi.args[0].layout.ty.is_ref() { - // If this impl is not an inherent impl, get the trait it implements - if let Some(trait_ref) = tcx.impl_trait_ref(impl_def_id.unwrap()) { - // Transform the concrete self into a reference to a trait object - let existential_predicate = trait_ref.map_bound(|trait_ref| { - ty::ExistentialPredicate::Trait(ty::ExistentialTraitRef::erase_self_ty( - tcx, trait_ref, - )) - }); - let existential_predicates = tcx.mk_poly_existential_predicates(&[ty::Binder::dummy( - existential_predicate.skip_binder(), - )]); - // Is the concrete self mutable? - let self_ty = if fn_abi.args[0].layout.ty.is_mutable_ptr() { - Ty::new_mut_ref( - tcx, - tcx.lifetimes.re_erased, - Ty::new_dynamic(tcx, existential_predicates, tcx.lifetimes.re_erased, ty::Dyn), - ) - } else { - Ty::new_imm_ref( - tcx, - tcx.lifetimes.re_erased, - Ty::new_dynamic(tcx, existential_predicates, tcx.lifetimes.re_erased, ty::Dyn), - ) - }; - - // Replace the concrete self in an fn_abi clone by the reference to a trait object - let mut fn_abi = fn_abi.clone(); - // HACK(rcvalle): It is okay to not replace or update the entire ArgAbi here because the - // other fields are never used. - fn_abi.args[0].layout.ty = self_ty; - - return typeid_for_fnabi(tcx, &fn_abi, options); - } - } - typeid_for_fnabi(tcx, fn_abi, options) } @@ -1182,3 +1180,36 @@ fn strip_receiver_auto<'tcx>( let new_rcvr = Ty::new_dynamic(tcx, filtered_preds, *lifetime, *kind); tcx.mk_args_trait(new_rcvr, args.into_iter().skip(1)) } + +fn trait_object_ty<'tcx>(tcx: TyCtxt<'tcx>, poly_trait_ref: ty::PolyTraitRef<'tcx>) -> Ty<'tcx> { + assert!(!poly_trait_ref.has_non_region_param()); + let principal_pred = poly_trait_ref.map_bound(|trait_ref| { + ty::ExistentialPredicate::Trait(ty::ExistentialTraitRef::erase_self_ty(tcx, trait_ref)) + }); + let mut assoc_preds: Vec<_> = traits::supertraits(tcx, poly_trait_ref) + .flat_map(|super_poly_trait_ref| { + tcx.associated_items(super_poly_trait_ref.def_id()) + .in_definition_order() + .filter(|item| item.kind == ty::AssocKind::Type) + .map(move |assoc_ty| { + super_poly_trait_ref.map_bound(|super_trait_ref| { + let alias_ty = ty::AliasTy::new(tcx, assoc_ty.def_id, super_trait_ref.args); + let resolved = tcx.normalize_erasing_regions( + ty::ParamEnv::reveal_all(), + alias_ty.to_ty(tcx), + ); + ty::ExistentialPredicate::Projection(ty::ExistentialProjection { + def_id: assoc_ty.def_id, + args: ty::ExistentialTraitRef::erase_self_ty(tcx, super_trait_ref).args, + term: resolved.into(), + }) + }) + }) + }) + .collect(); + assoc_preds.sort_by(|a, b| a.skip_binder().stable_cmp(tcx, &b.skip_binder())); + let preds = tcx.mk_poly_existential_predicates_from_iter( + iter::once(principal_pred).chain(assoc_preds.into_iter()), + ); + Ty::new_dynamic(tcx, preds, tcx.lifetimes.re_erased, ty::Dyn) +} diff --git a/tests/ui/sanitizer/cfi-complex-receiver.rs b/tests/ui/sanitizer/cfi-complex-receiver.rs new file mode 100644 index 0000000000000..c3e59258db207 --- /dev/null +++ b/tests/ui/sanitizer/cfi-complex-receiver.rs @@ -0,0 +1,42 @@ +// Check that more complex receivers work: +// * Arc as for custom receivers +// * &dyn Bar for type constraints + +//@ needs-sanitizer-cfi +// FIXME(#122848) Remove only-linux once OSX CFI binaries work +//@ only-linux +//@ compile-flags: --crate-type=bin -Cprefer-dynamic=off -Clto -Zsanitizer=cfi +//@ compile-flags: -C target-feature=-crt-static -C codegen-units=1 -C opt-level=0 +//@ run-pass + +use std::sync::Arc; + +trait Foo { + fn foo(self: Arc); +} + +struct FooImpl; + +impl Foo for FooImpl { + fn foo(self: Arc) {} +} + +trait Bar { + type T; + fn bar(&self) -> Self::T; +} + +struct BarImpl; + +impl Bar for BarImpl { + type T = i32; + fn bar(&self) -> Self::T { 7 } +} + +fn main() { + let foo: Arc = Arc::new(FooImpl); + foo.foo(); + + let bar: &dyn Bar = &BarImpl; + assert_eq!(bar.bar(), 7); +} From 50b49aec3639ad59bf6afbfba25bcfcc24d7cf1c Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Mon, 25 Mar 2024 10:33:54 +1100 Subject: [PATCH 05/10] Temporarily remove nnethercote from the review rotation. I will be on vacation for the next three weeks. I will re-add myself when I return. --- triagebot.toml | 4 ---- 1 file changed, 4 deletions(-) diff --git a/triagebot.toml b/triagebot.toml index 2bcc77ae43323..a42bafe148c5f 100644 --- a/triagebot.toml +++ b/triagebot.toml @@ -775,7 +775,6 @@ compiler-team = [ compiler-team-contributors = [ "@TaKO8Ki", "@Nadrieril", - "@nnethercote", "@fmease", "@fee1-dead", ] @@ -831,17 +830,14 @@ parser = [ "@compiler-errors", "@davidtwco", "@estebank", - "@nnethercote", "@petrochenkov", "@spastorino", ] lexer = [ - "@nnethercote", "@petrochenkov", "@estebank", ] arena = [ - "@nnethercote", "@spastorino", ] mir = [ From 42066b029f3009518e2ba8c7bd20f5781b180b8f Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 13 Mar 2024 15:34:05 +1100 Subject: [PATCH 06/10] Inline and remove `Parser::parse_expr_tuple_field_access_float`. It has a single call site, and afterwards all the calls to `parse_expr_tuple_field_access` are in a single method, which is nice. --- compiler/rustc_parse/src/parser/expr.rs | 74 +++++++++++++------------ 1 file changed, 38 insertions(+), 36 deletions(-) diff --git a/compiler/rustc_parse/src/parser/expr.rs b/compiler/rustc_parse/src/parser/expr.rs index fe17eba0d7b8b..cf7771f87a8c9 100644 --- a/compiler/rustc_parse/src/parser/expr.rs +++ b/compiler/rustc_parse/src/parser/expr.rs @@ -1005,7 +1005,44 @@ impl<'a> Parser<'a> { Ok(self.parse_expr_tuple_field_access(lo, base, symbol, suffix, None)) } token::Literal(token::Lit { kind: token::Float, symbol, suffix }) => { - Ok(self.parse_expr_tuple_field_access_float(lo, base, symbol, suffix)) + Ok(match self.break_up_float(symbol, self.token.span) { + // 1e2 + DestructuredFloat::Single(sym, _sp) => { + self.parse_expr_tuple_field_access(lo, base, sym, suffix, None) + } + // 1. + DestructuredFloat::TrailingDot(sym, ident_span, dot_span) => { + assert!(suffix.is_none()); + self.token = Token::new(token::Ident(sym, IdentIsRaw::No), ident_span); + let next_token = (Token::new(token::Dot, dot_span), self.token_spacing); + self.parse_expr_tuple_field_access(lo, base, sym, None, Some(next_token)) + } + // 1.2 | 1.2e3 + DestructuredFloat::MiddleDot( + symbol1, + ident1_span, + dot_span, + symbol2, + ident2_span, + ) => { + self.token = Token::new(token::Ident(symbol1, IdentIsRaw::No), ident1_span); + // This needs to be `Spacing::Alone` to prevent regressions. + // See issue #76399 and PR #76285 for more details + let next_token1 = (Token::new(token::Dot, dot_span), Spacing::Alone); + let base1 = self.parse_expr_tuple_field_access( + lo, + base, + symbol1, + None, + Some(next_token1), + ); + let next_token2 = + Token::new(token::Ident(symbol2, IdentIsRaw::No), ident2_span); + self.bump_with((next_token2, self.token_spacing)); // `.` + self.parse_expr_tuple_field_access(lo, base1, symbol2, suffix, None) + } + DestructuredFloat::Error => base, + }) } _ => { self.error_unexpected_after_dot(); @@ -1119,41 +1156,6 @@ impl<'a> Parser<'a> { } } - fn parse_expr_tuple_field_access_float( - &mut self, - lo: Span, - base: P, - float: Symbol, - suffix: Option, - ) -> P { - match self.break_up_float(float, self.token.span) { - // 1e2 - DestructuredFloat::Single(sym, _sp) => { - self.parse_expr_tuple_field_access(lo, base, sym, suffix, None) - } - // 1. - DestructuredFloat::TrailingDot(sym, ident_span, dot_span) => { - assert!(suffix.is_none()); - self.token = Token::new(token::Ident(sym, IdentIsRaw::No), ident_span); - let next_token = (Token::new(token::Dot, dot_span), self.token_spacing); - self.parse_expr_tuple_field_access(lo, base, sym, None, Some(next_token)) - } - // 1.2 | 1.2e3 - DestructuredFloat::MiddleDot(symbol1, ident1_span, dot_span, symbol2, ident2_span) => { - self.token = Token::new(token::Ident(symbol1, IdentIsRaw::No), ident1_span); - // This needs to be `Spacing::Alone` to prevent regressions. - // See issue #76399 and PR #76285 for more details - let next_token1 = (Token::new(token::Dot, dot_span), Spacing::Alone); - let base1 = - self.parse_expr_tuple_field_access(lo, base, symbol1, None, Some(next_token1)); - let next_token2 = Token::new(token::Ident(symbol2, IdentIsRaw::No), ident2_span); - self.bump_with((next_token2, self.token_spacing)); // `.` - self.parse_expr_tuple_field_access(lo, base1, symbol2, suffix, None) - } - DestructuredFloat::Error => base, - } - } - /// Parse the field access used in offset_of, matched by `$(e:expr)+`. /// Currently returns a list of idents. However, it should be possible in /// future to also do array indices, which might be arbitrary expressions. From 90eeb3d6817303469059041ef7c8c57f3508f476 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 13 Mar 2024 15:45:27 +1100 Subject: [PATCH 07/10] Remove `next_token` handling from `parse_expr_tuple_field_access`. It's clearer at the call site. --- compiler/rustc_parse/src/parser/expr.rs | 28 +++++++++---------------- 1 file changed, 10 insertions(+), 18 deletions(-) diff --git a/compiler/rustc_parse/src/parser/expr.rs b/compiler/rustc_parse/src/parser/expr.rs index cf7771f87a8c9..50589ad3f1260 100644 --- a/compiler/rustc_parse/src/parser/expr.rs +++ b/compiler/rustc_parse/src/parser/expr.rs @@ -1002,20 +1002,22 @@ impl<'a> Parser<'a> { match self.token.uninterpolate().kind { token::Ident(..) => self.parse_dot_suffix(base, lo), token::Literal(token::Lit { kind: token::Integer, symbol, suffix }) => { - Ok(self.parse_expr_tuple_field_access(lo, base, symbol, suffix, None)) + self.bump(); + Ok(self.parse_expr_tuple_field_access(lo, base, symbol, suffix)) } token::Literal(token::Lit { kind: token::Float, symbol, suffix }) => { Ok(match self.break_up_float(symbol, self.token.span) { // 1e2 DestructuredFloat::Single(sym, _sp) => { - self.parse_expr_tuple_field_access(lo, base, sym, suffix, None) + self.bump(); + self.parse_expr_tuple_field_access(lo, base, sym, suffix) } // 1. DestructuredFloat::TrailingDot(sym, ident_span, dot_span) => { assert!(suffix.is_none()); self.token = Token::new(token::Ident(sym, IdentIsRaw::No), ident_span); - let next_token = (Token::new(token::Dot, dot_span), self.token_spacing); - self.parse_expr_tuple_field_access(lo, base, sym, None, Some(next_token)) + self.bump_with((Token::new(token::Dot, dot_span), self.token_spacing)); + self.parse_expr_tuple_field_access(lo, base, sym, None) } // 1.2 | 1.2e3 DestructuredFloat::MiddleDot( @@ -1028,18 +1030,13 @@ impl<'a> Parser<'a> { self.token = Token::new(token::Ident(symbol1, IdentIsRaw::No), ident1_span); // This needs to be `Spacing::Alone` to prevent regressions. // See issue #76399 and PR #76285 for more details - let next_token1 = (Token::new(token::Dot, dot_span), Spacing::Alone); - let base1 = self.parse_expr_tuple_field_access( - lo, - base, - symbol1, - None, - Some(next_token1), - ); + self.bump_with((Token::new(token::Dot, dot_span), Spacing::Alone)); + let base1 = self.parse_expr_tuple_field_access(lo, base, symbol1, None); let next_token2 = Token::new(token::Ident(symbol2, IdentIsRaw::No), ident2_span); self.bump_with((next_token2, self.token_spacing)); // `.` - self.parse_expr_tuple_field_access(lo, base1, symbol2, suffix, None) + self.bump(); + self.parse_expr_tuple_field_access(lo, base1, symbol2, suffix) } DestructuredFloat::Error => base, }) @@ -1263,12 +1260,7 @@ impl<'a> Parser<'a> { base: P, field: Symbol, suffix: Option, - next_token: Option<(Token, Spacing)>, ) -> P { - match next_token { - Some(next_token) => self.bump_with(next_token), - None => self.bump(), - } let span = self.prev_token.span; let field = ExprKind::Field(base, Ident::new(field, span)); if let Some(suffix) = suffix { From 9c091160dc57aa5224a35e6755c357f7089ff2e5 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 13 Mar 2024 15:59:41 +1100 Subject: [PATCH 08/10] Change `parse_expr_tuple_field_access`. Pass in the span for the field rather than using `prev_token`. Also rename it `mk_expr_tuple_field_access`, because it doesn't do any actual parsing, it just creates an expression with what it's given. Not much of a clarity win by itself, but unlocks additional subsequent simplifications. --- compiler/rustc_parse/src/parser/expr.rs | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/compiler/rustc_parse/src/parser/expr.rs b/compiler/rustc_parse/src/parser/expr.rs index 50589ad3f1260..f96e44eb0f761 100644 --- a/compiler/rustc_parse/src/parser/expr.rs +++ b/compiler/rustc_parse/src/parser/expr.rs @@ -1002,22 +1002,25 @@ impl<'a> Parser<'a> { match self.token.uninterpolate().kind { token::Ident(..) => self.parse_dot_suffix(base, lo), token::Literal(token::Lit { kind: token::Integer, symbol, suffix }) => { + let ident_span = self.token.span; self.bump(); - Ok(self.parse_expr_tuple_field_access(lo, base, symbol, suffix)) + Ok(self.mk_expr_tuple_field_access(lo, ident_span, base, symbol, suffix)) } token::Literal(token::Lit { kind: token::Float, symbol, suffix }) => { Ok(match self.break_up_float(symbol, self.token.span) { // 1e2 DestructuredFloat::Single(sym, _sp) => { + let ident_span = self.token.span; self.bump(); - self.parse_expr_tuple_field_access(lo, base, sym, suffix) + self.mk_expr_tuple_field_access(lo, ident_span, base, sym, suffix) } // 1. DestructuredFloat::TrailingDot(sym, ident_span, dot_span) => { assert!(suffix.is_none()); self.token = Token::new(token::Ident(sym, IdentIsRaw::No), ident_span); + let ident_span = self.token.span; self.bump_with((Token::new(token::Dot, dot_span), self.token_spacing)); - self.parse_expr_tuple_field_access(lo, base, sym, None) + self.mk_expr_tuple_field_access(lo, ident_span, base, sym, None) } // 1.2 | 1.2e3 DestructuredFloat::MiddleDot( @@ -1030,13 +1033,16 @@ impl<'a> Parser<'a> { self.token = Token::new(token::Ident(symbol1, IdentIsRaw::No), ident1_span); // This needs to be `Spacing::Alone` to prevent regressions. // See issue #76399 and PR #76285 for more details + let ident_span = self.token.span; self.bump_with((Token::new(token::Dot, dot_span), Spacing::Alone)); - let base1 = self.parse_expr_tuple_field_access(lo, base, symbol1, None); + let base1 = + self.mk_expr_tuple_field_access(lo, ident_span, base, symbol1, None); let next_token2 = Token::new(token::Ident(symbol2, IdentIsRaw::No), ident2_span); self.bump_with((next_token2, self.token_spacing)); // `.` + let ident_span = self.token.span; self.bump(); - self.parse_expr_tuple_field_access(lo, base1, symbol2, suffix) + self.mk_expr_tuple_field_access(lo, ident_span, base1, symbol2, suffix) } DestructuredFloat::Error => base, }) @@ -1254,19 +1260,18 @@ impl<'a> Parser<'a> { Ok(fields.into_iter().collect()) } - fn parse_expr_tuple_field_access( + fn mk_expr_tuple_field_access( &mut self, lo: Span, + ident_span: Span, base: P, field: Symbol, suffix: Option, ) -> P { - let span = self.prev_token.span; - let field = ExprKind::Field(base, Ident::new(field, span)); if let Some(suffix) = suffix { - self.expect_no_tuple_index_suffix(span, suffix); + self.expect_no_tuple_index_suffix(ident_span, suffix); } - self.mk_expr(lo.to(span), field) + self.mk_expr(lo.to(ident_span), ExprKind::Field(base, Ident::new(field, ident_span))) } /// Parse a function call expression, `expr(...)`. From dce0f7f5c2a62a5aabb1d02351473d6f2b125541 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 21 Mar 2024 16:14:56 +1100 Subject: [PATCH 09/10] Clarify `parse_dot_suffix_expr`. For the `MiddleDot` case, current behaviour: - For a case like `1.2`, `sym1` is `1` and `sym2` is `2`, and `self.token` holds `1.2`. - It creates a new ident token from `sym1` that it puts into `self.token`. - Then it does `bump_with` with a new dot token, which moves the `sym1` token into `prev_token`. - Then it does `bump_with` with a new ident token from `sym2`, which moves the `dot` token into `prev_token` and discards the `sym1` token. - Then it does `bump`, which puts whatever is next into `self.token`, moves the `sym2` token into `prev_token`, and discards the `dot` token altogether. New behaviour: - Skips creating and inserting the `sym1` and dot tokens, because they are unnecessary. - This also demonstrates that the comment about `Spacing::Alone` is wrong -- that value is never used. That comment was added in #77250, and AFAICT it has always been incorrect. The commit also expands comments. I found this code hard to read previously, the examples in comments make it easier. --- compiler/rustc_parse/src/parser/expr.rs | 35 ++++++++++++++----------- 1 file changed, 19 insertions(+), 16 deletions(-) diff --git a/compiler/rustc_parse/src/parser/expr.rs b/compiler/rustc_parse/src/parser/expr.rs index f96e44eb0f761..7e317c3df1465 100644 --- a/compiler/rustc_parse/src/parser/expr.rs +++ b/compiler/rustc_parse/src/parser/expr.rs @@ -16,7 +16,6 @@ use core::mem; use core::ops::ControlFlow; use rustc_ast::ptr::P; use rustc_ast::token::{self, Delimiter, Token, TokenKind}; -use rustc_ast::tokenstream::Spacing; use rustc_ast::util::case::Case; use rustc_ast::util::classify; use rustc_ast::util::parser::{prec_let_scrutinee_needs_par, AssocOp, Fixity}; @@ -999,6 +998,8 @@ impl<'a> Parser<'a> { } pub fn parse_dot_suffix_expr(&mut self, lo: Span, base: P) -> PResult<'a, P> { + // At this point we've consumed something like `expr.` and `self.token` holds the token + // after the dot. match self.token.uninterpolate().kind { token::Ident(..) => self.parse_dot_suffix(base, lo), token::Literal(token::Lit { kind: token::Integer, symbol, suffix }) => { @@ -1010,39 +1011,41 @@ impl<'a> Parser<'a> { Ok(match self.break_up_float(symbol, self.token.span) { // 1e2 DestructuredFloat::Single(sym, _sp) => { + // `foo.1e2`: a single complete dot access, fully consumed. We end up with + // the `1e2` token in `self.prev_token` and the following token in + // `self.token`. let ident_span = self.token.span; self.bump(); self.mk_expr_tuple_field_access(lo, ident_span, base, sym, suffix) } // 1. DestructuredFloat::TrailingDot(sym, ident_span, dot_span) => { + // `foo.1.`: a single complete dot access and the start of another. + // We end up with the `sym` (`1`) token in `self.prev_token` and a dot in + // `self.token`. assert!(suffix.is_none()); self.token = Token::new(token::Ident(sym, IdentIsRaw::No), ident_span); - let ident_span = self.token.span; self.bump_with((Token::new(token::Dot, dot_span), self.token_spacing)); self.mk_expr_tuple_field_access(lo, ident_span, base, sym, None) } // 1.2 | 1.2e3 DestructuredFloat::MiddleDot( - symbol1, + sym1, ident1_span, - dot_span, - symbol2, + _dot_span, + sym2, ident2_span, ) => { - self.token = Token::new(token::Ident(symbol1, IdentIsRaw::No), ident1_span); - // This needs to be `Spacing::Alone` to prevent regressions. - // See issue #76399 and PR #76285 for more details - let ident_span = self.token.span; - self.bump_with((Token::new(token::Dot, dot_span), Spacing::Alone)); - let base1 = - self.mk_expr_tuple_field_access(lo, ident_span, base, symbol1, None); + // `foo.1.2` (or `foo.1.2e3`): two complete dot accesses. We end up with + // the `sym2` (`2` or `2e3`) token in `self.prev_token` and the following + // token in `self.token`. let next_token2 = - Token::new(token::Ident(symbol2, IdentIsRaw::No), ident2_span); - self.bump_with((next_token2, self.token_spacing)); // `.` - let ident_span = self.token.span; + Token::new(token::Ident(sym2, IdentIsRaw::No), ident2_span); + self.bump_with((next_token2, self.token_spacing)); self.bump(); - self.mk_expr_tuple_field_access(lo, ident_span, base1, symbol2, suffix) + let base1 = + self.mk_expr_tuple_field_access(lo, ident1_span, base, sym1, None); + self.mk_expr_tuple_field_access(lo, ident2_span, base1, sym2, suffix) } DestructuredFloat::Error => base, }) From 1bbab7148bb0a44b59ce7a98db726b66ea8e7c00 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Sun, 24 Mar 2024 15:11:39 +1100 Subject: [PATCH 10/10] Add more comments to the bootstrap code that handles `tests/coverage` --- src/bootstrap/src/core/build_steps/test.rs | 65 ++++++++++++++++------ 1 file changed, 49 insertions(+), 16 deletions(-) diff --git a/src/bootstrap/src/core/build_steps/test.rs b/src/bootstrap/src/core/build_steps/test.rs index 6d3163b90b10c..f22a9ca604ee2 100644 --- a/src/bootstrap/src/core/build_steps/test.rs +++ b/src/bootstrap/src/core/build_steps/test.rs @@ -1289,9 +1289,9 @@ macro_rules! test_definitions { /// Adapted from [`test_definitions`]. macro_rules! coverage_test_alias { ($name:ident { - alias_and_mode: $alias_and_mode:expr, - default: $default:expr, - only_hosts: $only_hosts:expr $(,)? + alias_and_mode: $alias_and_mode:expr, // &'static str + default: $default:expr, // bool + only_hosts: $only_hosts:expr $(,)? // bool }) => { #[derive(Debug, Clone, PartialEq, Eq, Hash)] pub struct $name { @@ -1309,6 +1309,8 @@ macro_rules! coverage_test_alias { const ONLY_HOSTS: bool = $only_hosts; fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> { + // Register the mode name as a command-line alias. + // This allows `x test coverage-map` and `x test coverage-run`. run.alias($alias_and_mode) } @@ -1319,8 +1321,7 @@ macro_rules! coverage_test_alias { } fn run(self, builder: &Builder<'_>) { - Coverage { compiler: self.compiler, target: self.target } - .run_unified_suite(builder, Self::MODE) + Coverage::run_coverage_tests(builder, self.compiler, self.target, Self::MODE); } } }; @@ -1449,11 +1450,20 @@ host_test!(RunMakeFullDeps { default_test!(Assembly { path: "tests/assembly", mode: "assembly", suite: "assembly" }); -/// Custom test step that is responsible for running the coverage tests -/// in multiple different modes. +/// Coverage tests are a bit more complicated than other test suites, because +/// we want to run the same set of test files in multiple different modes, +/// in a way that's convenient and flexible when invoked manually. +/// +/// This combined step runs the specified tests (or all of `tests/coverage`) +/// in both "coverage-map" and "coverage-run" modes. +/// +/// Used by: +/// - `x test coverage` +/// - `x test tests/coverage` +/// - `x test tests/coverage/trivial.rs` (etc) /// -/// Each individual mode also has its own alias that will run the tests in -/// just that mode. +/// (Each individual mode also has its own step that will run the tests in +/// just that mode.) #[derive(Debug, Clone, PartialEq, Eq, Hash)] pub struct Coverage { pub compiler: Compiler, @@ -1464,24 +1474,41 @@ impl Coverage { const PATH: &'static str = "tests/coverage"; const SUITE: &'static str = "coverage"; - fn run_unified_suite(&self, builder: &Builder<'_>, mode: &'static str) { + /// Runs the coverage test suite (or a user-specified subset) in one mode. + /// + /// This same function is used by the multi-mode step ([`Coverage`]) and by + /// the single-mode steps ([`CoverageMap`] and [`CoverageRun`]), to help + /// ensure that they all behave consistently with each other, regardless of + /// how the coverage tests have been invoked. + fn run_coverage_tests( + builder: &Builder<'_>, + compiler: Compiler, + target: TargetSelection, + mode: &'static str, + ) { + // Like many other test steps, we delegate to a `Compiletest` step to + // actually run the tests. (See `test_definitions!`.) builder.ensure(Compiletest { - compiler: self.compiler, - target: self.target, + compiler, + target, mode, suite: Self::SUITE, path: Self::PATH, compare_mode: None, - }) + }); } } impl Step for Coverage { type Output = (); + // We rely on the individual CoverageMap/CoverageRun steps to run themselves. const DEFAULT: bool = false; + // When manually invoked, try to run as much as possible. + // Compiletest will automatically skip the "coverage-run" tests if necessary. const ONLY_HOSTS: bool = false; fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> { + // Take responsibility for command-line paths within `tests/coverage`. run.suite_path(Self::PATH) } @@ -1492,20 +1519,26 @@ impl Step for Coverage { } fn run(self, builder: &Builder<'_>) { - self.run_unified_suite(builder, CoverageMap::MODE); - self.run_unified_suite(builder, CoverageRun::MODE); + // Run the specified coverage tests (possibly all of them) in both modes. + Self::run_coverage_tests(builder, self.compiler, self.target, CoverageMap::MODE); + Self::run_coverage_tests(builder, self.compiler, self.target, CoverageRun::MODE); } } -// Aliases for running the coverage tests in only one mode. +// Runs `tests/coverage` in "coverage-map" mode only. +// Used by `x test` and `x test coverage-map`. coverage_test_alias!(CoverageMap { alias_and_mode: "coverage-map", default: true, only_hosts: false, }); +// Runs `tests/coverage` in "coverage-run" mode only. +// Used by `x test` and `x test coverage-run`. coverage_test_alias!(CoverageRun { alias_and_mode: "coverage-run", default: true, + // Compiletest knows how to automatically skip these tests when cross-compiling, + // but skipping the whole step here makes it clearer that they haven't run at all. only_hosts: true, });