From 2c31c550206ae0f5d88db5f3d46e0957b669dfc9 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Wed, 25 Dec 2024 01:27:21 +0000 Subject: [PATCH 1/8] Use PostBorrowckAnalysis in check_coroutine_obligations --- .../rustc_hir_analysis/src/check/check.rs | 23 ++++++++++++------- tests/ui/coroutine/issue-52304.rs | 2 ++ 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/compiler/rustc_hir_analysis/src/check/check.rs b/compiler/rustc_hir_analysis/src/check/check.rs index 5548a6a6ef79..8c6059d49a84 100644 --- a/compiler/rustc_hir_analysis/src/check/check.rs +++ b/compiler/rustc_hir_analysis/src/check/check.rs @@ -1845,13 +1845,18 @@ pub(super) fn check_coroutine_obligations( debug!(?typeck_results.coroutine_stalled_predicates); + let mode = if tcx.next_trait_solver_globally() { + TypingMode::post_borrowck_analysis(tcx, def_id) + } else { + TypingMode::analysis_in_body(tcx, def_id) + }; + let infcx = tcx .infer_ctxt() // typeck writeback gives us predicates with their regions erased. // As borrowck already has checked lifetimes, we do not need to do it again. .ignoring_regions() - // FIXME(#132279): This should eventually use the already defined hidden types. - .build(TypingMode::analysis_in_body(tcx, def_id)); + .build(mode); let ocx = ObligationCtxt::new_with_diagnostics(&infcx); for (predicate, cause) in &typeck_results.coroutine_stalled_predicates { @@ -1864,12 +1869,14 @@ pub(super) fn check_coroutine_obligations( return Err(infcx.err_ctxt().report_fulfillment_errors(errors)); } - // Check that any hidden types found when checking these stalled coroutine obligations - // are valid. - for (key, ty) in infcx.take_opaque_types() { - let hidden_type = infcx.resolve_vars_if_possible(ty.hidden_type); - let key = infcx.resolve_vars_if_possible(key); - sanity_check_found_hidden_type(tcx, key, hidden_type)?; + if !tcx.next_trait_solver_globally() { + // Check that any hidden types found when checking these stalled coroutine obligations + // are valid. + for (key, ty) in infcx.take_opaque_types() { + let hidden_type = infcx.resolve_vars_if_possible(ty.hidden_type); + let key = infcx.resolve_vars_if_possible(key); + sanity_check_found_hidden_type(tcx, key, hidden_type)?; + } } Ok(()) diff --git a/tests/ui/coroutine/issue-52304.rs b/tests/ui/coroutine/issue-52304.rs index 552bc0028ee0..77dfe8391956 100644 --- a/tests/ui/coroutine/issue-52304.rs +++ b/tests/ui/coroutine/issue-52304.rs @@ -1,4 +1,6 @@ //@ check-pass +//@ revisions: current next +//@ ignore-compare-mode-next-solver (explicit revisions) #![feature(coroutines, coroutine_trait)] From 592259930b27525789f14f07396f02b254d4dfc9 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Wed, 25 Dec 2024 20:14:43 +0000 Subject: [PATCH 2/8] Report correct SelectionError for ConstArgHasType in new solver fulfill --- .../src/solve/fulfill.rs | 19 ++++++++++++++++++- ...nst-in-impl-fn-return-type.current.stderr} | 4 ++-- .../const-in-impl-fn-return-type.next.stderr | 15 +++++++++++++++ .../const-in-impl-fn-return-type.rs | 5 +++++ 4 files changed, 40 insertions(+), 3 deletions(-) rename tests/ui/typeck/issue-114918/{const-in-impl-fn-return-type.stderr => const-in-impl-fn-return-type.current.stderr} (81%) create mode 100644 tests/ui/typeck/issue-114918/const-in-impl-fn-return-type.next.stderr diff --git a/compiler/rustc_trait_selection/src/solve/fulfill.rs b/compiler/rustc_trait_selection/src/solve/fulfill.rs index 2b2623a050ec..c79a8abca204 100644 --- a/compiler/rustc_trait_selection/src/solve/fulfill.rs +++ b/compiler/rustc_trait_selection/src/solve/fulfill.rs @@ -10,9 +10,9 @@ use rustc_infer::traits::{ self, FromSolverError, MismatchedProjectionTypes, Obligation, ObligationCause, ObligationCauseCode, PredicateObligation, PredicateObligations, SelectionError, TraitEngine, }; -use rustc_middle::bug; use rustc_middle::ty::error::{ExpectedFound, TypeError}; use rustc_middle::ty::{self, TyCtxt}; +use rustc_middle::{bug, span_bug}; use rustc_next_trait_solver::solve::{GenerateProofTree, HasChanged, SolverDelegateEvalExt as _}; use tracing::{instrument, trace}; @@ -258,6 +258,23 @@ fn fulfillment_error_for_no_solution<'tcx>( MismatchedProjectionTypes { err: TypeError::Mismatch }, ) } + ty::PredicateKind::Clause(ty::ClauseKind::ConstArgHasType(ct, expected_ty)) => { + let ct_ty = match ct.kind() { + ty::ConstKind::Unevaluated(uv) => { + infcx.tcx.type_of(uv.def).instantiate(infcx.tcx, uv.args) + } + ty::ConstKind::Param(param_ct) => param_ct.find_ty_from_env(obligation.param_env), + _ => span_bug!( + obligation.cause.span, + "ConstArgHasWrongType failed but we don't know how to compute type" + ), + }; + FulfillmentErrorCode::Select(SelectionError::ConstArgHasWrongType { + ct, + ct_ty, + expected_ty, + }) + } ty::PredicateKind::NormalizesTo(..) => { FulfillmentErrorCode::Project(MismatchedProjectionTypes { err: TypeError::Mismatch }) } diff --git a/tests/ui/typeck/issue-114918/const-in-impl-fn-return-type.stderr b/tests/ui/typeck/issue-114918/const-in-impl-fn-return-type.current.stderr similarity index 81% rename from tests/ui/typeck/issue-114918/const-in-impl-fn-return-type.stderr rename to tests/ui/typeck/issue-114918/const-in-impl-fn-return-type.current.stderr index 8017e5446cc7..1bcc0dbaf672 100644 --- a/tests/ui/typeck/issue-114918/const-in-impl-fn-return-type.stderr +++ b/tests/ui/typeck/issue-114918/const-in-impl-fn-return-type.current.stderr @@ -1,11 +1,11 @@ error[E0308]: mismatched types - --> $DIR/const-in-impl-fn-return-type.rs:15:39 + --> $DIR/const-in-impl-fn-return-type.rs:20:39 | LL | fn func() -> [(); { () }] { | ^^ expected `usize`, found `()` error: the constant `N` is not of type `usize` - --> $DIR/const-in-impl-fn-return-type.rs:7:32 + --> $DIR/const-in-impl-fn-return-type.rs:12:32 | LL | fn func() -> [(); N]; | ^^^^^^^ expected `usize`, found `u32` diff --git a/tests/ui/typeck/issue-114918/const-in-impl-fn-return-type.next.stderr b/tests/ui/typeck/issue-114918/const-in-impl-fn-return-type.next.stderr new file mode 100644 index 000000000000..1bcc0dbaf672 --- /dev/null +++ b/tests/ui/typeck/issue-114918/const-in-impl-fn-return-type.next.stderr @@ -0,0 +1,15 @@ +error[E0308]: mismatched types + --> $DIR/const-in-impl-fn-return-type.rs:20:39 + | +LL | fn func() -> [(); { () }] { + | ^^ expected `usize`, found `()` + +error: the constant `N` is not of type `usize` + --> $DIR/const-in-impl-fn-return-type.rs:12:32 + | +LL | fn func() -> [(); N]; + | ^^^^^^^ expected `usize`, found `u32` + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0308`. diff --git a/tests/ui/typeck/issue-114918/const-in-impl-fn-return-type.rs b/tests/ui/typeck/issue-114918/const-in-impl-fn-return-type.rs index 5eef26887211..6bbac9d45bbe 100644 --- a/tests/ui/typeck/issue-114918/const-in-impl-fn-return-type.rs +++ b/tests/ui/typeck/issue-114918/const-in-impl-fn-return-type.rs @@ -1,4 +1,9 @@ +//@ revisions: current next +//@[next] compile-flags: -Znext-solver +//@ ignore-compare-mode-next-solver (explicit revisions) + // Regression test for #114918 + // Test that a const generic enclosed in a block within the return type // of an impl fn produces a type mismatch error instead of triggering // a const eval cycle From 899d9e67692012b665755824b027adeb014497be Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Tue, 31 Dec 2024 03:08:55 +0000 Subject: [PATCH 3/8] Suppress host effect predicates if underlying trait doesn't hold --- .../traits/fulfillment_errors.rs | 53 +++++++++++++------ .../double-error-for-unimplemented-trait.rs | 22 ++++++++ ...ouble-error-for-unimplemented-trait.stderr | 41 ++++++++++++++ 3 files changed, 99 insertions(+), 17 deletions(-) create mode 100644 tests/ui/traits/const-traits/double-error-for-unimplemented-trait.rs create mode 100644 tests/ui/traits/const-traits/double-error-for-unimplemented-trait.stderr diff --git a/compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs b/compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs index 885b606326c5..163c453bdbe7 100644 --- a/compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs +++ b/compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs @@ -538,23 +538,7 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> { } ty::PredicateKind::Clause(ty::ClauseKind::HostEffect(predicate)) => { - // FIXME(const_trait_impl): We should recompute the predicate with `~const` - // if it's `const`, and if it holds, explain that this bound only - // *conditionally* holds. If that fails, we should also do selection - // to drill this down to an impl or built-in source, so we can - // point at it and explain that while the trait *is* implemented, - // that implementation is not const. - let err_msg = self.get_standard_error_message( - bound_predicate.rebind(ty::TraitPredicate { - trait_ref: predicate.trait_ref, - polarity: ty::PredicatePolarity::Positive, - }), - None, - Some(predicate.constness), - None, - String::new(), - ); - struct_span_code_err!(self.dcx(), span, E0277, "{}", err_msg) + self.report_host_effect_error(bound_predicate.rebind(predicate), obligation.param_env, span) } ty::PredicateKind::Subtype(predicate) => { @@ -763,6 +747,41 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> { applied_do_not_recommend } + fn report_host_effect_error( + &self, + predicate: ty::Binder<'tcx, ty::HostEffectPredicate<'tcx>>, + param_env: ty::ParamEnv<'tcx>, + span: Span, + ) -> Diag<'a> { + // FIXME(const_trait_impl): We should recompute the predicate with `~const` + // if it's `const`, and if it holds, explain that this bound only + // *conditionally* holds. If that fails, we should also do selection + // to drill this down to an impl or built-in source, so we can + // point at it and explain that while the trait *is* implemented, + // that implementation is not const. + let trait_ref = predicate.map_bound(|predicate| ty::TraitPredicate { + trait_ref: predicate.trait_ref, + polarity: ty::PredicatePolarity::Positive, + }); + let err_msg = self.get_standard_error_message( + trait_ref, + None, + Some(predicate.constness()), + None, + String::new(), + ); + let mut diag = struct_span_code_err!(self.dcx(), span, E0277, "{}", err_msg); + if !self.predicate_may_hold(&Obligation::new( + self.tcx, + ObligationCause::dummy(), + param_env, + trait_ref, + )) { + diag.downgrade_to_delayed_bug(); + } + diag + } + fn emit_specialized_closure_kind_error( &self, obligation: &PredicateObligation<'tcx>, diff --git a/tests/ui/traits/const-traits/double-error-for-unimplemented-trait.rs b/tests/ui/traits/const-traits/double-error-for-unimplemented-trait.rs new file mode 100644 index 000000000000..f4b01efe9590 --- /dev/null +++ b/tests/ui/traits/const-traits/double-error-for-unimplemented-trait.rs @@ -0,0 +1,22 @@ +// Make sure we don't issue *two* error messages for the trait predicate *and* host predicate. + +#![feature(const_trait_impl)] + +#[const_trait] +trait Trait { + type Out; +} + +const fn needs_const(_: &T) {} + +const IN_CONST: () = { + needs_const(&()); + //~^ ERROR the trait bound `(): Trait` is not satisfied +}; + +const fn conditionally_const() { + needs_const(&()); + //~^ ERROR the trait bound `(): Trait` is not satisfied +} + +fn main() {} diff --git a/tests/ui/traits/const-traits/double-error-for-unimplemented-trait.stderr b/tests/ui/traits/const-traits/double-error-for-unimplemented-trait.stderr new file mode 100644 index 000000000000..cd68cdaf8a2b --- /dev/null +++ b/tests/ui/traits/const-traits/double-error-for-unimplemented-trait.stderr @@ -0,0 +1,41 @@ +error[E0277]: the trait bound `(): Trait` is not satisfied + --> $DIR/double-error-for-unimplemented-trait.rs:13:15 + | +LL | needs_const(&()); + | ----------- ^^^ the trait `Trait` is not implemented for `()` + | | + | required by a bound introduced by this call + | +help: this trait has no implementations, consider adding one + --> $DIR/double-error-for-unimplemented-trait.rs:6:1 + | +LL | trait Trait { + | ^^^^^^^^^^^ +note: required by a bound in `needs_const` + --> $DIR/double-error-for-unimplemented-trait.rs:10:25 + | +LL | const fn needs_const(_: &T) {} + | ^^^^^^^^^^^^ required by this bound in `needs_const` + +error[E0277]: the trait bound `(): Trait` is not satisfied + --> $DIR/double-error-for-unimplemented-trait.rs:18:15 + | +LL | needs_const(&()); + | ----------- ^^^ the trait `Trait` is not implemented for `()` + | | + | required by a bound introduced by this call + | +help: this trait has no implementations, consider adding one + --> $DIR/double-error-for-unimplemented-trait.rs:6:1 + | +LL | trait Trait { + | ^^^^^^^^^^^ +note: required by a bound in `needs_const` + --> $DIR/double-error-for-unimplemented-trait.rs:10:25 + | +LL | const fn needs_const(_: &T) {} + | ^^^^^^^^^^^^ required by this bound in `needs_const` + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0277`. From 62d6ee3a26a9fbc2f0d99939bffd387452793856 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Tue, 31 Dec 2024 03:09:31 +0000 Subject: [PATCH 4/8] nit: what the heck is `o` --- .../src/error_reporting/traits/fulfillment_errors.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs b/compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs index 163c453bdbe7..92aba2489880 100644 --- a/compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs +++ b/compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs @@ -114,7 +114,7 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> { // // We rely on a few heuristics to identify cases where this root // obligation is more important than the leaf obligation: - let (main_trait_predicate, o) = if let ty::PredicateKind::Clause( + let (main_trait_predicate, main_obligation) = if let ty::PredicateKind::Clause( ty::ClauseKind::Trait(root_pred) ) = root_obligation.predicate.kind().skip_binder() && !leaf_trait_predicate.self_ty().skip_binder().has_escaping_bound_vars() @@ -199,7 +199,7 @@ impl<'a, 'tcx> TypeErrCtxt<'a, 'tcx> { notes, parent_label, append_const_msg, - } = self.on_unimplemented_note(main_trait_predicate, o, &mut long_ty_file); + } = self.on_unimplemented_note(main_trait_predicate, main_obligation, &mut long_ty_file); let have_alt_message = message.is_some() || label.is_some(); let is_try_conversion = self.is_try_conversion(span, main_trait_ref.def_id()); From 85e0d42a776cc0fc655b39cb9e085d36caec071b Mon Sep 17 00:00:00 2001 From: Zalathar Date: Mon, 6 Jan 2025 12:50:10 +1100 Subject: [PATCH 5/8] Don't enable anyhow's `backtrace` feature in opt-dist As of the stabilization of `std::backtrace` in Rust 1.65, this package flag has no effect other than to enable an unused dependency on the `backtrace` crate. --- Cargo.lock | 3 --- src/tools/opt-dist/Cargo.toml | 2 +- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 62541eb9d479..0b38cb92e2b8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -185,9 +185,6 @@ name = "anyhow" version = "1.0.95" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "34ac096ce696dc2fcabef30516bb13c0a68a11d30131d3df6f04711467681b04" -dependencies = [ - "backtrace", -] [[package]] name = "ar_archive_writer" diff --git a/src/tools/opt-dist/Cargo.toml b/src/tools/opt-dist/Cargo.toml index d0413911014f..cea234cc74cb 100644 --- a/src/tools/opt-dist/Cargo.toml +++ b/src/tools/opt-dist/Cargo.toml @@ -7,7 +7,7 @@ edition = "2021" build_helper = { path = "../../build_helper" } env_logger = "0.11" log = "0.4" -anyhow = { version = "1", features = ["backtrace"] } +anyhow = "1" humantime = "2" humansize = "2" sysinfo = { version = "0.31.2", default-features = false, features = ["disk"] } From 591bf634394867c44cfc5cdaf391f948e6c3d11a Mon Sep 17 00:00:00 2001 From: crystalstall Date: Mon, 6 Jan 2025 15:47:49 +0800 Subject: [PATCH 6/8] chore: remove redundant words in comment Signed-off-by: crystalstall --- library/std/src/ffi/os_str.rs | 2 +- library/std/src/pipe.rs | 2 +- library/std/src/sync/poison/mutex.rs | 2 +- .../patches/gcc/8.5.0/0002-hidden-jump-target.patch | 2 +- tests/ui/duplicate/multiple-types-with-same-name-and-derive.rs | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/library/std/src/ffi/os_str.rs b/library/std/src/ffi/os_str.rs index fff140f1564f..7fb57d410431 100644 --- a/library/std/src/ffi/os_str.rs +++ b/library/std/src/ffi/os_str.rs @@ -550,7 +550,7 @@ impl OsString { OsStr::from_inner_mut(self.inner.leak()) } - /// Truncate the the `OsString` to the specified length. + /// Truncate the `OsString` to the specified length. /// /// # Panics /// Panics if `len` does not lie on a valid `OsStr` boundary diff --git a/library/std/src/pipe.rs b/library/std/src/pipe.rs index 06f3fd9fdffe..913c22588a76 100644 --- a/library/std/src/pipe.rs +++ b/library/std/src/pipe.rs @@ -97,7 +97,7 @@ impl PipeReader { /// let mut jobs = vec![]; /// let (reader, mut writer) = std::pipe::pipe()?; /// - /// // Write NUM_SLOT characters the the pipe. + /// // Write NUM_SLOT characters the pipe. /// writer.write_all(&[b'|'; NUM_SLOT as usize])?; /// /// // Spawn several processes that read a character from the pipe, do some work, then diff --git a/library/std/src/sync/poison/mutex.rs b/library/std/src/sync/poison/mutex.rs index e28c2090afed..01ef71a187fe 100644 --- a/library/std/src/sync/poison/mutex.rs +++ b/library/std/src/sync/poison/mutex.rs @@ -534,7 +534,7 @@ impl Mutex { /// # Errors /// /// If another user of this mutex panicked while holding the mutex, then - /// this call will return an error containing the the underlying data + /// this call will return an error containing the underlying data /// instead. /// /// # Examples diff --git a/src/ci/docker/host-x86_64/dist-riscv64-linux/patches/gcc/8.5.0/0002-hidden-jump-target.patch b/src/ci/docker/host-x86_64/dist-riscv64-linux/patches/gcc/8.5.0/0002-hidden-jump-target.patch index 7ae4469428b1..1ae0ecf6cb5e 100644 --- a/src/ci/docker/host-x86_64/dist-riscv64-linux/patches/gcc/8.5.0/0002-hidden-jump-target.patch +++ b/src/ci/docker/host-x86_64/dist-riscv64-linux/patches/gcc/8.5.0/0002-hidden-jump-target.patch @@ -10,7 +10,7 @@ https://sourceware.org/bugzilla/show_bug.cgi?id=28509 And this is the first version of the proposed binutils patch, https://sourceware.org/pipermail/binutils/2021-November/118398.html -After applying the binutils patch, I get the the unexpected error when +After applying the binutils patch, I get the unexpected error when building libgcc, /scratch/nelsonc/riscv-gnu-toolchain/riscv-gcc/libgcc/config/riscv/div.S:42: diff --git a/tests/ui/duplicate/multiple-types-with-same-name-and-derive.rs b/tests/ui/duplicate/multiple-types-with-same-name-and-derive.rs index 72375eb0b3e8..f3bf299aa65f 100644 --- a/tests/ui/duplicate/multiple-types-with-same-name-and-derive.rs +++ b/tests/ui/duplicate/multiple-types-with-same-name-and-derive.rs @@ -1,5 +1,5 @@ // Here, there are two types with the same name. One of these has a `derive` annotation, but in the -// expansion these `impl`s are associated to the the *other* type. There is a suggestion to remove +// expansion these `impl`s are associated to the *other* type. There is a suggestion to remove // unneeded type parameters, but because we're now point at a type with no type parameters, the // suggestion would suggest removing code from an empty span, which would ICE in nightly. // From 5298f85a0a2eadad45368137d57c1ff5a4adfd92 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Sat, 4 Jan 2025 17:21:53 +1100 Subject: [PATCH 7/8] Consolidate coverage test suite steps into a single step --- src/bootstrap/src/core/build_steps/test.rs | 186 ++++++++------------- src/bootstrap/src/core/builder/mod.rs | 2 - src/bootstrap/src/core/builder/tests.rs | 33 ++++ 3 files changed, 101 insertions(+), 120 deletions(-) diff --git a/src/bootstrap/src/core/build_steps/test.rs b/src/bootstrap/src/core/build_steps/test.rs index 84d09bbc2e09..3a19505d2c89 100644 --- a/src/bootstrap/src/core/build_steps/test.rs +++ b/src/bootstrap/src/core/build_steps/test.rs @@ -27,7 +27,7 @@ use crate::utils::helpers::{ linker_args, linker_flags, t, target_supports_cranelift_backend, up_to_date, }; use crate::utils::render_tests::{add_flags_and_try_run_tests, try_run_tests}; -use crate::{CLang, DocTests, GitRepo, Mode, envify}; +use crate::{CLang, DocTests, GitRepo, Mode, PathSet, envify}; const ADB_TEST_DIR: &str = "/data/local/tmp/work"; @@ -1185,53 +1185,6 @@ macro_rules! test { }; } -/// Declares an alias for running the [`Coverage`] tests in only one mode. -/// Adapted from [`test`]. -macro_rules! coverage_test_alias { - ( - $( #[$attr:meta] )* // allow docstrings and attributes - $name:ident { - alias_and_mode: $alias_and_mode:expr, // &'static str - default: $default:expr, // bool - only_hosts: $only_hosts:expr // bool - $( , )? // optional trailing comma - } - ) => { - $( #[$attr] )* - #[derive(Debug, Clone, PartialEq, Eq, Hash)] - pub struct $name { - pub compiler: Compiler, - pub target: TargetSelection, - } - - impl $name { - const MODE: &'static str = $alias_and_mode; - } - - impl Step for $name { - type Output = (); - const DEFAULT: bool = $default; - 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) - } - - fn make_run(run: RunConfig<'_>) { - let compiler = run.builder.compiler(run.builder.top_stage, run.build_triple()); - - run.builder.ensure($name { compiler, target: run.target }); - } - - fn run(self, builder: &Builder<'_>) { - Coverage::run_coverage_tests(builder, self.compiler, self.target, Self::MODE); - } - } - }; -} - #[derive(Debug, Copy, Clone, Hash, PartialEq, Eq, Ord, PartialOrd)] pub struct RunMakeSupport { pub compiler: Compiler, @@ -1473,99 +1426,96 @@ impl Step for RunMake { test!(Assembly { path: "tests/assembly", mode: "assembly", suite: "assembly", default: true }); -/// 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 step that will run the tests in -/// just that mode.) -#[derive(Debug, Clone, PartialEq, Eq, Hash)] +/// Runs the coverage test suite at `tests/coverage` in some or all of the +/// coverage test modes. +#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] pub struct Coverage { pub compiler: Compiler, pub target: TargetSelection, + pub mode: &'static str, } impl Coverage { const PATH: &'static str = "tests/coverage"; const SUITE: &'static str = "coverage"; - - /// 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, - target, - mode, - suite: Self::SUITE, - path: Self::PATH, - compare_mode: None, - }); - } + const ALL_MODES: &[&str] = &["coverage-map", "coverage-run"]; } 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. + const DEFAULT: bool = true; /// 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) + fn should_run(mut run: ShouldRun<'_>) -> ShouldRun<'_> { + // Support various invocation styles, including: + // - `./x test coverage` + // - `./x test tests/coverage/trivial.rs` + // - `./x test coverage-map` + // - `./x test coverage-run -- tests/coverage/trivial.rs` + run = run.suite_path(Self::PATH); + for mode in Self::ALL_MODES { + run = run.alias(mode); + } + run } fn make_run(run: RunConfig<'_>) { let compiler = run.builder.compiler(run.builder.top_stage, run.build_triple()); + let target = run.target; + + // List of (coverage) test modes that the coverage test suite will be + // run in. It's OK for this to contain duplicates, because the call to + // `Builder::ensure` below will take care of deduplication. + let mut modes = vec![]; + + // From the pathsets that were selected on the command-line (or by default), + // determine which modes to run in. + for path in &run.paths { + match path { + PathSet::Set(_) => { + for mode in Self::ALL_MODES { + if path.assert_single_path().path == Path::new(mode) { + modes.push(mode); + break; + } + } + } + PathSet::Suite(_) => { + modes.extend(Self::ALL_MODES); + break; + } + } + } + + // Skip any modes that were explicitly skipped/excluded on the command-line. + // FIXME(Zalathar): Integrate this into central skip handling somehow? + modes.retain(|mode| !run.builder.config.skip.iter().any(|skip| skip == Path::new(mode))); + + // FIXME(Zalathar): Make these commands skip all coverage tests, as expected: + // - `./x test --skip=tests` + // - `./x test --skip=tests/coverage` + // - `./x test --skip=coverage` + // Skip handling currently doesn't have a way to know that skipping the coverage + // suite should also skip the `coverage-map` and `coverage-run` aliases. - run.builder.ensure(Coverage { compiler, target: run.target }); + for mode in modes { + run.builder.ensure(Coverage { compiler, target, mode }); + } } fn run(self, builder: &Builder<'_>) { - // 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); - } -} - -coverage_test_alias! { - /// Runs the `tests/coverage` test suite in "coverage-map" mode only. - /// Used by `x test` and `x test coverage-map`. - CoverageMap { - alias_and_mode: "coverage-map", - default: true, - only_hosts: false, - } -} -coverage_test_alias! { - /// Runs the `tests/coverage` test suite in "coverage-run" mode only. - /// Used by `x test` and `x test coverage-run`. - 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, + let Self { compiler, target, mode } = self; + // Like other compiletest suite test steps, delegate to an internal + // compiletest task to actually run the tests. + builder.ensure(Compiletest { + compiler, + target, + mode, + suite: Self::SUITE, + path: Self::PATH, + compare_mode: None, + }); } } diff --git a/src/bootstrap/src/core/builder/mod.rs b/src/bootstrap/src/core/builder/mod.rs index 04d51fab5d52..adf6d9ae72ab 100644 --- a/src/bootstrap/src/core/builder/mod.rs +++ b/src/bootstrap/src/core/builder/mod.rs @@ -943,8 +943,6 @@ impl<'a> Builder<'a> { test::Ui, test::Crashes, test::Coverage, - test::CoverageMap, - test::CoverageRun, test::MirOpt, test::Codegen, test::CodegenUnits, diff --git a/src/bootstrap/src/core/builder/tests.rs b/src/bootstrap/src/core/builder/tests.rs index 5769198afac6..0c27597083de 100644 --- a/src/bootstrap/src/core/builder/tests.rs +++ b/src/bootstrap/src/core/builder/tests.rs @@ -828,3 +828,36 @@ fn test_test_compiler() { assert_eq!((compiler, cranelift, gcc), (true, false, false)); } + +#[test] +fn test_test_coverage() { + struct Case { + cmd: &'static [&'static str], + expected: &'static [&'static str], + } + let cases = &[ + Case { cmd: &["test"], expected: &["coverage-map", "coverage-run"] }, + Case { cmd: &["test", "coverage"], expected: &["coverage-map", "coverage-run"] }, + Case { cmd: &["test", "coverage-map"], expected: &["coverage-map"] }, + Case { cmd: &["test", "coverage-run"], expected: &["coverage-run"] }, + Case { cmd: &["test", "coverage", "--skip=coverage"], expected: &[] }, + Case { cmd: &["test", "coverage", "--skip=tests/coverage"], expected: &[] }, + Case { cmd: &["test", "coverage", "--skip=coverage-map"], expected: &["coverage-run"] }, + Case { cmd: &["test", "coverage", "--skip=coverage-run"], expected: &["coverage-map"] }, + Case { cmd: &["test", "--skip=coverage-map", "--skip=coverage-run"], expected: &[] }, + Case { cmd: &["test", "coverage", "--skip=tests"], expected: &[] }, + ]; + + for &Case { cmd, expected } in cases { + // Print each test case so that if one fails, the most recently printed + // case is the one that failed. + println!("Testing case: {cmd:?}"); + let cmd = cmd.iter().copied().map(str::to_owned).collect::>(); + let config = configure_with_args(&cmd, &[TEST_TRIPLE_1], &[TEST_TRIPLE_1]); + let mut cache = run_build(&config.paths.clone(), config); + + let modes = + cache.all::().iter().map(|(step, ())| step.mode).collect::>(); + assert_eq!(modes, expected); + } +} From 0705ea2a1cef7cd92cdd15ea12d30782c7bfb4bb Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Mon, 6 Jan 2025 12:13:19 +0100 Subject: [PATCH 8/8] Move the has_errors check in rustdoc back to after TyCtxt is created --- src/librustdoc/lib.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/librustdoc/lib.rs b/src/librustdoc/lib.rs index d74dcc98cb05..96ca96ee6bc1 100644 --- a/src/librustdoc/lib.rs +++ b/src/librustdoc/lib.rs @@ -865,11 +865,11 @@ fn main_args( } let krate = rustc_interface::passes::parse(sess); - if sess.dcx().has_errors().is_some() { - sess.dcx().fatal("Compilation failed, aborting rustdoc"); - } - rustc_interface::create_and_enter_global_ctxt(compiler, krate, |tcx| { + if sess.dcx().has_errors().is_some() { + sess.dcx().fatal("Compilation failed, aborting rustdoc"); + } + let (krate, render_opts, mut cache) = sess.time("run_global_ctxt", || { core::run_global_ctxt(tcx, show_coverage, render_options, output_format) });