Auto merge of #17457 - roife:remove-circle, r=Veykril
fix: ensure there are no cycles in the source_root_parent_map See #17409 We can view the connections between roots as a graph. The problem is that this graph may contain cycles, so when adding edges, it is necessary to check whether it will lead to a cycle. Since we ensure that each node has at most one outgoing edge (because each SourceRoot can have only one parent), we can use a disjoint-set to maintain the connectivity between nodes. If an edge’s two nodes belong to the same set, they are already connected. Additionally, this PR includes the following three changes: 1. Removed the workaround from #17409. 2. Added an optimization: If `map.contains_key(&SourceRootId(*root_id as u32))`, we can skip the current loop iteration since we have already found its parent. 3. Modified the inner loop to iterate in reverse order with `roots[..idx].iter().rev()` at line 319. This ensures that if we are looking for the parent of `a/b/c`, and both `a` and `a/b` meet the criteria, we will choose the longer match (`a/b`).
This commit is contained in:
commit
153a2bab58
2 changed files with 63 additions and 30 deletions
|
@ -285,28 +285,54 @@ impl SourceRootConfig {
|
||||||
/// If a `SourceRoot` doesn't have a parent and is local then it is not contained in this mapping but it can be asserted that it is a root `SourceRoot`.
|
/// If a `SourceRoot` doesn't have a parent and is local then it is not contained in this mapping but it can be asserted that it is a root `SourceRoot`.
|
||||||
pub fn source_root_parent_map(&self) -> FxHashMap<SourceRootId, SourceRootId> {
|
pub fn source_root_parent_map(&self) -> FxHashMap<SourceRootId, SourceRootId> {
|
||||||
let roots = self.fsc.roots();
|
let roots = self.fsc.roots();
|
||||||
roots
|
|
||||||
.iter()
|
let mut map = FxHashMap::default();
|
||||||
.enumerate()
|
|
||||||
.filter(|(_, (_, id))| self.local_filesets.contains(id))
|
// See https://github.com/rust-lang/rust-analyzer/issues/17409
|
||||||
.filter_map(|(idx, (root, root_id))| {
|
//
|
||||||
// We are interested in parents if they are also local source roots.
|
// We can view the connections between roots as a graph. The problem is
|
||||||
// So instead of a non-local parent we may take a local ancestor as a parent to a node.
|
// that this graph may contain cycles, so when adding edges, it is necessary
|
||||||
//
|
// to check whether it will lead to a cycle.
|
||||||
// Here paths in roots are sorted lexicographically, so if a root
|
//
|
||||||
// is a parent of another root, it will be before it in the list.
|
// Since we ensure that each node has at most one outgoing edge (because
|
||||||
roots[..idx].iter().find_map(|(root2, root2_id)| {
|
// each SourceRoot can have only one parent), we can use a disjoint-set to
|
||||||
if self.local_filesets.contains(root2_id)
|
// maintain the connectivity between nodes. If an edge’s two nodes belong
|
||||||
&& root.starts_with(root2)
|
// to the same set, they are already connected.
|
||||||
&& root_id != root2_id
|
let mut dsu = FxHashMap::default();
|
||||||
{
|
fn find_parent(dsu: &mut FxHashMap<u64, u64>, id: u64) -> u64 {
|
||||||
return Some((root_id, root2_id));
|
if let Some(&parent) = dsu.get(&id) {
|
||||||
|
let parent = find_parent(dsu, parent);
|
||||||
|
dsu.insert(id, parent);
|
||||||
|
parent
|
||||||
|
} else {
|
||||||
|
id
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
for (idx, (root, root_id)) in roots.iter().enumerate() {
|
||||||
|
if !self.local_filesets.contains(root_id)
|
||||||
|
|| map.contains_key(&SourceRootId(*root_id as u32))
|
||||||
|
{
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
|
for (root2, root2_id) in roots[..idx].iter().rev() {
|
||||||
|
if self.local_filesets.contains(root2_id)
|
||||||
|
&& root_id != root2_id
|
||||||
|
&& root.starts_with(root2)
|
||||||
|
{
|
||||||
|
// check if the edge will create a cycle
|
||||||
|
if find_parent(&mut dsu, *root_id) != find_parent(&mut dsu, *root2_id) {
|
||||||
|
map.insert(SourceRootId(*root_id as u32), SourceRootId(*root2_id as u32));
|
||||||
|
dsu.insert(*root_id, *root2_id);
|
||||||
}
|
}
|
||||||
None
|
|
||||||
})
|
break;
|
||||||
})
|
}
|
||||||
.map(|(&child, &parent)| (SourceRootId(child as u32), SourceRootId(parent as u32)))
|
}
|
||||||
.collect()
|
}
|
||||||
|
|
||||||
|
map
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -592,4 +618,20 @@ mod tests {
|
||||||
|
|
||||||
assert_eq!(vc, vec![(SourceRootId(1), SourceRootId(0)),])
|
assert_eq!(vc, vec![(SourceRootId(1), SourceRootId(0)),])
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn circular_reference() {
|
||||||
|
let mut builder = FileSetConfigBuilder::default();
|
||||||
|
builder.add_file_set(vec![
|
||||||
|
VfsPath::new_virtual_path("/ROOT/def".to_owned()),
|
||||||
|
VfsPath::new_virtual_path("/ROOT/def/abc/def".to_owned()),
|
||||||
|
]);
|
||||||
|
builder.add_file_set(vec![VfsPath::new_virtual_path("/ROOT/def/abc".to_owned())]);
|
||||||
|
let fsc = builder.build();
|
||||||
|
let src = SourceRootConfig { fsc, local_filesets: vec![0, 1] };
|
||||||
|
let mut vc = src.source_root_parent_map().into_iter().collect::<Vec<_>>();
|
||||||
|
vc.sort_by(|x, y| x.0 .0.cmp(&y.0 .0));
|
||||||
|
|
||||||
|
assert_eq!(vc, vec![(SourceRootId(1), SourceRootId(0)),])
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
|
@ -2531,7 +2531,6 @@ macro_rules! _impl_for_config_data {
|
||||||
#[allow(non_snake_case)]
|
#[allow(non_snake_case)]
|
||||||
$vis fn $field(&self, source_root: Option<SourceRootId>) -> &$ty {
|
$vis fn $field(&self, source_root: Option<SourceRootId>) -> &$ty {
|
||||||
let mut par: Option<SourceRootId> = source_root;
|
let mut par: Option<SourceRootId> = source_root;
|
||||||
let mut traversals = 0;
|
|
||||||
while let Some(source_root_id) = par {
|
while let Some(source_root_id) = par {
|
||||||
par = self.source_root_parent_map.get(&source_root_id).copied();
|
par = self.source_root_parent_map.get(&source_root_id).copied();
|
||||||
if let Some((config, _)) = self.ratoml_files.get(&source_root_id) {
|
if let Some((config, _)) = self.ratoml_files.get(&source_root_id) {
|
||||||
|
@ -2539,14 +2538,6 @@ macro_rules! _impl_for_config_data {
|
||||||
return value;
|
return value;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
// Prevent infinite loops caused by cycles by giving up when it's
|
|
||||||
// clear that we must have either visited all source roots or
|
|
||||||
// encountered a cycle.
|
|
||||||
traversals += 1;
|
|
||||||
if traversals >= self.source_root_parent_map.len() {
|
|
||||||
// i.e. no source root contains the config we're looking for
|
|
||||||
break;
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
if let Some((root_path_ratoml, _)) = self.root_ratoml.as_ref() {
|
if let Some((root_path_ratoml, _)) = self.root_ratoml.as_ref() {
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue