When constructing a crate graph, detect and forbid cycles.

fixed #300
This commit is contained in:
gfreezy 2018-12-21 22:27:04 +08:00
parent 463e5af3f2
commit 792dabc0a6

View file

@ -7,11 +7,12 @@
/// actual IO is done and lowered to input. /// actual IO is done and lowered to input.
use std::sync::Arc; use std::sync::Arc;
use rustc_hash::{FxHashMap};
use relative_path::RelativePathBuf; use relative_path::RelativePathBuf;
use ra_syntax::SmolStr; use rustc_hash::FxHashMap;
use salsa; use salsa;
use ra_syntax::SmolStr;
/// `FileId` is an integer which uniquely identifies a file. File paths are /// `FileId` is an integer which uniquely identifies a file. File paths are
/// messy and system-dependent, so most of the code should work directly with /// messy and system-dependent, so most of the code should work directly with
/// `FileId`, without inspecting the path. The mapping between `FileId` and path /// `FileId`, without inspecting the path. The mapping between `FileId` and path
@ -92,10 +93,10 @@ impl CrateGraph {
assert!(prev.is_none()); assert!(prev.is_none());
crate_id crate_id
} }
// FIXME: check that we don't have cycles here.
// Just a simple depth first search from `to` should work,
// the graph is small.
pub fn add_dep(&mut self, from: CrateId, name: SmolStr, to: CrateId) { pub fn add_dep(&mut self, from: CrateId, name: SmolStr, to: CrateId) {
if self.dfs(from, to) {
panic!("Cycle dependencies found.")
}
self.arena.get_mut(&from).unwrap().add_dep(name, to) self.arena.get_mut(&from).unwrap().add_dep(name, to)
} }
pub fn crate_root(&self, crate_id: CrateId) -> FileId { pub fn crate_root(&self, crate_id: CrateId) -> FileId {
@ -114,7 +115,52 @@ impl CrateGraph {
) -> impl Iterator<Item=&'a Dependency> + 'a { ) -> impl Iterator<Item=&'a Dependency> + 'a {
self.arena[&crate_id].dependencies.iter() self.arena[&crate_id].dependencies.iter()
} }
fn dfs(&self, target: CrateId, from: CrateId) -> bool {
for dep in self.dependencies(from) {
let crate_id = dep.crate_id();
if crate_id == target {
return true;
} }
if self.arena.contains_key(&crate_id) {
if self.dfs(target, crate_id) {
return true;
}
}
}
return false;
}
}
mod test {
use super::{CrateGraph, FxHashMap, FileId, SmolStr};
#[test]
#[should_panic]
fn it_should_painc_because_of_cycle_dependencies() {
let mut graph = CrateGraph {
arena: FxHashMap::default()
};
let crate1 = graph.add_crate_root(FileId(1u32));
let crate2 = graph.add_crate_root(FileId(2u32));
let crate3 = graph.add_crate_root(FileId(3u32));
graph.add_dep(crate1, SmolStr::new("crate2"), crate2);
graph.add_dep(crate2, SmolStr::new("crate3"), crate3);
graph.add_dep(crate3, SmolStr::new("crate1"), crate1);
}
#[test]
fn it_works() {
let mut graph = CrateGraph {
arena: FxHashMap::default()
};
let crate1 = graph.add_crate_root(FileId(1u32));
let crate2 = graph.add_crate_root(FileId(2u32));
let crate3 = graph.add_crate_root(FileId(3u32));
graph.add_dep(crate1, SmolStr::new("crate2"), crate2);
graph.add_dep(crate2, SmolStr::new("crate3"), crate3);
}
}
salsa::query_group! { salsa::query_group! {
pub trait FilesDatabase: salsa::Database { pub trait FilesDatabase: salsa::Database {