Skip to content

Commit 92fd45b

Browse files
authored
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 ```
2 parents 7d928a9 + f83af2a commit 92fd45b

File tree

3 files changed

+75
-2
lines changed

3 files changed

+75
-2
lines changed

src/bootstrap/src/core/build_steps/run.rs

+25
Original file line numberDiff line numberDiff line change
@@ -367,3 +367,28 @@ impl Step for FeaturesStatusDump {
367367
cmd.run(builder);
368368
}
369369
}
370+
371+
/// Dummy step that can be used to deliberately trigger bootstrap's step cycle
372+
/// detector, for automated and manual testing.
373+
#[derive(Clone, Debug, PartialEq, Eq, Hash)]
374+
pub struct CyclicStep {
375+
n: u32,
376+
}
377+
378+
impl Step for CyclicStep {
379+
type Output = ();
380+
381+
fn should_run(run: ShouldRun<'_>) -> ShouldRun<'_> {
382+
run.alias("cyclic-step")
383+
}
384+
385+
fn make_run(run: RunConfig<'_>) {
386+
// Start with n=2, so that we build up a few stack entries before panicking.
387+
run.builder.ensure(CyclicStep { n: 2 })
388+
}
389+
390+
fn run(self, builder: &Builder<'_>) -> Self::Output {
391+
// When n=0, the step will try to ensure itself, causing a step cycle.
392+
builder.ensure(CyclicStep { n: self.n.saturating_sub(1) })
393+
}
394+
}

src/bootstrap/src/core/builder/mod.rs

+17-1
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ pub struct Builder<'a> {
5050

5151
/// A stack of [`Step`]s to run before we can run this builder. The output
5252
/// of steps is cached in [`Self::cache`].
53-
stack: RefCell<Vec<Box<dyn Any>>>,
53+
stack: RefCell<Vec<Box<dyn AnyDebug>>>,
5454

5555
/// The total amount of time we spent running [`Step`]s in [`Self::stack`].
5656
time_spent_on_dependencies: Cell<Duration>,
@@ -69,6 +69,21 @@ impl Deref for Builder<'_> {
6969
}
7070
}
7171

72+
/// This trait is similar to `Any`, except that it also exposes the underlying
73+
/// type's [`Debug`] implementation.
74+
///
75+
/// (Trying to debug-print `dyn Any` results in the unhelpful `"Any { .. }"`.)
76+
trait AnyDebug: Any + Debug {}
77+
impl<T: Any + Debug> AnyDebug for T {}
78+
impl dyn AnyDebug {
79+
/// Equivalent to `<dyn Any>::downcast_ref`.
80+
fn downcast_ref<T: Any>(&self) -> Option<&T> {
81+
(self as &dyn Any).downcast_ref()
82+
}
83+
84+
// Feel free to add other `dyn Any` methods as necessary.
85+
}
86+
7287
pub trait Step: 'static + Clone + Debug + PartialEq + Eq + Hash {
7388
/// Result type of `Step::run`.
7489
type Output: Clone;
@@ -1102,6 +1117,7 @@ impl<'a> Builder<'a> {
11021117
run::GenerateCompletions,
11031118
run::UnicodeTableGenerator,
11041119
run::FeaturesStatusDump,
1120+
run::CyclicStep,
11051121
),
11061122
Kind::Setup => {
11071123
describe!(setup::Profile, setup::Hook, setup::Link, setup::Editor)

src/bootstrap/src/core/builder/tests.rs

+33-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use std::thread;
1+
use std::{panic, thread};
22

33
use llvm::prebuilt_llvm_config;
44

@@ -1135,3 +1135,35 @@ fn test_get_tool_rustc_compiler() {
11351135
let actual = tool::get_tool_rustc_compiler(&builder, compiler);
11361136
assert_eq!(expected, actual);
11371137
}
1138+
1139+
/// When bootstrap detects a step dependency cycle (which is a bug), its panic
1140+
/// message should show the actual steps on the stack, not just several copies
1141+
/// of `Any { .. }`.
1142+
#[test]
1143+
fn step_cycle_debug() {
1144+
let cmd = ["run", "cyclic-step"].map(str::to_owned);
1145+
let config = configure_with_args(&cmd, &[TEST_TRIPLE_1], &[TEST_TRIPLE_1]);
1146+
1147+
let err = panic::catch_unwind(|| run_build(&config.paths.clone(), config)).unwrap_err();
1148+
let err = err.downcast_ref::<String>().unwrap().as_str();
1149+
1150+
assert!(!err.contains("Any"));
1151+
assert!(err.contains("CyclicStep { n: 1 }"));
1152+
}
1153+
1154+
/// The `AnyDebug` trait should delegate to the underlying type's `Debug`, and
1155+
/// should also allow downcasting as expected.
1156+
#[test]
1157+
fn any_debug() {
1158+
#[derive(Debug, PartialEq, Eq)]
1159+
struct MyStruct {
1160+
x: u32,
1161+
}
1162+
1163+
let x: &dyn AnyDebug = &MyStruct { x: 7 };
1164+
1165+
// Debug-formatting should delegate to the underlying type.
1166+
assert_eq!(format!("{x:?}"), format!("{:?}", MyStruct { x: 7 }));
1167+
// Downcasting to the underlying type should succeed.
1168+
assert_eq!(x.downcast_ref::<MyStruct>(), Some(&MyStruct { x: 7 }));
1169+
}

0 commit comments

Comments
 (0)