From 5878fcfdb8ce926ce1aee48ddc07282dcc953c4e Mon Sep 17 00:00:00 2001 From: Gil Ben-Shachar <111221618+gilbens-starkware@users.noreply.github.com> Date: Mon, 6 Nov 2023 15:21:20 +0200 Subject: [PATCH] Raise diagnostic on impl inference cycle. (#4367) --- crates/cairo-lang-semantic/src/db.rs | 2 + crates/cairo-lang-semantic/src/diagnostic.rs | 4 +- .../src/expr/test_data/generics | 34 ++++++++++++++ crates/cairo-lang-semantic/src/items/imp.rs | 33 ++++++++++++-- .../src/items/tests/module | 2 +- .../cairo-lang-semantic/src/items/tests/trait | 44 ++++++++++++++++++- 6 files changed, 112 insertions(+), 7 deletions(-) diff --git a/crates/cairo-lang-semantic/src/db.rs b/crates/cairo-lang-semantic/src/db.rs index e490e7a04aa..c83a0e59f0f 100644 --- a/crates/cairo-lang-semantic/src/db.rs +++ b/crates/cairo-lang-semantic/src/db.rs @@ -458,6 +458,7 @@ pub trait SemanticGroup: // ============== /// Returns candidate [ImplDefId]s for a specific trait lookup constraint. #[salsa::invoke(items::imp::module_impl_ids_for_trait_filter)] + #[salsa::cycle(items::imp::module_impl_ids_for_trait_filter_cycle)] fn module_impl_ids_for_trait_filter( &self, module_id: ModuleId, @@ -497,6 +498,7 @@ pub trait SemanticGroup: fn impl_def_resolver_data(&self, impl_def_id: ImplDefId) -> Maybe>; /// Returns the concrete trait that is implemented by the impl. #[salsa::invoke(items::imp::impl_def_concrete_trait)] + #[salsa::cycle(items::imp::impl_def_concrete_trait_cycle)] fn impl_def_concrete_trait(&self, impl_def_id: ImplDefId) -> Maybe; /// Returns the attributes attached to the impl. #[salsa::invoke(items::imp::impl_def_attributes)] diff --git a/crates/cairo-lang-semantic/src/diagnostic.rs b/crates/cairo-lang-semantic/src/diagnostic.rs index b4800c5b5aa..75dd0c4e71b 100644 --- a/crates/cairo-lang-semantic/src/diagnostic.rs +++ b/crates/cairo-lang-semantic/src/diagnostic.rs @@ -135,7 +135,9 @@ impl DiagnosticEntry for SemanticDiagnostic { "Cycle detected while resolving 'impls alias' items.".into() } SemanticDiagnosticKind::ImplRequirementCycle => { - "Cycle detected while resolving generic param.".into() + "Cycle detected while resolving generic param. Try specifying the generic impl \ + parameter explicitly to break the cycle." + .into() } SemanticDiagnosticKind::MissingMember { member_name } => { format!(r#"Missing member "{member_name}"."#) diff --git a/crates/cairo-lang-semantic/src/expr/test_data/generics b/crates/cairo-lang-semantic/src/expr/test_data/generics index 3689b9eea30..1a398866381 100644 --- a/crates/cairo-lang-semantic/src/expr/test_data/generics +++ b/crates/cairo-lang-semantic/src/expr/test_data/generics @@ -433,3 +433,37 @@ error: Trait not found. --> lib.cairo:1:16 fn bar() {} ^*****^ + +//! > ========================================================================== + +//! > Test impl infernce cycle + +//! > test_runner_name +test_function_diagnostics(expect_diagnostics: true) + +//! > function +trait Introspect { +} + +#[derive(Drop)] +struct Generic> { + value: T, +} + +fn foo() { + let generic = Generic { value: 123 }; +} + +impl GenericStructIntrospect> of Introspect> {} +impl Felt252Introspect of Introspect {} + +//! > function_name +foo + +//! > module_code + +//! > expected_diagnostics +error: Cycle detected while resolving generic param. Try specifying the generic impl parameter explicitly to break the cycle. + --> lib.cairo:13:72 +impl GenericStructIntrospect> of Introspect> {} + ^********************^ diff --git a/crates/cairo-lang-semantic/src/items/imp.rs b/crates/cairo-lang-semantic/src/items/imp.rs index c8a96276fa9..cb5234668b8 100644 --- a/crates/cairo-lang-semantic/src/items/imp.rs +++ b/crates/cairo-lang-semantic/src/items/imp.rs @@ -253,6 +253,17 @@ pub fn impl_def_concrete_trait( db.priv_impl_declaration_data(impl_def_id)?.concrete_trait } +/// Cycle handling for [crate::db::SemanticGroup::impl_def_concrete_trait]. +pub fn impl_def_concrete_trait_cycle( + _db: &dyn SemanticGroup, + _cycle: &[String], + _impl_def_id: &ImplDefId, +) -> Maybe { + // The diagnostics will be reported from the calling function, specifically from + // `priv_impl_declaration_data_inner`. + Err(skip_diagnostic()) +} + /// Query implementation of [crate::db::SemanticGroup::impl_def_attributes]. pub fn impl_def_attributes( db: &dyn SemanticGroup, @@ -364,10 +375,10 @@ pub fn priv_impl_declaration_data_inner( .and_then(|concrete_item| { try_extract_matches!(concrete_item, ResolvedConcreteItem::Trait) }) + .ok_or_else(|| diagnostics.report(&trait_path_syntax, NotATrait)) } else { - None - } - .ok_or_else(|| diagnostics.report(&trait_path_syntax, NotATrait)); + Err(diagnostics.report(&trait_path_syntax, ImplRequirementCycle)) + }; // Check fully resolved. if let Some((stable_ptr, inference_err)) = resolver.inference().finalize() { @@ -719,7 +730,9 @@ pub fn module_impl_ids_for_trait_filter( if trait_id != trait_filter.trait_id { continue; } - let Ok(concrete_trait_id) = uninferred_impl.concrete_trait(db) else { continue }; + let Ok(concrete_trait_id) = uninferred_impl.concrete_trait(db) else { + continue; + }; if let Ok(true) = concrete_trait_fits_trait_filter(db, concrete_trait_id, &trait_filter) { res.push(uninferred_impl); } @@ -728,6 +741,18 @@ pub fn module_impl_ids_for_trait_filter( Ok(res) } +/// Cycle handeling for [crate::db::SemanticGroup::module_impl_ids_for_trait_filter]. +pub fn module_impl_ids_for_trait_filter_cycle( + _db: &dyn SemanticGroup, + _cycle: &[String], + _module_id: &ModuleId, + _trait_filter: &TraitFilter, +) -> Maybe> { + // The diagnostics will be reported from the calling function, specifically from + // `priv_impl_declaration_data_inner`. + Err(skip_diagnostic()) +} + /// Checks whether an [ImplDefId] passes a [TraitFilter]. fn concrete_trait_fits_trait_filter( db: &dyn SemanticGroup, diff --git a/crates/cairo-lang-semantic/src/items/tests/module b/crates/cairo-lang-semantic/src/items/tests/module index e682dbc4918..cb0d4b12399 100644 --- a/crates/cairo-lang-semantic/src/items/tests/module +++ b/crates/cairo-lang-semantic/src/items/tests/module @@ -69,7 +69,7 @@ error: The name `abc` is defined multiple times. impl abc of abc; ^*^ -error: Not a trait. +error: Cycle detected while resolving generic param. Try specifying the generic impl parameter explicitly to break the cycle. --> lib.cairo:22:13 impl abc of abc; ^*^ diff --git a/crates/cairo-lang-semantic/src/items/tests/trait b/crates/cairo-lang-semantic/src/items/tests/trait index d5e3cbebbe3..63e76b55613 100644 --- a/crates/cairo-lang-semantic/src/items/tests/trait +++ b/crates/cairo-lang-semantic/src/items/tests/trait @@ -141,7 +141,7 @@ error: Return type of impl function `MyImpl2::no_ret_ty` is incompatible with `M fn no_ret_ty(ref a: u128) { ^ -error: Not a trait. +error: Cycle detected while resolving generic param. Try specifying the generic impl parameter explicitly to break the cycle. --> lib.cairo:34:13 impl abc of abc; ^*^ @@ -619,3 +619,45 @@ error: The number of parameters in the impl function `MyImpl::foo` is incompatib --> lib.cairo:7:14 fn foo(a: felt252, b: S, c: felt252) {} ^**************************^ + +//! > ========================================================================== + +//! > Test impl of an impl instead of trait. + +//! > test_runner_name +test_function_diagnostics(expect_diagnostics: true) + +//! > function +fn bla() { +} + +//! > function_name +bla + +//! > module_code +trait t1 {} +impl imp1 of t1 {} +impl imp2 of imp1 {} +impl imp3 of imp3 {} +mod inner { + trait t_inner {} + impl imp of t_inner {} +} +use inner::imp as inner_imp; +impl imp4 of inner_imp {} + +//! > expected_diagnostics +error: Not a trait. + --> lib.cairo:3:14 +impl imp2 of imp1 {} + ^**^ + +error: Cycle detected while resolving generic param. Try specifying the generic impl parameter explicitly to break the cycle. + --> lib.cairo:4:14 +impl imp3 of imp3 {} + ^**^ + +error: Not a trait. + --> lib.cairo:10:14 +impl imp4 of inner_imp {} + ^*******^