Skip to content

Commit

Permalink
fix: fix schema dep cycle error. Determine whether there is a cycle d…
Browse files Browse the repository at this point in the history
…epdence from the current node

Signed-off-by: he1pa <[email protected]>
  • Loading branch information
He1pa committed Sep 9, 2024
1 parent c2ff2d3 commit 61a660e
Show file tree
Hide file tree
Showing 5 changed files with 25 additions and 11 deletions.
2 changes: 1 addition & 1 deletion kclvm/sema/src/core/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -959,7 +959,7 @@ impl SymbolData {
.insert(r#ref);
}
SymbolKind::Rule => {
self.attributes
self.rules
.get_mut(def.get_id())
.unwrap()
.r#ref
Expand Down
4 changes: 2 additions & 2 deletions kclvm/sema/src/resolver/global.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {}",
Expand Down Expand Up @@ -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 {}",
Expand Down
11 changes: 8 additions & 3 deletions kclvm/sema/src/resolver/import.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -235,11 +235,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(),
);
Expand Down
4 changes: 2 additions & 2 deletions kclvm/sema/src/resolver/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
15 changes: 12 additions & 3 deletions kclvm/sema/src/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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".
Expand Down

0 comments on commit 61a660e

Please sign in to comment.