Skip to content

Commit

Permalink
Add no item dep cycle check.
Browse files Browse the repository at this point in the history
- Also add ItemKind::is_typedef().
  • Loading branch information
maloneymr committed Jul 31, 2024
1 parent 3f70fe0 commit a72cddc
Show file tree
Hide file tree
Showing 7 changed files with 126 additions and 3 deletions.
7 changes: 7 additions & 0 deletions virdant/examples/errors/item_dep_cycle.vir
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
mod Foo {
mod bar of Bar;
}

mod Bar {
mod foo of Foo;
}
44 changes: 44 additions & 0 deletions virdant/src/cycle.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
use std::hash::Hash;
use indexmap::{IndexMap, IndexSet};

pub fn detect_cycle<T: Eq + Hash + Clone>(graph: &IndexMap<T, Vec<T>>) -> Result<(), Vec<T>> {
let mut visited = IndexSet::new();
let mut recursion_stack = Vec::new();

for node in graph.keys() {
if !visited.contains(node) {
if let Some(cycle) = dfs(graph, node, &mut visited, &mut recursion_stack) {
return Err(cycle);
}
}
}
Ok(())
}

fn dfs<T: Clone + Eq + Hash>(
graph: &IndexMap<T, Vec<T>>,
node: &T,
visited: &mut IndexSet<T>,
recursion_stack: &mut Vec<T>,
) -> Option<Vec<T>> {
visited.insert(node.clone());
recursion_stack.push(node.clone());

if let Some(neighbors) = graph.get(node) {
for neighbor in neighbors {
if !visited.contains(neighbor) {
if let Some(cycle) = dfs(graph, neighbor, visited, recursion_stack) {
return Some(cycle);
}
} else if recursion_stack.contains(neighbor) {
// Found a cycle, extract the cycle path
let cycle_start_index = recursion_stack.iter().position(|x| x == neighbor).unwrap();
let cycle = recursion_stack[cycle_start_index..].to_vec();
return Some(cycle);
}
}
}

recursion_stack.pop();
None
}
1 change: 1 addition & 0 deletions virdant/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ pub enum VirErr {
CantImport(String),
DupImport(String),
UnresolvedIdent(String),
ItemDepCycle(Vec<String>),
Other(String),
}

Expand Down
2 changes: 1 addition & 1 deletion virdant/src/id.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ impl<T> std::fmt::Debug for Id<T> {
pub mod types {
macro_rules! id_type {
($name:ident) => {
#[derive(Clone, Copy, Eq, PartialEq, Hash)]
#[derive(Clone, Copy, Eq, PartialEq, Hash, Debug)]
pub struct $name(());
};
}
Expand Down
38 changes: 36 additions & 2 deletions virdant/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,12 @@ pub mod id;

mod table;
mod ready;
mod cycle;

#[cfg(test)]
mod tests;

use cycle::detect_cycle;
use indexmap::IndexMap;
use indexmap::IndexSet;
use parse::QualIdent;
Expand All @@ -30,14 +32,14 @@ pub struct Virdant<'a> {
items: Table<Item, ItemInfo<'a>>,
}

#[derive(Default, Clone)]
#[derive(Default, Clone, Debug)]
struct PackageInfo<'a> {
name: String,
source: std::path::PathBuf,
ast: Ready<Ast<'a>>,
}

#[derive(Default, Clone)]
#[derive(Default, Clone, Debug)]
struct ItemInfo<'a> {
name: String,
package: Ready<Id<Package>>,
Expand All @@ -46,6 +48,7 @@ struct ItemInfo<'a> {
deps: Ready<Vec<Id<Item>>>,
}


////////////////////////////////////////////////////////////////////////////////
// Public Virdant<'a> API
////////////////////////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -92,6 +95,8 @@ impl<'a> Virdant<'a> {
item_info.deps.set(item_deps);
}

self.check_no_item_dep_cycles();

self.errors.clone().check()
}
}
Expand Down Expand Up @@ -248,6 +253,23 @@ impl<'a> Virdant<'a> {
}
}
}

fn check_no_item_dep_cycles(&mut self) {
let mut dep_graph = IndexMap::new();

for (item, item_info) in self.items.iter() {
let deps = item_info.deps.unwrap();
dep_graph.insert(item.clone(), deps.to_owned());
}

if let Err(cycle) = detect_cycle(&dep_graph) {
let cycle_names: Vec<String> = cycle
.into_iter()
.map(|item| item.to_string())
.collect();
self.errors.add(VirErr::ItemDepCycle(cycle_names));
}
}
}

////////////////////////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -357,3 +379,15 @@ pub enum ItemKind {
BuiltinDef,
PortDef,
}

impl ItemKind {
pub fn is_typedef(&self) -> bool {
match self {
ItemKind::ModDef => false,
ItemKind::UnionDef => true,
ItemKind::StructDef => true,
ItemKind::BuiltinDef => true,
ItemKind::PortDef => false,
}
}
}
1 change: 1 addition & 0 deletions virdant/src/table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use indexmap::IndexMap;
use std::hash::Hash;
use crate::id::Id;

#[derive(Debug)]
pub struct Table<E, D>(IndexMap<Id<E>, D>);

impl<E: Copy + Eq + Hash, D: Default> Default for Table<E, D> {
Expand Down
36 changes: 36 additions & 0 deletions virdant/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,19 @@ fn test_example_files() -> impl Iterator<Item = std::path::PathBuf> {
results.into_iter()
}

#[test]
fn test_top() {
let examples_dir = std::path::Path::new(EXAMPLES_DIR);
let test_examples_dir = std::path::Path::new(TEST_EXAMPLES_DIR);

let mut virdant = Virdant::new(&[
("builtin", examples_dir.join("builtin.vir")),
("top", test_examples_dir.join("top.vir")),
]);

virdant.check().unwrap();
}

#[test]
fn test_check_syntax_error() {
let error_examples_dir = std::path::Path::new(ERROR_EXAMPLES_DIR);
Expand Down Expand Up @@ -187,3 +200,26 @@ fn test_check_missing_dependency() {
_ => panic!(),
}
}

#[test]
fn test_check_item_dep_cycle() {
let examples_dir = std::path::Path::new(EXAMPLES_DIR);
let error_examples_dir = std::path::Path::new(ERROR_EXAMPLES_DIR);
let mut virdant = Virdant::new(&[
("builtin", examples_dir.join("builtin.vir")),
("top", error_examples_dir.join("item_dep_cycle.vir")),
]);

match virdant.check() {
Err(errors) => {
eprintln!("{errors:?}");
assert_eq!(errors.len(), 1);
if let VirErr::ItemDepCycle(_) = &errors[0] {
()
} else {
panic!()
}
},
_ => panic!(),
}
}

0 comments on commit a72cddc

Please sign in to comment.