From 41c3e9ff0e0d345e0723c265dd5904ee323c8a4b Mon Sep 17 00:00:00 2001 From: Mateusz Kowalski Date: Mon, 30 Sep 2024 15:04:27 +0200 Subject: [PATCH 1/4] Create api for diagnostics ensure with no need for lowering group --- crates/cairo-lang-compiler/src/diagnostics.rs | 48 +++++++++++++++++-- 1 file changed, 43 insertions(+), 5 deletions(-) diff --git a/crates/cairo-lang-compiler/src/diagnostics.rs b/crates/cairo-lang-compiler/src/diagnostics.rs index 264751d008a..ba751a0bf1a 100644 --- a/crates/cairo-lang-compiler/src/diagnostics.rs +++ b/crates/cairo-lang-compiler/src/diagnostics.rs @@ -3,6 +3,7 @@ use std::fmt::Write; use cairo_lang_defs::db::DefsGroup; use cairo_lang_defs::ids::ModuleId; use cairo_lang_diagnostics::{DiagnosticEntry, Diagnostics, FormattedDiagnosticEntry, Severity}; +use cairo_lang_filesystem::db::FilesGroup; use cairo_lang_filesystem::ids::{CrateId, FileLongId}; use cairo_lang_lowering::db::LoweringGroup; use cairo_lang_parser::db::ParserGroup; @@ -122,16 +123,31 @@ impl<'a> DiagnosticsReporter<'a> { } /// Returns the crate ids for which the diagnostics will be checked. - fn crates_of_interest(&self, db: &dyn LoweringGroup) -> Vec { + fn crates_of_interest(&self, db: &dyn FilesGroup) -> Vec { if self.crate_ids.is_empty() { db.crates() } else { self.crate_ids.clone() } } - /// Checks if there are diagnostics and reports them to the provided callback as strings. + /// Checks if there are diagnostics. /// Returns `true` if diagnostics were found. pub fn check(&mut self, db: &dyn LoweringGroup) -> bool { let mut found_diagnostics = false; + found_diagnostics |= self.check_common(db.upcast()); + found_diagnostics |= self.check_lowering_group(db); + found_diagnostics + } - let crates = self.crates_of_interest(db); + /// Checks if there are diagnostics. + /// Ignores all the diagnostics related to LoweringGroup. + /// Returns `true` if diagnostics were found. + pub fn check_without_lowering_group(&mut self, db: &dyn SemanticGroup) -> bool { + self.check_common(db) + } + + /// Checks if there are diagnostics and reports them to the provided callback as strings. + fn check_common(&mut self, db: &dyn SemanticGroup) -> bool { + let mut found_diagnostics = false; + + let crates = self.crates_of_interest(db.upcast()); for crate_id in &crates { let Ok(module_file) = db.module_main_file(ModuleId::CrateRoot(*crate_id)) else { found_diagnostics = true; @@ -176,10 +192,22 @@ impl<'a> DiagnosticsReporter<'a> { if let Ok(group) = db.module_semantic_diagnostics(*module_id) { found_diagnostics |= - self.check_diag_group(db.upcast(), group, ignore_warnings_in_crate); + self.check_diag_group(db.elongate(), group, ignore_warnings_in_crate); } + } + } + found_diagnostics + } - if let Ok(group) = db.module_lowering_diagnostics(*module_id) { + /// Checks if there are any diagnostics related to LoweringGroup. + fn check_lowering_group(&mut self, db: &dyn LoweringGroup) -> bool { + let mut found_diagnostics = false; + let crates = self.crates_of_interest(db.upcast()); + for crate_id in &crates { + let ignore_warnings_in_crate = self.ignore_warnings_crate_ids.contains(crate_id); + let modules = db.crate_modules(*crate_id); + for module_id in modules.iter() { + if let Ok(group) = db.module_semantic_diagnostics(*module_id) { found_diagnostics |= self.check_diag_group(db.upcast(), group, ignore_warnings_in_crate); } @@ -215,6 +243,16 @@ impl<'a> DiagnosticsReporter<'a> { if self.check(db) { Err(DiagnosticsError) } else { Ok(()) } } + /// Checks if there are diagnostics and reports them to the provided callback as strings. + /// Ignores all the diagnostics related to LoweringGroup. + /// Returns `Err` if diagnostics were found. + pub fn ensure_without_lowering_group( + &mut self, + db: &dyn SemanticGroup, + ) -> Result<(), DiagnosticsError> { + if self.check_common(db) { Err(DiagnosticsError) } else { Ok(()) } + } + /// Spawns threads to compute the diagnostics queries, making sure later calls for these queries /// would be faster as the queries were already computed. pub(crate) fn warm_up_diagnostics(&self, db: &RootDatabase) { From 1f5f052e59b209354278ca19f14986644af69744 Mon Sep 17 00:00:00 2001 From: Mateusz Kowalski Date: Tue, 1 Oct 2024 11:13:40 +0200 Subject: [PATCH 2/4] Fix tests --- crates/cairo-lang-compiler/src/diagnostics.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/cairo-lang-compiler/src/diagnostics.rs b/crates/cairo-lang-compiler/src/diagnostics.rs index ba751a0bf1a..d4833594d3e 100644 --- a/crates/cairo-lang-compiler/src/diagnostics.rs +++ b/crates/cairo-lang-compiler/src/diagnostics.rs @@ -207,7 +207,7 @@ impl<'a> DiagnosticsReporter<'a> { let ignore_warnings_in_crate = self.ignore_warnings_crate_ids.contains(crate_id); let modules = db.crate_modules(*crate_id); for module_id in modules.iter() { - if let Ok(group) = db.module_semantic_diagnostics(*module_id) { + if let Ok(group) = db.module_lowering_diagnostics(*module_id) { found_diagnostics |= self.check_diag_group(db.upcast(), group, ignore_warnings_in_crate); } From 3b46ff379c81cfd750cd3eb476793e551298ba3b Mon Sep 17 00:00:00 2001 From: Mateusz Kowalski Date: Wed, 2 Oct 2024 13:43:28 +0200 Subject: [PATCH 3/4] Make lowering diagnostics optional --- crates/cairo-lang-compiler/src/diagnostics.rs | 79 ++++++------------- 1 file changed, 26 insertions(+), 53 deletions(-) diff --git a/crates/cairo-lang-compiler/src/diagnostics.rs b/crates/cairo-lang-compiler/src/diagnostics.rs index d4833594d3e..1808389225e 100644 --- a/crates/cairo-lang-compiler/src/diagnostics.rs +++ b/crates/cairo-lang-compiler/src/diagnostics.rs @@ -3,7 +3,6 @@ use std::fmt::Write; use cairo_lang_defs::db::DefsGroup; use cairo_lang_defs::ids::ModuleId; use cairo_lang_diagnostics::{DiagnosticEntry, Diagnostics, FormattedDiagnosticEntry, Severity}; -use cairo_lang_filesystem::db::FilesGroup; use cairo_lang_filesystem::ids::{CrateId, FileLongId}; use cairo_lang_lowering::db::LoweringGroup; use cairo_lang_parser::db::ParserGroup; @@ -45,6 +44,8 @@ pub struct DiagnosticsReporter<'a> { crate_ids: Vec, /// If true, compilation will not fail due to warnings. allow_warnings: bool, + /// If the diagnostics from lowering group should be included. + include_lowering_diagnostics: bool, } impl DiagnosticsReporter<'static> { @@ -55,12 +56,13 @@ impl DiagnosticsReporter<'static> { crate_ids: vec![], ignore_warnings_crate_ids: vec![], allow_warnings: false, + include_lowering_diagnostics: true, } } /// Create a reporter which prints all diagnostics to [`std::io::Stderr`]. pub fn stderr() -> Self { - Self::callback(|diagnostic| eprint!("{diagnostic}")) + Self::callback(|diagnostic| eprint!("{diagnostic}"), true) } } @@ -69,7 +71,10 @@ impl<'a> DiagnosticsReporter<'a> { // impl DiagnosticCallback for F where F: FnMut(Severity,String) // and `new` could accept regular functions without need for this separate method. /// Create a reporter which calls `callback` for each diagnostic. - pub fn callback(callback: impl FnMut(FormattedDiagnosticEntry) + 'a) -> Self { + pub fn callback( + callback: impl FnMut(FormattedDiagnosticEntry) + 'a, + include_lowering_diagnostics: bool, + ) -> Self { struct Func(F); impl DiagnosticCallback for Func @@ -81,23 +86,27 @@ impl<'a> DiagnosticsReporter<'a> { } } - Self::new(Func(callback)) + Self::new(Func(callback), include_lowering_diagnostics) } /// Create a reporter which appends all diagnostics to provided string. pub fn write_to_string(string: &'a mut String) -> Self { - Self::callback(|diagnostic| { - write!(string, "{diagnostic}").unwrap(); - }) + Self::callback( + |diagnostic| { + write!(string, "{diagnostic}").unwrap(); + }, + true, + ) } /// Create a reporter which calls [`DiagnosticCallback::on_diagnostic`]. - fn new(callback: impl DiagnosticCallback + 'a) -> Self { + fn new(callback: impl DiagnosticCallback + 'a, include_lowering_diagnostics: bool) -> Self { Self { callback: Some(Box::new(callback)), crate_ids: vec![], ignore_warnings_crate_ids: vec![], allow_warnings: false, + include_lowering_diagnostics, } } @@ -123,31 +132,16 @@ impl<'a> DiagnosticsReporter<'a> { } /// Returns the crate ids for which the diagnostics will be checked. - fn crates_of_interest(&self, db: &dyn FilesGroup) -> Vec { + fn crates_of_interest(&self, db: &dyn LoweringGroup) -> Vec { if self.crate_ids.is_empty() { db.crates() } else { self.crate_ids.clone() } } - /// Checks if there are diagnostics. + /// Checks if there are diagnostics and reports them to the provided callback as strings. /// Returns `true` if diagnostics were found. pub fn check(&mut self, db: &dyn LoweringGroup) -> bool { let mut found_diagnostics = false; - found_diagnostics |= self.check_common(db.upcast()); - found_diagnostics |= self.check_lowering_group(db); - found_diagnostics - } - - /// Checks if there are diagnostics. - /// Ignores all the diagnostics related to LoweringGroup. - /// Returns `true` if diagnostics were found. - pub fn check_without_lowering_group(&mut self, db: &dyn SemanticGroup) -> bool { - self.check_common(db) - } - /// Checks if there are diagnostics and reports them to the provided callback as strings. - fn check_common(&mut self, db: &dyn SemanticGroup) -> bool { - let mut found_diagnostics = false; - - let crates = self.crates_of_interest(db.upcast()); + let crates = self.crates_of_interest(db); for crate_id in &crates { let Ok(module_file) = db.module_main_file(ModuleId::CrateRoot(*crate_id)) else { found_diagnostics = true; @@ -191,26 +185,15 @@ impl<'a> DiagnosticsReporter<'a> { } if let Ok(group) = db.module_semantic_diagnostics(*module_id) { - found_diagnostics |= - self.check_diag_group(db.elongate(), group, ignore_warnings_in_crate); - } - } - } - found_diagnostics - } - - /// Checks if there are any diagnostics related to LoweringGroup. - fn check_lowering_group(&mut self, db: &dyn LoweringGroup) -> bool { - let mut found_diagnostics = false; - let crates = self.crates_of_interest(db.upcast()); - for crate_id in &crates { - let ignore_warnings_in_crate = self.ignore_warnings_crate_ids.contains(crate_id); - let modules = db.crate_modules(*crate_id); - for module_id in modules.iter() { - if let Ok(group) = db.module_lowering_diagnostics(*module_id) { found_diagnostics |= self.check_diag_group(db.upcast(), group, ignore_warnings_in_crate); } + if self.include_lowering_diagnostics { + if let Ok(group) = db.module_lowering_diagnostics(*module_id) { + found_diagnostics |= + self.check_diag_group(db.upcast(), group, ignore_warnings_in_crate); + } + } } } found_diagnostics @@ -243,16 +226,6 @@ impl<'a> DiagnosticsReporter<'a> { if self.check(db) { Err(DiagnosticsError) } else { Ok(()) } } - /// Checks if there are diagnostics and reports them to the provided callback as strings. - /// Ignores all the diagnostics related to LoweringGroup. - /// Returns `Err` if diagnostics were found. - pub fn ensure_without_lowering_group( - &mut self, - db: &dyn SemanticGroup, - ) -> Result<(), DiagnosticsError> { - if self.check_common(db) { Err(DiagnosticsError) } else { Ok(()) } - } - /// Spawns threads to compute the diagnostics queries, making sure later calls for these queries /// would be faster as the queries were already computed. pub(crate) fn warm_up_diagnostics(&self, db: &RootDatabase) { From 2f84d321a9a08722957c0a50965f00f9cdd219dc Mon Sep 17 00:00:00 2001 From: Mateusz Kowalski Date: Thu, 3 Oct 2024 10:26:29 +0200 Subject: [PATCH 4/4] Change diagnostics api --- crates/cairo-lang-compiler/src/diagnostics.rs | 46 ++++++++++--------- 1 file changed, 24 insertions(+), 22 deletions(-) diff --git a/crates/cairo-lang-compiler/src/diagnostics.rs b/crates/cairo-lang-compiler/src/diagnostics.rs index 1808389225e..22641f669da 100644 --- a/crates/cairo-lang-compiler/src/diagnostics.rs +++ b/crates/cairo-lang-compiler/src/diagnostics.rs @@ -44,8 +44,8 @@ pub struct DiagnosticsReporter<'a> { crate_ids: Vec, /// If true, compilation will not fail due to warnings. allow_warnings: bool, - /// If the diagnostics from lowering group should be included. - include_lowering_diagnostics: bool, + /// If true, will ignore diagnostics from LoweringGroup during the ensure function. + skip_lowering_diagnostics: bool, } impl DiagnosticsReporter<'static> { @@ -56,13 +56,13 @@ impl DiagnosticsReporter<'static> { crate_ids: vec![], ignore_warnings_crate_ids: vec![], allow_warnings: false, - include_lowering_diagnostics: true, + skip_lowering_diagnostics: false, } } /// Create a reporter which prints all diagnostics to [`std::io::Stderr`]. pub fn stderr() -> Self { - Self::callback(|diagnostic| eprint!("{diagnostic}"), true) + Self::callback(|diagnostic| eprint!("{diagnostic}")) } } @@ -71,10 +71,7 @@ impl<'a> DiagnosticsReporter<'a> { // impl DiagnosticCallback for F where F: FnMut(Severity,String) // and `new` could accept regular functions without need for this separate method. /// Create a reporter which calls `callback` for each diagnostic. - pub fn callback( - callback: impl FnMut(FormattedDiagnosticEntry) + 'a, - include_lowering_diagnostics: bool, - ) -> Self { + pub fn callback(callback: impl FnMut(FormattedDiagnosticEntry) + 'a) -> Self { struct Func(F); impl DiagnosticCallback for Func @@ -86,27 +83,24 @@ impl<'a> DiagnosticsReporter<'a> { } } - Self::new(Func(callback), include_lowering_diagnostics) + Self::new(Func(callback)) } /// Create a reporter which appends all diagnostics to provided string. pub fn write_to_string(string: &'a mut String) -> Self { - Self::callback( - |diagnostic| { - write!(string, "{diagnostic}").unwrap(); - }, - true, - ) + Self::callback(|diagnostic| { + write!(string, "{diagnostic}").unwrap(); + }) } /// Create a reporter which calls [`DiagnosticCallback::on_diagnostic`]. - fn new(callback: impl DiagnosticCallback + 'a, include_lowering_diagnostics: bool) -> Self { + fn new(callback: impl DiagnosticCallback + 'a) -> Self { Self { callback: Some(Box::new(callback)), crate_ids: vec![], ignore_warnings_crate_ids: vec![], allow_warnings: false, - include_lowering_diagnostics, + skip_lowering_diagnostics: false, } } @@ -188,11 +182,14 @@ impl<'a> DiagnosticsReporter<'a> { found_diagnostics |= self.check_diag_group(db.upcast(), group, ignore_warnings_in_crate); } - if self.include_lowering_diagnostics { - if let Ok(group) = db.module_lowering_diagnostics(*module_id) { - found_diagnostics |= - self.check_diag_group(db.upcast(), group, ignore_warnings_in_crate); - } + + if self.skip_lowering_diagnostics { + continue; + } + + if let Ok(group) = db.module_lowering_diagnostics(*module_id) { + found_diagnostics |= + self.check_diag_group(db.upcast(), group, ignore_warnings_in_crate); } } } @@ -254,6 +251,11 @@ impl<'a> DiagnosticsReporter<'a> { }); } } + + pub fn skip_lowering_diagnostics(mut self) -> Self { + self.skip_lowering_diagnostics = true; + self + } } impl Default for DiagnosticsReporter<'static> {