bootstrap: Fix stack printing when a step cycle is detected

Author: Zalathar

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) })
+    }
+}

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;
@@ -1101,6 +1116,7 @@ impl<'a> Builder<'a> {
                 run::GenerateCompletions,
                 run::UnicodeTableGenerator,
                 run::FeaturesStatusDump,
+                run::CyclicStep,
             ),
             Kind::Setup => {
                 describe!(setup::Profile, setup::Hook, setup::Link, setup::Editor)

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 }));
+}