diff options
| author | Matthias Krüger <476013+matthiaskrgr@users.noreply.github.com> | 2025-03-09 10:34:53 +0100 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2025-03-09 10:34:53 +0100 |
| commit | 92fd45b14f5cdeaf25f2f7dd6aff49f7686880e1 (patch) | |
| tree | ffa24cf92d9772b81d3eabe55fe29f3e63392847 /src | |
| parent | 7d928a9a2e303344588f914329adfb34a3a8a04e (diff) | |
| parent | f83af2aa6b395c2e6c5639e8536f08f24d3f2128 (diff) | |
| download | rust-92fd45b14f5cdeaf25f2f7dd6aff49f7686880e1.tar.gz rust-92fd45b14f5cdeaf25f2f7dd6aff49f7686880e1.zip | |
Rollup merge of #138216 - Zalathar:any-debug, r=onur-ozkan
bootstrap: Fix stack printing when a step cycle is detected
When bootstrap detects a step dependency cycle (which represents a bootstrap bug), it is supposed to print out the contents of the step stack as part of its panic message.
However, while investigating #138205 it was found that bootstrap was actually printing out several copies of `Any { .. }`, because that is the Debug implementation for `dyn Any`. This is sadly not very helpful.
This PR fixes that problem by introducing a `trait AnyDebug: Any + Debug` that delegates to the underlying type's Debug implementation, while still allowing downcasting via Any.
---
The fixed behaviour can be verified manually (and is tested automatically) via a new dummy command, `./x run cyclic-step`:
```
$ x run cyclic-step
Building bootstrap
Finished `dev` profile [unoptimized] target(s) in 0.02s
thread 'main' panicked at src/bootstrap/src/core/builder/mod.rs:1521:17:
Cycle in build detected when adding CyclicStep { n: 0 }
CyclicStep { n: 0 }
CyclicStep { n: 1 }
CyclicStep { n: 2 }
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
Build completed unsuccessfully in 0:00:00
```
Diffstat (limited to 'src')
| -rw-r--r-- | src/bootstrap/src/core/build_steps/run.rs | 25 | ||||
| -rw-r--r-- | src/bootstrap/src/core/builder/mod.rs | 18 | ||||
| -rw-r--r-- | src/bootstrap/src/core/builder/tests.rs | 34 |
3 files changed, 75 insertions, 2 deletions
diff --git a/src/bootstrap/src/core/build_steps/run.rs b/src/bootstrap/src/core/build_steps/run.rs index fea8232296e..1ef86e674f0 100644 --- a/src/bootstrap/src/core/build_steps/run.rs +++ b/src/bootstrap/src/core/build_steps/run.rs @@ -367,3 +367,28 @@ impl Step for FeaturesStatusDump { cmd.run(builder); } } + +/// Dummy step that can be used to deliberately trigger bootstrap's step cycle +/// detector, for automated and manual testing. +#[derive(Clone, Debug, PartialEq, Eq, Hash)] +pub struct CyclicStep { + n: u32, +} + +impl Step for CyclicStep { + type Output = (); + + fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> { + run.alias("cyclic-step") + } + + fn make_run(run: RunConfig<'_>) { + // Start with n=2, so that we build up a few stack entries before panicking. + run.builder.ensure(CyclicStep { n: 2 }) + } + + fn run(self, builder: &Builder<'_>) -> Self::Output { + // When n=0, the step will try to ensure itself, causing a step cycle. + builder.ensure(CyclicStep { n: self.n.saturating_sub(1) }) + } +} diff --git a/src/bootstrap/src/core/builder/mod.rs b/src/bootstrap/src/core/builder/mod.rs index c24001b749f..8e1cecfcd18 100644 --- a/src/bootstrap/src/core/builder/mod.rs +++ b/src/bootstrap/src/core/builder/mod.rs @@ -50,7 +50,7 @@ pub struct Builder<'a> { /// A stack of [`Step`]s to run before we can run this builder. The output /// of steps is cached in [`Self::cache`]. - stack: RefCell<Vec<Box<dyn Any>>>, + stack: RefCell<Vec<Box<dyn AnyDebug>>>, /// The total amount of time we spent running [`Step`]s in [`Self::stack`]. time_spent_on_dependencies: Cell<Duration>, @@ -69,6 +69,21 @@ impl Deref for Builder<'_> { } } +/// This trait is similar to `Any`, except that it also exposes the underlying +/// type's [`Debug`] implementation. +/// +/// (Trying to debug-print `dyn Any` results in the unhelpful `"Any { .. }"`.) +trait AnyDebug: Any + Debug {} +impl<T: Any + Debug> AnyDebug for T {} +impl dyn AnyDebug { + /// Equivalent to `<dyn Any>::downcast_ref`. + fn downcast_ref<T: Any>(&self) -> Option<&T> { + (self as &dyn Any).downcast_ref() + } + + // Feel free to add other `dyn Any` methods as necessary. +} + pub trait Step: 'static + Clone + Debug + PartialEq + Eq + Hash { /// Result type of `Step::run`. type Output: Clone; @@ -1102,6 +1117,7 @@ impl<'a> Builder<'a> { run::GenerateCompletions, run::UnicodeTableGenerator, run::FeaturesStatusDump, + run::CyclicStep, ), Kind::Setup => { describe!(setup::Profile, setup::Hook, setup::Link, setup::Editor) diff --git a/src/bootstrap/src/core/builder/tests.rs b/src/bootstrap/src/core/builder/tests.rs index 63a1bbc24f1..e8820e3a828 100644 --- a/src/bootstrap/src/core/builder/tests.rs +++ b/src/bootstrap/src/core/builder/tests.rs @@ -1,4 +1,4 @@ -use std::thread; +use std::{panic, thread}; use llvm::prebuilt_llvm_config; @@ -1135,3 +1135,35 @@ fn test_get_tool_rustc_compiler() { let actual = tool::get_tool_rustc_compiler(&builder, compiler); assert_eq!(expected, actual); } + +/// When bootstrap detects a step dependency cycle (which is a bug), its panic +/// message should show the actual steps on the stack, not just several copies +/// of `Any { .. }`. +#[test] +fn step_cycle_debug() { + let cmd = ["run", "cyclic-step"].map(str::to_owned); + let config = configure_with_args(&cmd, &[TEST_TRIPLE_1], &[TEST_TRIPLE_1]); + + let err = panic::catch_unwind(|| run_build(&config.paths.clone(), config)).unwrap_err(); + let err = err.downcast_ref::<String>().unwrap().as_str(); + + assert!(!err.contains("Any")); + assert!(err.contains("CyclicStep { n: 1 }")); +} + +/// The `AnyDebug` trait should delegate to the underlying type's `Debug`, and +/// should also allow downcasting as expected. +#[test] +fn any_debug() { + #[derive(Debug, PartialEq, Eq)] + struct MyStruct { + x: u32, + } + + let x: &dyn AnyDebug = &MyStruct { x: 7 }; + + // Debug-formatting should delegate to the underlying type. + assert_eq!(format!("{x:?}"), format!("{:?}", MyStruct { x: 7 })); + // Downcasting to the underlying type should succeed. + assert_eq!(x.downcast_ref::<MyStruct>(), Some(&MyStruct { x: 7 })); +} |
