From 115892c3ea66b1f2fab0a4c81ad58b15d09e6582 Mon Sep 17 00:00:00 2001
From: He1pa <56333845+He1pa@users.noreply.github.com>
Date: Mon, 9 Sep 2024 20:09:28 +0800
Subject: [PATCH] =?UTF-8?q?fix:=20fix=20schema=20dep=20cycle=20error.=20?=
 =?UTF-8?q?=E2=80=A6=20(#1634)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

fix: fix schema dep cycle error. Determine whether there is a cycle depdence from the current node

Signed-off-by: he1pa <18012015693@163.com>
---
 kclvm/sema/src/core/symbol.rs     |  2 +-
 kclvm/sema/src/resolver/global.rs |  4 ++--
 kclvm/sema/src/resolver/import.rs | 11 ++++++++---
 kclvm/sema/src/resolver/tests.rs  |  4 ++--
 kclvm/sema/src/ty/context.rs      | 15 ++++++++++++---
 5 files changed, 25 insertions(+), 11 deletions(-)

diff --git a/kclvm/sema/src/core/symbol.rs b/kclvm/sema/src/core/symbol.rs
index fcfaa52a7..450618383 100644
--- a/kclvm/sema/src/core/symbol.rs
+++ b/kclvm/sema/src/core/symbol.rs
@@ -959,7 +959,7 @@ impl SymbolData {
                     .insert(r#ref);
             }
             SymbolKind::Rule => {
-                self.attributes
+                self.rules
                     .get_mut(def.get_id())
                     .unwrap()
                     .r#ref
diff --git a/kclvm/sema/src/resolver/global.rs b/kclvm/sema/src/resolver/global.rs
index d4ad2a76b..df470c144 100644
--- a/kclvm/sema/src/resolver/global.rs
+++ b/kclvm/sema/src/resolver/global.rs
@@ -765,7 +765,7 @@ impl<'ctx> Resolver<'ctx> {
                 self.ctx
                     .ty_ctx
                     .add_dependencies(&schema_runtime_ty, &parent_schema_runtime_ty);
-                if self.ctx.ty_ctx.is_cyclic() {
+                if self.ctx.ty_ctx.is_cyclic_from_node(&schema_runtime_ty) {
                     self.handler.add_compile_error(
                         &format!(
                             "There is a circular reference between schema {} and {}",
@@ -877,7 +877,7 @@ impl<'ctx> Resolver<'ctx> {
                 self.ctx
                     .ty_ctx
                     .add_dependencies(&schema_runtime_ty, &parent_schema_runtime_ty);
-                if self.ctx.ty_ctx.is_cyclic() {
+                if self.ctx.ty_ctx.is_cyclic_from_node(&schema_runtime_ty) {
                     self.handler.add_compile_error(
                         &format!(
                             "There is a circular reference between rule {} and {}",
diff --git a/kclvm/sema/src/resolver/import.rs b/kclvm/sema/src/resolver/import.rs
index 5d1ce9831..46d5c22af 100644
--- a/kclvm/sema/src/resolver/import.rs
+++ b/kclvm/sema/src/resolver/import.rs
@@ -7,7 +7,7 @@ use crate::{
     ty::{Type, TypeKind},
 };
 use indexmap::{IndexMap, IndexSet};
-use kclvm_ast::ast;
+use kclvm_ast::{ast, MAIN_PKG};
 use kclvm_error::*;
 use std::rc::Rc;
 use std::sync::Arc;
@@ -236,11 +236,16 @@ impl<'ctx> Resolver<'ctx> {
                             self.ctx
                                 .ty_ctx
                                 .add_dependencies(&self.ctx.pkgpath, &import_stmt.path.node);
-                            if self.ctx.ty_ctx.is_cyclic() {
+                            if self.ctx.ty_ctx.is_cyclic_from_node(&self.ctx.pkgpath) {
+                                let pkg_path = if self.ctx.pkgpath == MAIN_PKG {
+                                    self.ctx.filename.clone()
+                                } else {
+                                    self.ctx.pkgpath.clone()
+                                };
                                 self.handler.add_compile_error(
                                     &format!(
                                         "There is a circular import reference between module {} and {}",
-                                        self.ctx.pkgpath, import_stmt.path.node,
+                                        pkg_path, import_stmt.path.node,
                                     ),
                                     stmt.get_span_pos(),
                                 );
diff --git a/kclvm/sema/src/resolver/tests.rs b/kclvm/sema/src/resolver/tests.rs
index 246571611..da0324aeb 100644
--- a/kclvm/sema/src/resolver/tests.rs
+++ b/kclvm/sema/src/resolver/tests.rs
@@ -238,9 +238,9 @@ fn test_resolve_program_cycle_reference_fail() {
     let scope = resolve_program(&mut program);
     let err_messages = [
         "There is a circular import reference between module file1 and file2",
-        "There is a circular reference between schema SchemaBase and SchemaSub",
+        // "There is a circular reference between schema SchemaBase and SchemaSub",
         "There is a circular reference between schema SchemaSub and SchemaBase",
-        "There is a circular reference between rule RuleBase and RuleSub",
+        // "There is a circular reference between rule RuleBase and RuleSub",
         "There is a circular reference between rule RuleSub and RuleBase",
         "Module 'file2' imported but unused",
         "Module 'file1' imported but unused",
diff --git a/kclvm/sema/src/ty/context.rs b/kclvm/sema/src/ty/context.rs
index 5cfce1514..bea447e71 100644
--- a/kclvm/sema/src/ty/context.rs
+++ b/kclvm/sema/src/ty/context.rs
@@ -4,6 +4,7 @@ use std::sync::Arc;
 use super::{sup, DictType, Type, TypeFlags, TypeKind, TypeRef};
 use petgraph::algo::is_cyclic_directed;
 use petgraph::graph::{DiGraph, NodeIndex};
+use petgraph::visit::{depth_first_search, DfsEvent};
 
 /// TypeContext responsible for type generation, calculation,
 /// and equality and subtype judgment between types.
@@ -49,10 +50,18 @@ impl TypeContext {
         }
     }
 
-    /// Return true if the dep graph contains a cycle.
+    /// Return true if the dep graph contains a cycle from node.
     #[inline]
-    pub fn is_cyclic(&self) -> bool {
-        is_cyclic_directed(&self.dep_graph)
+    pub fn is_cyclic_from_node(&self, node: &String) -> bool {
+        let idx = match self.node_index_map.get(node) {
+            Some(idx) => idx,
+            None => return false,
+        };
+        depth_first_search(&self.dep_graph, vec![idx.clone()], |event| match event {
+            DfsEvent::BackEdge(_, _) => Err(()),
+            _ => Ok(()),
+        })
+        .is_err()
     }
 
     /// Add dependencies between "from" and "to".