Skip to content

Commit

Permalink
Auto merge of #17381 - roife:fix-issue-17378, r=Veykril
Browse files Browse the repository at this point in the history
fix: ensure that the parent of a SourceRoot cannot be itself

fix #17378.

In `FileSetConfig.map`, different roots might be mapped to the same `root_id` due to deduplication in `ProjectFolders::new`:

```rust
// Example from rustup
/Users/roife/code/rustup/target/debug/build/rustup-863a063426b56c51/out
/Users/roife/code/rustup
```

In `source_root_parent_map`, r-a might encounter paths where their SourceRootId (i.e. `root_id`) is identical, yet one the them is the parent of the another. This situation can cause the `root_id` to be its own parent, potentially leading to an infinite loop.

This PR resolves such cases by adding a check.
  • Loading branch information
bors committed Jun 10, 2024
2 parents 14a1f45 + 733995c commit c86d623
Showing 1 changed file with 23 additions and 3 deletions.
26 changes: 23 additions & 3 deletions crates/load-cargo/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -285,17 +285,21 @@ 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`.
pub fn source_root_parent_map(&self) -> FxHashMap<SourceRootId, SourceRootId> {
let roots = self.fsc.roots();
let mut i = 0;
roots
.iter()
.enumerate()
.filter(|(_, (_, id))| self.local_filesets.contains(id))
.filter_map(|(idx, (root, root_id))| {
// We are interested in parents if they are also local source roots.
// So instead of a non-local parent we may take a local ancestor as a parent to a node.
//
// 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.
roots[..idx].iter().find_map(|(root2, root2_id)| {
i += 1;
if self.local_filesets.contains(root2_id) && root.starts_with(root2) {
if self.local_filesets.contains(root2_id)
&& root.starts_with(root2)
&& root_id != root2_id
{
return Some((root_id, root2_id));
}
None
Expand Down Expand Up @@ -572,4 +576,20 @@ mod tests {

assert_eq!(vc, vec![(SourceRootId(3), SourceRootId(1)),])
}

#[test]
fn parents_with_identical_root_id() {
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/def/ghi".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)),])
}
}

0 comments on commit c86d623

Please sign in to comment.