Added destructors in cycle detectors.

crates/cairo-lang-lowering/src/borrow_check/mod.rs

@@ -2,8 +2,11 @@
 #[path = "test.rs"]
 mod test;
 
-use cairo_lang_defs::ids::ModuleFileId;
+use cairo_lang_defs::ids::{ModuleFileId, TraitFunctionId};
 use cairo_lang_diagnostics::{DiagnosticNote, Maybe};
+use cairo_lang_semantic::corelib::get_core_trait;
+use cairo_lang_semantic::items::functions::{GenericFunctionId, ImplGenericFunctionId};
+use cairo_lang_utils::ordered_hash_map::OrderedHashMap;
 use itertools::{zip_eq, Itertools};
 
 use self::analysis::{Analyzer, StatementLocation};
@@ -14,7 +17,7 @@ use crate::borrow_check::analysis::BackAnalysis;
 use crate::db::LoweringGroup;
 use crate::diagnostic::LoweringDiagnosticKind::*;
 use crate::diagnostic::LoweringDiagnostics;
-use crate::ids::LocationId;
+use crate::ids::{FunctionId, LocationId, SemanticFunctionIdEx};
 use crate::{BlockId, FlatLowered, MatchInfo, Statement, VarRemapping, VarUsage, VariableId};
 
 pub mod analysis;
@@ -26,6 +29,9 @@ pub struct BorrowChecker<'a> {
     diagnostics: &'a mut LoweringDiagnostics,
     lowered: &'a FlatLowered,
     success: Maybe<()>,
+    block_extra_calls: OrderedHashMap<BlockId, Vec<FunctionId>>,
+    destruct_fn: TraitFunctionId,
+    panic_destruct_fn: TraitFunctionId,
 }
 
 /// A state saved for each position in the back analysis.
@@ -77,27 +83,49 @@ impl DropPosition {
 impl<'a> DemandReporter<VariableId, PanicState> for BorrowChecker<'a> {
     // Note that for in BorrowChecker `IntroducePosition` is used to pass the cause of
     // the drop.
-    type IntroducePosition = Option<DropPosition>;
+    type IntroducePosition = (Option<DropPosition>, BlockId);
     type UsePosition = LocationId;
 
     fn drop_aux(
         &mut self,
-        opt_drop_position: Option<DropPosition>,
+        (opt_drop_position, block_id): (Option<DropPosition>, BlockId),
         var_id: VariableId,
         panic_state: PanicState,
     ) {
         let var = &self.lowered.variables[var_id];
         let Err(drop_err) = var.droppable.clone() else {
             return;
         };
-        let Err(destruct_err) = var.destruct_impl.clone() else {
-            return;
+        let mut add_called_fn = |impl_id, function| {
+            self.block_extra_calls.entry(block_id).or_default().push(
+                self.db
+                    .intern_function(cairo_lang_semantic::FunctionLongId {
+                        function: cairo_lang_semantic::ConcreteFunction {
+                            generic_function: GenericFunctionId::Impl(ImplGenericFunctionId {
+                                impl_id,
+                                function,
+                            }),
+                            generic_args: vec![],
+                        },
+                    })
+                    .lowered(self.db),
+            );
         };
-        let panic_destruct_err = if matches!(panic_state, PanicState::EndsWithPanic) {
-            let Err(panic_destruct_err) = var.panic_destruct_impl.clone() else {
+        let destruct_err = match var.destruct_impl.clone() {
+            Ok(impl_id) => {
+                add_called_fn(impl_id, self.destruct_fn);
                 return;
-            };
-            Some(panic_destruct_err)
+            }
+            Err(err) => err,
+        };
+        let panic_destruct_err = if matches!(panic_state, PanicState::EndsWithPanic) {
+            match var.panic_destruct_impl.clone() {
+                Ok(impl_id) => {
+                    add_called_fn(impl_id, self.panic_destruct_fn);
+                    return;
+                }
+                Err(err) => Some(err),
+            }
         } else {
             None
         };
@@ -139,10 +167,10 @@ impl<'a> Analyzer<'_> for BorrowChecker<'a> {
     fn visit_stmt(
         &mut self,
         info: &mut Self::Info,
-        _statement_location: StatementLocation,
+        (block_id, _): StatementLocation,
         stmt: &Statement,
     ) {
-        info.variables_introduced(self, stmt.outputs(), None);
+        info.variables_introduced(self, stmt.outputs(), (None, block_id));
         match stmt {
             Statement::Call(stmt) => {
                 if let Ok(signature) = stmt.function.signature(self.db) {
@@ -152,11 +180,9 @@ impl<'a> Analyzer<'_> for BorrowChecker<'a> {
                             aux: PanicState::EndsWithPanic,
                             ..Default::default()
                         };
+                        let location = (Some(DropPosition::Panic(stmt.location)), block_id);
                         *info = BorrowCheckerDemand::merge_demands(
-                            &[
-                                (panic_demand, Some(DropPosition::Panic(stmt.location))),
-                                (info.clone(), Some(DropPosition::Panic(stmt.location))),
-                            ],
+                            &[(panic_demand, location), (info.clone(), location)],
                             self,
                         );
                     }
@@ -198,16 +224,16 @@ impl<'a> Analyzer<'_> for BorrowChecker<'a> {
 
     fn merge_match(
         &mut self,
-        _statement_location: StatementLocation,
+        (block_id, _): StatementLocation,
         match_info: &MatchInfo,
         infos: impl Iterator<Item = Self::Info>,
     ) -> Self::Info {
         let infos: Vec<_> = infos.collect();
         let arm_demands = zip_eq(match_info.arms(), &infos)
             .map(|(arm, demand)| {
                 let mut demand = demand.clone();
-                demand.variables_introduced(self, &arm.var_ids, None);
-                (demand, Some(DropPosition::Diverge(*match_info.location())))
+                demand.variables_introduced(self, &arm.var_ids, (None, block_id));
+                (demand, (Some(DropPosition::Diverge(*match_info.location())), block_id))
             })
             .collect_vec();
         let mut demand = BorrowCheckerDemand::merge_demands(&arm_demands, self);
@@ -243,6 +269,7 @@ impl<'a> Analyzer<'_> for BorrowChecker<'a> {
 }
 
 /// Report borrow checking diagnostics.
+/// Additionally adds the potential destruct function calls per block.
 pub fn borrow_check(
     db: &dyn LoweringGroup,
     module_file_id: ModuleFileId,
@@ -252,16 +279,38 @@ pub fn borrow_check(
     diagnostics.diagnostics.extend(std::mem::take(&mut lowered.diagnostics));
 
     if lowered.blocks.has_root().is_ok() {
-        let checker = BorrowChecker { db, diagnostics: &mut diagnostics, lowered, success: Ok(()) };
+        let destruct_trait_id = get_core_trait(db.upcast(), "Destruct".into());
+        let destruct_fn =
+            db.trait_function_by_name(destruct_trait_id, "destruct".into()).unwrap().unwrap();
+        let panic_destruct_trait_id = get_core_trait(db.upcast(), "PanicDestruct".into());
+        let panic_destruct_fn = db
+            .trait_function_by_name(panic_destruct_trait_id, "panic_destruct".into())
+            .unwrap()
+            .unwrap();
+        let checker = BorrowChecker {
+            db,
+            diagnostics: &mut diagnostics,
+            lowered,
+            success: Ok(()),
+            block_extra_calls: Default::default(),
+            destruct_fn,
+            panic_destruct_fn,
+        };
         let mut analysis = BackAnalysis::new(lowered, checker);
         let mut root_demand = analysis.get_root_info();
-        root_demand.variables_introduced(&mut analysis.analyzer, &lowered.parameters, None);
+        root_demand.variables_introduced(
+            &mut analysis.analyzer,
+            &lowered.parameters,
+            (None, BlockId::root()),
+        );
+        let block_extra_calls = analysis.analyzer.block_extra_calls;
         let success = analysis.analyzer.success;
         assert!(root_demand.finalize(), "Undefined variable should not happen at this stage");
 
         if let Err(diag_added) = success {
             lowered.blocks = Blocks::new_errored(diag_added);
         }
+        lowered.block_extra_calls = block_extra_calls;
     }
 
     lowered.diagnostics = diagnostics.build();

crates/cairo-lang-lowering/src/db.rs

@@ -478,6 +478,9 @@ pub(crate) fn get_direct_callees(
                 }
             }
         }
+        if let Some(extra_calls) = lowered_function.block_extra_calls.get(&block_id) {
+            direct_callees.extend(extra_calls.iter().copied());
+        }
         match &block.end {
             FlatBlockEnd::NotSet | FlatBlockEnd::Return(..) | FlatBlockEnd::Panic(_) => {}
             FlatBlockEnd::Goto(next, _) => stack.push(*next),

crates/cairo-lang-lowering/src/inline/mod.rs

@@ -282,6 +282,7 @@ impl<'db> FunctionInlinerRewriter<'db> {
             blocks,
             parameters: flat_lower.parameters.clone(),
             signature: flat_lower.signature.clone(),
+            block_extra_calls: Default::default(),
         })
     }
 

crates/cairo-lang-lowering/src/lower/mod.rs

@@ -156,6 +156,7 @@ pub fn lower_function(
         blocks,
         signature: ctx.signature.clone(),
         parameters,
+        block_extra_calls: Default::default(),
     })
 }
 
@@ -376,6 +377,7 @@ pub fn lower_loop_function(
         blocks,
         signature: ctx.signature.clone(),
         parameters,
+        block_extra_calls: Default::default(),
     })
 }
 

crates/cairo-lang-lowering/src/objects.rs

@@ -123,6 +123,9 @@ pub struct FlatLowered {
     pub blocks: FlatBlocks,
     /// function parameters, including implicits.
     pub parameters: Vec<VariableId>,
+    /// Extra calls for block to functions considered as called from it.
+    /// Corresponds to `Destruct::destruct` and `PanicDestruct::panic_destruct` calls.
+    pub block_extra_calls: OrderedHashMap<BlockId, Vec<FunctionId>>,
 }
 
 /// Remapping of lowered variable ids. Useful for convergence of branches.

crates/cairo-lang-lowering/src/panic/mod.rs

@@ -43,6 +43,7 @@ pub fn lower_panics(
             blocks: lowered.blocks.clone(),
             parameters: lowered.parameters.clone(),
             signature: lowered.signature.clone(),
+            block_extra_calls: Default::default(),
         });
     }
 
@@ -66,6 +67,7 @@ pub fn lower_panics(
         blocks: ctx.flat_blocks.build().unwrap(),
         parameters: lowered.parameters.clone(),
         signature: lowered.signature.clone(),
+        block_extra_calls: Default::default(),
     })
 }
 

crates/cairo-lang-lowering/src/test_data/cycles

@@ -137,3 +137,53 @@ blk2:
 Statements:
 End:
   Return(v6, v7)
+
+//! > ==========================================================================
+
+//! > Test destructor basic cycle.
+
+//! > test_runner_name
+test_function_lowering
+
+//! > function
+fn foo() {}
+
+//! > function_name
+foo
+
+//! > module_code
+struct A {}
+impl ADestruct of Destruct<A> {
+    fn destruct(self: A) nopanic {
+        let A { } = self;
+        B {};
+    }
+}
+
+struct B {}
+impl BDestruct of Destruct<B> {
+    fn destruct(self: B) nopanic {
+        let B { } = self;
+        A {};
+    }
+}
+
+//! > semantic_diagnostics
+
+//! > lowering_diagnostics
+error: Call cycle of `nopanic` functions is not allowed.
+ --> lib.cairo:3:5
+    fn destruct(self: A) nopanic {
+    ^****************************^
+
+error: Call cycle of `nopanic` functions is not allowed.
+ --> lib.cairo:11:5
+    fn destruct(self: B) nopanic {
+    ^****************************^
+
+//! > lowering_flat
+Parameters:
+blk0 (root):
+Statements:
+End:
+  Return()

crates/cairo-lang-lowering/src/test_data/destruct

@@ -22,6 +22,7 @@ struct A {}
 impl ADestruct of Destruct<A> {
     #[inline(never)]
     fn destruct(self: A) nopanic {
+        let A { } = self;
         // Use RangeCheck, a previously unused implicit.
         match u128_overflowing_add(1_u128, 2_u128) {
             Result::Ok(v) => v,
@@ -356,12 +357,24 @@ impl ADestruct of Destruct<A> {
 //! > semantic_diagnostics
 
 //! > lowering_diagnostics
+error: Call cycle of `nopanic` functions is not allowed.
+ --> lib.cairo:3:5
+    #[inline(always)]
+    ^***************^
+
+error: Cannot inline a function that might call itself.
+ --> lib.cairo:3:5
+    #[inline(always)]
+    ^***************^
 
 //! > lowering_flat
-Parameters:
+Parameters: v0: core::RangeCheck, v1: core::gas::GasBuiltin
 blk0 (root):
 Statements:
-  (v0: test::A) <- struct_construct()
-  () <- test::ADestruct::destruct(v0)
+  (v2: test::A) <- struct_construct()
+  (v3: core::RangeCheck, v4: core::gas::GasBuiltin) <- test::ADestruct::destruct(v0, v1, v2)
+  (v5: ()) <- struct_construct()
+  (v6: ((),)) <- struct_construct(v5)
+  (v7: core::panics::PanicResult::<((),)>) <- PanicResult::Ok(v6)
 End:
-  Return()
+  Return(v3, v4, v7)