From 2db01be5844ded1139eb4fb5f1434d0090b6ba38 Mon Sep 17 00:00:00 2001 From: "Celina G. Val" Date: Wed, 30 Aug 2023 16:50:07 -0700 Subject: [PATCH 1/4] Adjust StableMIR interface to return and not crash Invoking StableMir::run() on a crate that has any compilation error was crashing the entire process. Instead, return a `CompilerError` so the user knows compilation did not succeed. I believe ICE will also be converted to `CompilerError`. I'm also adding a return value to the callback, because I think it will be handy for users (at least it was for my current task of implementing a tool to validate stable-mir). However, if people disagree, I can remove that. --- compiler/rustc_smir/src/rustc_internal/mod.rs | 35 +++++++++++++------ compiler/rustc_smir/src/rustc_smir/mod.rs | 9 ++++- compiler/rustc_smir/src/stable_mir/mod.rs | 4 +++ 3 files changed, 37 insertions(+), 11 deletions(-) diff --git a/compiler/rustc_smir/src/rustc_internal/mod.rs b/compiler/rustc_smir/src/rustc_internal/mod.rs index 2e219e17a5f6e..4496a58327eba 100644 --- a/compiler/rustc_smir/src/rustc_internal/mod.rs +++ b/compiler/rustc_smir/src/rustc_internal/mod.rs @@ -7,6 +7,7 @@ use std::fmt::Debug; use std::ops::Index; use crate::rustc_internal; +use crate::stable_mir::CompilerError; use crate::{ rustc_smir::Tables, stable_mir::{self, with}, @@ -17,6 +18,7 @@ use rustc_middle::mir::interpret::AllocId; use rustc_middle::ty::TyCtxt; use rustc_session::EarlyErrorHandler; pub use rustc_span::def_id::{CrateNum, DefId}; +use rustc_span::ErrorGuaranteed; fn with_tables(mut f: impl FnMut(&mut Tables<'_>) -> R) -> R { let mut ret = None; @@ -189,27 +191,38 @@ pub(crate) fn opaque(value: &T) -> Opaque { Opaque(format!("{value:?}")) } -pub struct StableMir { +pub struct StableMir +where + T: Send, +{ args: Vec, - callback: fn(TyCtxt<'_>), + callback: fn(TyCtxt<'_>) -> T, + result: Option, } -impl StableMir { +impl StableMir +where + T: Send, +{ /// Creates a new `StableMir` instance, with given test_function and arguments. - pub fn new(args: Vec, callback: fn(TyCtxt<'_>)) -> Self { - StableMir { args, callback } + pub fn new(args: Vec, callback: fn(TyCtxt<'_>) -> T) -> Self { + StableMir { args, callback, result: None } } /// Runs the compiler against given target and tests it with `test_function` - pub fn run(&mut self) { + pub fn run(mut self) -> Result { rustc_driver::catch_fatal_errors(|| { - RunCompiler::new(&self.args.clone(), self).run().unwrap(); + RunCompiler::new(&self.args.clone(), &mut self).run().unwrap(); }) - .unwrap(); + .map_err(|e| >::into(e))?; + Ok(self.result.unwrap()) } } -impl Callbacks for StableMir { +impl Callbacks for StableMir +where + T: Send, +{ /// Called after analysis. Return value instructs the compiler whether to /// continue the compilation afterwards (defaults to `Compilation::Continue`) fn after_analysis<'tcx>( @@ -219,7 +232,9 @@ impl Callbacks for StableMir { queries: &'tcx Queries<'tcx>, ) -> Compilation { queries.global_ctxt().unwrap().enter(|tcx| { - rustc_internal::run(tcx, || (self.callback)(tcx)); + rustc_internal::run(tcx, || { + self.result = Some((self.callback)(tcx)); + }); }); // No need to keep going. Compilation::Stop diff --git a/compiler/rustc_smir/src/rustc_smir/mod.rs b/compiler/rustc_smir/src/rustc_smir/mod.rs index b4c150c591c48..827b51e8a3658 100644 --- a/compiler/rustc_smir/src/rustc_smir/mod.rs +++ b/compiler/rustc_smir/src/rustc_smir/mod.rs @@ -10,12 +10,13 @@ use crate::rustc_internal::{self, opaque}; use crate::stable_mir::mir::{CopyNonOverlapping, UserTypeProjection, VariantIdx}; use crate::stable_mir::ty::{FloatTy, GenericParamDef, IntTy, Movability, RigidTy, TyKind, UintTy}; -use crate::stable_mir::{self, Context}; +use crate::stable_mir::{self, CompilerError, Context}; use rustc_hir as hir; use rustc_middle::mir::interpret::{alloc_range, AllocId}; use rustc_middle::mir::{self, ConstantKind}; use rustc_middle::ty::{self, Ty, TyCtxt, Variance}; use rustc_span::def_id::{CrateNum, DefId, LOCAL_CRATE}; +use rustc_span::ErrorGuaranteed; use rustc_target::abi::FieldIdx; use tracing::debug; @@ -1452,3 +1453,9 @@ impl<'tcx> Stable<'tcx> for rustc_span::Span { opaque(self) } } + +impl From for CompilerError { + fn from(_error: ErrorGuaranteed) -> Self { + CompilerError + } +} diff --git a/compiler/rustc_smir/src/stable_mir/mod.rs b/compiler/rustc_smir/src/stable_mir/mod.rs index bfc2c15a42eb0..281d5aafb4cc3 100644 --- a/compiler/rustc_smir/src/stable_mir/mod.rs +++ b/compiler/rustc_smir/src/stable_mir/mod.rs @@ -56,6 +56,10 @@ pub type TraitDecls = Vec; /// A list of impl trait decls. pub type ImplTraitDecls = Vec; +/// An error type used to represent an error that has already been reported by the compiler. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub struct CompilerError; + /// Holds information about a crate. #[derive(Clone, PartialEq, Eq, Debug)] pub struct Crate { From 3b01f65aa5cd19fe02b1bfd4e7e60390d8ef83fc Mon Sep 17 00:00:00 2001 From: "Celina G. Val" Date: Thu, 31 Aug 2023 16:53:28 -0700 Subject: [PATCH 2/4] Diferentiate between ICE and compilation error --- compiler/rustc_smir/src/rustc_internal/mod.rs | 14 ++++++++------ compiler/rustc_smir/src/rustc_smir/mod.rs | 2 +- compiler/rustc_smir/src/stable_mir/mod.rs | 7 ++++++- tests/ui-fulldeps/stable-mir/crate-info.rs | 2 +- 4 files changed, 16 insertions(+), 9 deletions(-) diff --git a/compiler/rustc_smir/src/rustc_internal/mod.rs b/compiler/rustc_smir/src/rustc_internal/mod.rs index 4496a58327eba..1c5a0924f4a95 100644 --- a/compiler/rustc_smir/src/rustc_internal/mod.rs +++ b/compiler/rustc_smir/src/rustc_internal/mod.rs @@ -18,7 +18,6 @@ use rustc_middle::mir::interpret::AllocId; use rustc_middle::ty::TyCtxt; use rustc_session::EarlyErrorHandler; pub use rustc_span::def_id::{CrateNum, DefId}; -use rustc_span::ErrorGuaranteed; fn with_tables(mut f: impl FnMut(&mut Tables<'_>) -> R) -> R { let mut ret = None; @@ -211,11 +210,14 @@ where /// Runs the compiler against given target and tests it with `test_function` pub fn run(mut self) -> Result { - rustc_driver::catch_fatal_errors(|| { - RunCompiler::new(&self.args.clone(), &mut self).run().unwrap(); - }) - .map_err(|e| >::into(e))?; - Ok(self.result.unwrap()) + let compiler_result = rustc_driver::catch_fatal_errors(|| { + RunCompiler::new(&self.args.clone(), &mut self).run() + }); + match compiler_result { + Ok(Ok(())) => Ok(self.result.unwrap()), + Ok(Err(_)) => Err(CompilerError::CompilationFailed), + Err(_) => Err(CompilerError::ICE), + } } } diff --git a/compiler/rustc_smir/src/rustc_smir/mod.rs b/compiler/rustc_smir/src/rustc_smir/mod.rs index 827b51e8a3658..3909c3d85ebfa 100644 --- a/compiler/rustc_smir/src/rustc_smir/mod.rs +++ b/compiler/rustc_smir/src/rustc_smir/mod.rs @@ -1456,6 +1456,6 @@ impl<'tcx> Stable<'tcx> for rustc_span::Span { impl From for CompilerError { fn from(_error: ErrorGuaranteed) -> Self { - CompilerError + CompilerError::CompilationFailed } } diff --git a/compiler/rustc_smir/src/stable_mir/mod.rs b/compiler/rustc_smir/src/stable_mir/mod.rs index 281d5aafb4cc3..bdbbab68e396a 100644 --- a/compiler/rustc_smir/src/stable_mir/mod.rs +++ b/compiler/rustc_smir/src/stable_mir/mod.rs @@ -58,7 +58,12 @@ pub type ImplTraitDecls = Vec; /// An error type used to represent an error that has already been reported by the compiler. #[derive(Clone, Copy, Debug, PartialEq, Eq)] -pub struct CompilerError; +pub enum CompilerError { + /// Internal compiler error (I.e.: Compiler crashed). + ICE, + /// Compilation failed. + CompilationFailed, +} /// Holds information about a crate. #[derive(Clone, PartialEq, Eq, Debug)] diff --git a/tests/ui-fulldeps/stable-mir/crate-info.rs b/tests/ui-fulldeps/stable-mir/crate-info.rs index 00dce3e004e63..5439a884637e5 100644 --- a/tests/ui-fulldeps/stable-mir/crate-info.rs +++ b/tests/ui-fulldeps/stable-mir/crate-info.rs @@ -136,7 +136,7 @@ fn main() { CRATE_NAME.to_string(), path.to_string(), ]; - rustc_internal::StableMir::new(args, test_stable_mir).run(); + rustc_internal::StableMir::new(args, test_stable_mir).run().unwrap(); } fn generate_input(path: &str) -> std::io::Result<()> { From 1a8a5d0a296f24b5184a3997e6dbedb2d31c079a Mon Sep 17 00:00:00 2001 From: "Celina G. Val" Date: Thu, 31 Aug 2023 20:40:48 -0700 Subject: [PATCH 3/4] SMIR: Allow users to pick if compilation continues Currently we stop compilation, but some users might want to keep going. This is needed for us to test against rustc tests. Other tools, such as Kani, also implements parts of their logic as a backend so it is important for compilation to continue. --- compiler/rustc_smir/src/rustc_internal/mod.rs | 28 +++++++++++++------ 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/compiler/rustc_smir/src/rustc_internal/mod.rs b/compiler/rustc_smir/src/rustc_internal/mod.rs index 1c5a0924f4a95..26127c5eb854b 100644 --- a/compiler/rustc_smir/src/rustc_internal/mod.rs +++ b/compiler/rustc_smir/src/rustc_internal/mod.rs @@ -196,6 +196,7 @@ where { args: Vec, callback: fn(TyCtxt<'_>) -> T, + after_analysis: Compilation, result: Option, } @@ -205,16 +206,27 @@ where { /// Creates a new `StableMir` instance, with given test_function and arguments. pub fn new(args: Vec, callback: fn(TyCtxt<'_>) -> T) -> Self { - StableMir { args, callback, result: None } + StableMir { args, callback, result: None, after_analysis: Compilation::Stop } + } + + /// Configure object to stop compilation after callback is called. + pub fn stop_compilation(&mut self) -> &mut Self { + self.after_analysis = Compilation::Stop; + self + } + + /// Configure object to continue compilation after callback is called. + pub fn continue_compilation(&mut self) -> &mut Self { + self.after_analysis = Compilation::Continue; + self } /// Runs the compiler against given target and tests it with `test_function` - pub fn run(mut self) -> Result { - let compiler_result = rustc_driver::catch_fatal_errors(|| { - RunCompiler::new(&self.args.clone(), &mut self).run() - }); + pub fn run(&mut self) -> Result { + let compiler_result = + rustc_driver::catch_fatal_errors(|| RunCompiler::new(&self.args.clone(), self).run()); match compiler_result { - Ok(Ok(())) => Ok(self.result.unwrap()), + Ok(Ok(())) => Ok(self.result.take().unwrap()), Ok(Err(_)) => Err(CompilerError::CompilationFailed), Err(_) => Err(CompilerError::ICE), } @@ -238,7 +250,7 @@ where self.result = Some((self.callback)(tcx)); }); }); - // No need to keep going. - Compilation::Stop + // Let users define if they want to stop compilation. + self.after_analysis } } From d10d8290ac50b5ea8c0ca72bf216cf414b2ada24 Mon Sep 17 00:00:00 2001 From: "Celina G. Val" Date: Fri, 1 Sep 2023 21:12:46 -0700 Subject: [PATCH 4/4] Add tests and use ControlFlow --- compiler/rustc_smir/src/rustc_internal/mod.rs | 59 +++++++------- compiler/rustc_smir/src/rustc_smir/mod.rs | 2 +- compiler/rustc_smir/src/stable_mir/mod.rs | 7 +- .../stable-mir/compilation-result.rs | 77 +++++++++++++++++++ tests/ui-fulldeps/stable-mir/crate-info.rs | 5 +- 5 files changed, 115 insertions(+), 35 deletions(-) create mode 100644 tests/ui-fulldeps/stable-mir/compilation-result.rs diff --git a/compiler/rustc_smir/src/rustc_internal/mod.rs b/compiler/rustc_smir/src/rustc_internal/mod.rs index 26127c5eb854b..73b8e060dd830 100644 --- a/compiler/rustc_smir/src/rustc_internal/mod.rs +++ b/compiler/rustc_smir/src/rustc_internal/mod.rs @@ -4,7 +4,7 @@ //! until stable MIR is complete. use std::fmt::Debug; -use std::ops::Index; +use std::ops::{ControlFlow, Index}; use crate::rustc_internal; use crate::stable_mir::CompilerError; @@ -190,52 +190,44 @@ pub(crate) fn opaque(value: &T) -> Opaque { Opaque(format!("{value:?}")) } -pub struct StableMir +pub struct StableMir where - T: Send, + B: Send, + C: Send, { args: Vec, - callback: fn(TyCtxt<'_>) -> T, - after_analysis: Compilation, - result: Option, + callback: fn(TyCtxt<'_>) -> ControlFlow, + result: Option>, } -impl StableMir +impl StableMir where - T: Send, + B: Send, + C: Send, { /// Creates a new `StableMir` instance, with given test_function and arguments. - pub fn new(args: Vec, callback: fn(TyCtxt<'_>) -> T) -> Self { - StableMir { args, callback, result: None, after_analysis: Compilation::Stop } - } - - /// Configure object to stop compilation after callback is called. - pub fn stop_compilation(&mut self) -> &mut Self { - self.after_analysis = Compilation::Stop; - self - } - - /// Configure object to continue compilation after callback is called. - pub fn continue_compilation(&mut self) -> &mut Self { - self.after_analysis = Compilation::Continue; - self + pub fn new(args: Vec, callback: fn(TyCtxt<'_>) -> ControlFlow) -> Self { + StableMir { args, callback, result: None } } /// Runs the compiler against given target and tests it with `test_function` - pub fn run(&mut self) -> Result { + pub fn run(&mut self) -> Result> { let compiler_result = rustc_driver::catch_fatal_errors(|| RunCompiler::new(&self.args.clone(), self).run()); - match compiler_result { - Ok(Ok(())) => Ok(self.result.take().unwrap()), - Ok(Err(_)) => Err(CompilerError::CompilationFailed), - Err(_) => Err(CompilerError::ICE), + match (compiler_result, self.result.take()) { + (Ok(Ok(())), Some(ControlFlow::Continue(value))) => Ok(value), + (Ok(Ok(())), Some(ControlFlow::Break(value))) => Err(CompilerError::Interrupted(value)), + (Ok(Ok(_)), None) => Err(CompilerError::Skipped), + (Ok(Err(_)), _) => Err(CompilerError::CompilationFailed), + (Err(_), _) => Err(CompilerError::ICE), } } } -impl Callbacks for StableMir +impl Callbacks for StableMir where - T: Send, + B: Send, + C: Send, { /// Called after analysis. Return value instructs the compiler whether to /// continue the compilation afterwards (defaults to `Compilation::Continue`) @@ -249,8 +241,11 @@ where rustc_internal::run(tcx, || { self.result = Some((self.callback)(tcx)); }); - }); - // Let users define if they want to stop compilation. - self.after_analysis + if self.result.as_ref().is_some_and(|val| val.is_continue()) { + Compilation::Continue + } else { + Compilation::Stop + } + }) } } diff --git a/compiler/rustc_smir/src/rustc_smir/mod.rs b/compiler/rustc_smir/src/rustc_smir/mod.rs index 3909c3d85ebfa..52ba4bd4e579d 100644 --- a/compiler/rustc_smir/src/rustc_smir/mod.rs +++ b/compiler/rustc_smir/src/rustc_smir/mod.rs @@ -1454,7 +1454,7 @@ impl<'tcx> Stable<'tcx> for rustc_span::Span { } } -impl From for CompilerError { +impl From for CompilerError { fn from(_error: ErrorGuaranteed) -> Self { CompilerError::CompilationFailed } diff --git a/compiler/rustc_smir/src/stable_mir/mod.rs b/compiler/rustc_smir/src/stable_mir/mod.rs index bdbbab68e396a..f9eafd9de7ad8 100644 --- a/compiler/rustc_smir/src/stable_mir/mod.rs +++ b/compiler/rustc_smir/src/stable_mir/mod.rs @@ -58,11 +58,16 @@ pub type ImplTraitDecls = Vec; /// An error type used to represent an error that has already been reported by the compiler. #[derive(Clone, Copy, Debug, PartialEq, Eq)] -pub enum CompilerError { +pub enum CompilerError { /// Internal compiler error (I.e.: Compiler crashed). ICE, /// Compilation failed. CompilationFailed, + /// Compilation was interrupted. + Interrupted(T), + /// Compilation skipped. This happens when users invoke rustc to retrieve information such as + /// --version. + Skipped, } /// Holds information about a crate. diff --git a/tests/ui-fulldeps/stable-mir/compilation-result.rs b/tests/ui-fulldeps/stable-mir/compilation-result.rs new file mode 100644 index 0000000000000..23a9e2a064cb6 --- /dev/null +++ b/tests/ui-fulldeps/stable-mir/compilation-result.rs @@ -0,0 +1,77 @@ +// run-pass +// Test StableMIR behavior when different results are given + +// ignore-stage1 +// ignore-cross-compile +// ignore-remote +// edition: 2021 + +#![feature(rustc_private)] +#![feature(assert_matches)] + +extern crate rustc_middle; +extern crate rustc_smir; + +use rustc_middle::ty::TyCtxt; +use rustc_smir::{rustc_internal, stable_mir}; +use std::io::Write; +use std::ops::ControlFlow; + +/// This test will generate and analyze a dummy crate using the stable mir. +/// For that, it will first write the dummy crate into a file. +/// Then it will create a `StableMir` using custom arguments and then +/// it will run the compiler. +fn main() { + let path = "input_compilation_result_test.rs"; + generate_input(&path).unwrap(); + let args = vec!["rustc".to_string(), path.to_string()]; + test_continue(args.clone()); + test_break(args.clone()); + test_failed(args.clone()); + test_skipped(args); +} + +fn test_continue(args: Vec) { + let continue_fn = |_: TyCtxt| ControlFlow::Continue::<(), bool>(true); + let result = rustc_internal::StableMir::new(args, continue_fn).run(); + assert_eq!(result, Ok(true)); +} + +fn test_break(args: Vec) { + let continue_fn = |_: TyCtxt| ControlFlow::Break::(false); + let result = rustc_internal::StableMir::new(args, continue_fn).run(); + assert_eq!(result, Err(stable_mir::CompilerError::Interrupted(false))); +} + +fn test_skipped(mut args: Vec) { + args.push("--version".to_string()); + let unreach_fn = |_: TyCtxt| -> ControlFlow<()> { unreachable!() }; + let result = rustc_internal::StableMir::new(args, unreach_fn).run(); + assert_eq!(result, Err(stable_mir::CompilerError::Skipped)); +} + +fn test_failed(mut args: Vec) { + args.push("--cfg=broken".to_string()); + let unreach_fn = |_: TyCtxt| -> ControlFlow<()> { unreachable!() }; + let result = rustc_internal::StableMir::new(args, unreach_fn).run(); + assert_eq!(result, Err(stable_mir::CompilerError::CompilationFailed)); +} + +fn generate_input(path: &str) -> std::io::Result<()> { + let mut file = std::fs::File::create(path)?; + write!( + file, + r#" + // This should trigger a compilation failure when enabled. + #[cfg(broken)] + mod broken_mod {{ + fn call_invalid() {{ + invalid_fn(); + }} + }} + + fn main() {{}} + "# + )?; + Ok(()) +} diff --git a/tests/ui-fulldeps/stable-mir/crate-info.rs b/tests/ui-fulldeps/stable-mir/crate-info.rs index 5439a884637e5..182f97373ea44 100644 --- a/tests/ui-fulldeps/stable-mir/crate-info.rs +++ b/tests/ui-fulldeps/stable-mir/crate-info.rs @@ -18,11 +18,12 @@ use rustc_middle::ty::TyCtxt; use rustc_smir::{rustc_internal, stable_mir}; use std::assert_matches::assert_matches; use std::io::Write; +use std::ops::ControlFlow; const CRATE_NAME: &str = "input"; /// This function uses the Stable MIR APIs to get information about the test crate. -fn test_stable_mir(tcx: TyCtxt<'_>) { +fn test_stable_mir(tcx: TyCtxt<'_>) -> ControlFlow<()> { // Get the local crate using stable_mir API. let local = stable_mir::local_crate(); assert_eq!(&local.name, CRATE_NAME); @@ -108,6 +109,8 @@ fn test_stable_mir(tcx: TyCtxt<'_>) { stable_mir::mir::Terminator::Assert { .. } => {} other => panic!("{other:?}"), } + + ControlFlow::Continue(()) } // Use internal API to find a function in a crate.