fix(semantic): add salsa cycle recovery to the item generic-params queries

crates/cairo-lang-semantic/src/items/enm.rs

@@ -14,7 +14,10 @@ use cairo_lang_utils::ordered_hash_map::{Entry, OrderedHashMap};
 use salsa::Database;
 
 use super::attribute::SemanticQueryAttrs;
-use super::generics::{GenericParamsData, semantic_generic_params};
+use super::generics::{
+    GenericParamsData, generic_params_data_cycle, generic_params_data_initial,
+    semantic_generic_params,
+};
 use crate::corelib::unit_ty;
 use crate::diagnostic::SemanticDiagnosticKind::*;
 use crate::diagnostic::{SemanticDiagnostics, SemanticDiagnosticsBuilder};
@@ -70,7 +73,7 @@ fn enum_declaration_data<'db>(
 }
 
 /// Returns the generic parameters data of an enum.
-#[salsa::tracked(returns(ref))]
+#[salsa::tracked(returns(ref), cycle_fn=generic_params_data_cycle, cycle_initial=generic_params_data_initial)]
 fn enum_generic_params_data<'db>(
     db: &'db dyn Database,
     enum_id: EnumId<'db>,

crates/cairo-lang-semantic/src/items/extern_function.rs

@@ -13,7 +13,10 @@ use salsa::Database;
 
 use super::function_with_body::get_inline_config;
 use super::functions::{FunctionDeclarationData, GenericFunctionId, InlineConfiguration};
-use super::generics::{GenericParamsData, semantic_generic_params};
+use super::generics::{
+    GenericParamsData, generic_params_data_cycle, generic_params_data_initial,
+    semantic_generic_params,
+};
 use super::report_extern_item_outside_corelib;
 use crate::corelib::get_core_generic_function_id;
 use crate::diagnostic::SemanticDiagnosticKind::*;
@@ -33,7 +36,7 @@ mod test;
 
 /// Query implementation of
 /// [ExternFunctionSemantic::extern_function_declaration_generic_params_data].
-#[salsa::tracked(returns(ref))]
+#[salsa::tracked(returns(ref), cycle_fn=generic_params_data_cycle, cycle_initial=generic_params_data_initial)]
 fn extern_function_declaration_generic_params_data<'db>(
     db: &'db dyn Database,
     extern_function_id: ExternFunctionId<'db>,

crates/cairo-lang-semantic/src/items/free_function.rs

@@ -16,7 +16,10 @@ use super::functions::{
     FunctionDeclarationData, GenericFunctionId, InlineConfiguration,
     forbid_inline_always_with_impl_generic_param,
 };
-use super::generics::{GenericParamsData, semantic_generic_params};
+use super::generics::{
+    GenericParamsData, generic_params_data_cycle, generic_params_data_initial,
+    semantic_generic_params,
+};
 use crate::diagnostic::SemanticDiagnostics;
 use crate::expr::compute::{ComputationContext, ContextFunction, Environment, compute_root_expr};
 use crate::expr::inference::InferenceId;
@@ -32,7 +35,7 @@ use crate::{FunctionLongId, GenericParam, SemanticDiagnostic, semantic};
 mod test;
 
 /// Returns the generic params data of a free function.
-#[salsa::tracked(returns(ref))]
+#[salsa::tracked(returns(ref), cycle_fn=generic_params_data_cycle, cycle_initial=generic_params_data_initial)]
 fn free_function_generic_params_data<'db>(
     db: &'db dyn Database,
     free_function_id: FreeFunctionId<'db>,

crates/cairo-lang-semantic/src/items/generics.rs

@@ -281,6 +281,45 @@ pub struct GenericParamsData<'db> {
     pub resolver_data: Arc<ResolverData<'db>>,
 }
 
+/// Cycle handling for the `*_generic_params_data` queries.
+///
+/// Example cycle, given `struct W<T, +Drop<T>>` and an item-level macro call `E::A!(());` where
+/// variant `E::A`'s type is `W<felt252>`:
+/// `W`'s generic params -> resolving `+Drop<T>` -> the module's items -> expanding `E::A!`
+/// (`priv_macro_call_data`) -> resolving the path `E::A` -> `E`'s variants -> the variant's type
+/// `W<felt252>` -> `W`'s generic params.
+///
+/// Returning the freshly computed `value` lets salsa fixpoint-iterate, so the memoized result is
+/// the real computation and the item's own diagnostics survive. Returning the last provisional
+/// value instead would freeze the query at [generic_params_data_initial] and silently drop them.
+///
+/// The computed value depends only on whether the re-entrant read saw the initial `Err` or a real
+/// value, so the fixpoint is reached within 3 iterations (verified on the goldens via
+/// `Cycle::iteration()`, against salsa's `MAX_ITERATIONS` of 200):
+/// initial `Err` -> value computed against that `Err` -> value computed against a real value ->
+/// unchanged.
+pub fn generic_params_data_cycle<'db, TKey>(
+    _db: &'db dyn Database,
+    _cycle: &salsa::Cycle<'_>,
+    _last_provisional_value: &Maybe<GenericParamsData<'db>>,
+    value: Maybe<GenericParamsData<'db>>,
+    _key: TKey,
+) -> Maybe<GenericParamsData<'db>> {
+    value
+}
+
+/// The provisional value seen by the query that re-enters a `*_generic_params_data` query.
+///
+/// See [generic_params_data_cycle]. `skip_diagnostic` avoids reporting the cycle itself - what
+/// actually went wrong is reported by the queries participating in it.
+pub fn generic_params_data_initial<'db, TKey>(
+    _db: &'db dyn Database,
+    _id: salsa::Id,
+    _key: TKey,
+) -> Maybe<GenericParamsData<'db>> {
+    Err(skip_diagnostic())
+}
+
 /// Query implementation of [GenericsSemantic::generic_impl_param_trait].
 #[salsa::tracked(returns(copy))]
 fn generic_impl_param_trait<'db>(

crates/cairo-lang-semantic/src/items/imp.rs

@@ -50,7 +50,8 @@ use super::functions::{
 };
 use super::generics::{
     GenericArgumentHead, GenericParamImpl, GenericParamsData, displayable_concrete,
-    generic_params_to_args, semantic_generic_params,
+    generic_params_data_cycle, generic_params_data_initial, generic_params_to_args,
+    semantic_generic_params,
 };
 use super::impl_alias::{
     ImplAliasData, impl_alias_generic_params_data_helper, impl_alias_semantic_data_cycle_helper,
@@ -483,7 +484,7 @@ struct ImplDeclarationData<'db> {
 }
 
 /// Returns the generic parameters data of an impl definition.
-#[salsa::tracked(returns(ref))]
+#[salsa::tracked(returns(ref), cycle_fn=generic_params_data_cycle, cycle_initial=generic_params_data_initial)]
 fn impl_def_generic_params_data<'db>(
     db: &'db dyn Database,
     impl_def_id: ImplDefId<'db>,

crates/cairo-lang-semantic/src/items/module_type_alias.rs

@@ -2,11 +2,11 @@ use std::sync::Arc;
 
 use cairo_lang_defs::db::DefsGroup;
 use cairo_lang_defs::ids::{LanguageElementId, LookupItemId, ModuleItemId, ModuleTypeAliasId};
-use cairo_lang_diagnostics::{Diagnostics, Maybe, MaybeAsRef, ToMaybe};
+use cairo_lang_diagnostics::{Diagnostics, Maybe, MaybeAsRef, ToMaybe, skip_diagnostic};
 use cairo_lang_proc_macros::DebugWithDb;
 use salsa::Database;
 
-use super::generics::GenericParamsData;
+use super::generics::{GenericParamsData, generic_params_data_cycle, generic_params_data_initial};
 use super::type_aliases::{
     TypeAliasData, type_alias_generic_params_data_helper, type_alias_semantic_data_cycle_helper,
     type_alias_semantic_data_helper,
@@ -67,13 +67,20 @@ fn module_type_alias_semantic_data_cycle<'db>(
     db: &'db dyn Database,
     _id: salsa::Id,
     module_type_alias_id: ModuleTypeAliasId<'db>,
-    _in_cycle: bool,
+    in_cycle: bool,
 ) -> Maybe<ModuleTypeAliasData<'db>> {
+    if in_cycle {
+        // The `in_cycle` variant reads `module_type_alias_generic_params_data` before it branches
+        // on `in_cycle`, so it can itself be the query the cycle is detected on. Recovering by
+        // calling it again would re-enter this handler forever, so stop here - what went wrong is
+        // reported by the queries participating in the cycle.
+        return Err(skip_diagnostic());
+    }
     module_type_alias_semantic_data(db, module_type_alias_id, true).clone()
 }
 
 /// Returns the generic parameters data of a type alias.
-#[salsa::tracked(returns(ref))]
+#[salsa::tracked(returns(ref), cycle_fn=generic_params_data_cycle, cycle_initial=generic_params_data_initial)]
 fn module_type_alias_generic_params_data<'db>(
     db: &'db dyn Database,
     module_type_alias_id: ModuleTypeAliasId<'db>,
@@ -92,10 +99,17 @@ pub trait ModuleTypeAliasSemantic<'db>: Database {
         &'db self,
         id: ModuleTypeAliasId<'db>,
     ) -> Diagnostics<'db, SemanticDiagnostic<'db>> {
-        module_type_alias_semantic_data(self.as_dyn_database(), id, false)
-            .as_ref()
-            .map(|data| data.diagnostics.clone())
-            .unwrap_or_default()
+        let db = self.as_dyn_database();
+        match module_type_alias_semantic_data(db, id, false) {
+            Ok(data) => data.diagnostics.clone(),
+            // The data computation fails without reporting the generic params diagnostics - e.g.
+            // when recovering from a cycle through the alias's generic params - so fall back to
+            // them, as no other query surfaces them.
+            Err(_) => module_type_alias_generic_params_data(db, id)
+                .as_ref()
+                .map(|data| data.diagnostics.clone())
+                .unwrap_or_default(),
+        }
     }
     /// Returns the resolved type of a type alias.
     fn module_type_alias_resolved_type(

crates/cairo-lang-semantic/src/items/structure.rs

@@ -14,7 +14,10 @@ use cairo_lang_utils::ordered_hash_map::{Entry, OrderedHashMap};
 use salsa::Database;
 
 use super::attribute::SemanticQueryAttrs;
-use super::generics::{GenericParamsData, semantic_generic_params};
+use super::generics::{
+    GenericParamsData, generic_params_data_cycle, generic_params_data_initial,
+    semantic_generic_params,
+};
 use super::visibility::Visibility;
 use crate::diagnostic::SemanticDiagnosticKind::*;
 use crate::diagnostic::{SemanticDiagnostics, SemanticDiagnosticsBuilder};
@@ -72,7 +75,7 @@ fn struct_declaration_data<'db>(
 }
 
 /// Query implementation of [StructSemantic::struct_generic_params_data].
-#[salsa::tracked(returns(ref))]
+#[salsa::tracked(returns(ref), cycle_fn=generic_params_data_cycle, cycle_initial=generic_params_data_initial)]
 fn struct_generic_params_data<'db>(
     db: &'db dyn Database,
     struct_id: StructId<'db>,

crates/cairo-lang-semantic/src/items/test.rs

@@ -8,6 +8,7 @@ cairo_lang_test_utils::test_file_test!(
         extern_func: "extern_func",
         free_function: "free_function",
         impl_alias: "impl_alias",
+        impl_def: "impl_def",
         panicable: "panicable",
         struct_: "struct",
         trait_: "trait",

crates/cairo-lang-semantic/src/items/tests/enum

@@ -229,3 +229,36 @@ error[E2156]: Inline macro `MyEnum::A` not found.
  --> lib.cairo:4:1
 MyEnum::A(());
 ^^^^^^^^^^^^^^
+
+//! > ==========================================================================
+
+//! > Test no ICE when an item-level macro call names an enum variant whose type is a generic enum with a trait bound.
+
+//! > test_runner_name
+test_function_diagnostics(expect_diagnostics: true)
+
+//! > function_code
+fn foo() {}
+
+//! > function_name
+foo
+
+//! > module_code
+enum W<T, +Drop<T>> {
+    X: T,
+}
+enum E {
+    A: W<felt252>,
+}
+E::A!(());
+
+//! > expected_diagnostics
+error[E2028]: Cycle detected while resolving generic param. Try specifying the generic impl parameter explicitly to break the cycle.
+ --> lib.cairo:1:11
+enum W<T, +Drop<T>> {
+          ^^^^^^^^
+
+error[E2156]: Inline macro `E::A` not found.
+ --> lib.cairo:7:1
+E::A!(());
+^^^^^^^^^^

crates/cairo-lang-semantic/src/items/tests/extern_func

@@ -27,3 +27,40 @@ error[E2116]: An extern function must be marked as nopanic.
  _^
 | extern fn bar() -> bad_type;
 |____________________________^
+
+//! > ==========================================================================
+
+//! > Test no ICE when an item-level macro call names an enum variant whose type comes from a generic extern function with a trait bound.
+
+//! > test_runner_name
+test_function_diagnostics(expect_diagnostics: true)
+
+//! > function_code
+fn foo() {}
+
+//! > function_name
+foo
+
+//! > module_code
+#[allow(extern_outside_corelib)]
+extern fn g<T, +Drop<T>>(x: T) nopanic;
+enum E {
+    A: g::<felt252>::Coupon,
+}
+E::A!(());
+
+//! > expected_diagnostics
+error[E2028]: Cycle detected while resolving generic param. Try specifying the generic impl parameter explicitly to break the cycle.
+ --> lib.cairo:2:16
+extern fn g<T, +Drop<T>>(x: T) nopanic;
+               ^^^^^^^^
+
+error[E2165]: Coupon cannot be used with extern functions.
+ --> lib.cairo:4:22
+    A: g::<felt252>::Coupon,
+                     ^^^^^^
+
+error[E2156]: Inline macro `E::A` not found.
+ --> lib.cairo:6:1
+E::A!(());
+^^^^^^^^^^

crates/cairo-lang-semantic/src/items/tests/free_function

@@ -53,3 +53,34 @@ error[E2310]: Failed to infer constant.
  --> lib.cairo:3:5
     bar(a)
     ^^^
+
+//! > ==========================================================================
+
+//! > Test no ICE when an item-level macro call names an enum variant whose type comes from a generic free function with a trait bound.
+
+//! > test_runner_name
+test_function_diagnostics(expect_diagnostics: true)
+
+//! > function_code
+fn foo() {}
+
+//! > function_name
+foo
+
+//! > module_code
+fn g<T, +Drop<T>>(x: T) {}
+enum E {
+    A: g::<felt252>::Coupon,
+}
+E::A!(());
+
+//! > expected_diagnostics
+error[E2028]: Cycle detected while resolving generic param. Try specifying the generic impl parameter explicitly to break the cycle.
+ --> lib.cairo:1:9
+fn g<T, +Drop<T>>(x: T) {}
+        ^^^^^^^^
+
+error[E2156]: Inline macro `E::A` not found.
+ --> lib.cairo:5:1
+E::A!(());
+^^^^^^^^^^

crates/cairo-lang-semantic/src/items/tests/impl_def

@@ -0,0 +1,43 @@
+//! > Test no ICE when an item-level macro call names an enum variant whose type comes from a generic impl with a trait bound.
+
+//! > test_runner_name
+test_function_diagnostics(expect_diagnostics: true)
+
+//! > function_code
+fn foo() {}
+
+//! > function_name
+foo
+
+//! > module_code
+trait Tr<T> {
+    type Ty;
+}
+impl W<T, +Drop<T>> of Tr<T> {
+    type Ty = T;
+}
+enum E {
+    A: Tr::<felt252>::Ty,
+}
+E::A!(());
+
+//! > expected_diagnostics
+error[E2028]: Cycle detected while resolving generic param. Try specifying the generic impl parameter explicitly to break the cycle.
+ --> lib.cairo:4:24
+impl W<T, +Drop<T>> of Tr<T> {
+                       ^^^^^
+
+error[E2028]: Cycle detected while resolving generic param. Try specifying the generic impl parameter explicitly to break the cycle.
+ --> lib.cairo:4:11
+impl W<T, +Drop<T>> of Tr<T> {
+          ^^^^^^^^
+
+error[E2301]: Inference cycle detected
+ --> lib.cairo:8:23
+    A: Tr::<felt252>::Ty,
+                      ^^
+
+error[E2156]: Inline macro `E::A` not found.
+ --> lib.cairo:10:1
+E::A!(());
+^^^^^^^^^^