Skip to content

Commit

Permalink
v2
Browse files Browse the repository at this point in the history
  • Loading branch information
@alibektas
alibektas committed Nov 8, 2023
1 parent d7b72ce commit 1d268fc
Show file tree
Hide file tree
Showing 6 changed files with 282 additions and 238 deletions.
52 changes: 22 additions & 30 deletions crates/base-db/src/input.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -329,12 +329,10 @@ pub struct CrateData {
}

impl CrateData {
/**
Check if [`other`] is almost equal to [`self`].
This method has some obscure bits. These are mostly there to be compliant with
some patches. References to the patches are given.
*/
pub fn almost_eq(&self, other: &CrateData) -> bool {
/// Check if [`other`] is almost equal to [`self`] ignoring `CrateOrigin` value.
pub fn eq_ignoring_origin(&self, other: &CrateData) -> bool {
// This method has some obscure bits. These are mostly there to be compliant with
// some patches. References to the patches are given.
if self.root_file_id != other.root_file_id {
return false;
}
Expand All @@ -356,16 +354,16 @@ impl CrateData {
}

let mut opts = self.cfg_options.clone();
opts.apply_diff(CfgDiff {
disable: other.cfg_options.clone().into_iter().collect(),
enable: vec![],
});
opts.apply_diff(
CfgDiff::new(vec![], other.cfg_options.clone().into_iter().collect())
.expect("CfgOptions were expected to contain no duplicates."),
);

let mut cfgs = opts.into_iter();
if let Some(cfg) = cfgs.next() {
// Don't care if rust_analyzer CfgAtom is the only cfg in the difference set of self's and other's cfgs.
// https://github.com/rust-lang/rust-analyzer/blob/0840038f02daec6ba3238f05d8caa037d28701a0/crates/project-model/src/workspace.rs#L894
if !cfgs.next().is_none() || cfg.to_string() != "rust_analyzer" {
if cfgs.next().is_some() || cfg.to_string() != "rust_analyzer" {
return false;
}
}
Expand Down Expand Up @@ -686,41 +684,35 @@ impl CrateGraph {
/// Note that for deduplication to fully work, `self`'s crate dependencies must be sorted by crate id.
/// If the crate dependencies were sorted, the resulting graph from this `extend` call will also have the crate dependencies sorted.
pub fn extend(&mut self, mut other: CrateGraph, proc_macros: &mut ProcMacroPaths) {
enum ExtendStrategy {
Dedup(CrateId),
Replace(CrateId),
}

let topo = other.crates_in_topological_order();
let mut id_map: FxHashMap<CrateId, CrateId> = FxHashMap::default();
for topo in topo {
let crate_data = &mut other.arena[topo];

crate_data.dependencies.iter_mut().for_each(|dep| {
dep.crate_id = id_map[&dep.crate_id];
});
crate_data.dependencies.iter_mut().for_each(|dep| dep.crate_id = id_map[&dep.crate_id]);
crate_data.dependencies.sort_by_key(|dep| dep.crate_id);
let res = self.arena.iter().find_map(|(id, data)| {
if data.almost_eq(crate_data) {
if data.eq_ignoring_origin(crate_data) {
if data.origin.is_lib() && crate_data.origin.is_local() {
// See #15656 for a relevant example.
return Some(ExtendStrategy::Replace(id));
return Some((id, true));
}

return Some(ExtendStrategy::Dedup(id));
return Some((id, false));
}
None
});

if let Some(res) = res {
match res {
ExtendStrategy::Dedup(res) => id_map.insert(topo, res),
ExtendStrategy::Replace(res) => {
let id = self.arena.alloc(crate_data.clone());
let _ = self.remove_and_replace(res, id);
id_map.insert(topo, id)
if let Some((res, should_update_lib_to_local)) = res {
id_map.insert(topo, res);
if should_update_lib_to_local {
let origin_old = self.arena[res].origin.clone();
assert!(origin_old.is_lib());

if let CrateOrigin::Library { repo, name } = origin_old {
self.arena[res].origin = CrateOrigin::Local { repo, name: Some(name) };
}
};
}
} else {
let id = self.arena.alloc(crate_data.clone());
id_map.insert(topo, id);
Expand Down
4 changes: 2 additions & 2 deletions crates/cfg/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,8 +115,8 @@ impl<'a> IntoIterator for &'a CfgOptions {
#[derive(Default, Clone, Debug, PartialEq, Eq)]
pub struct CfgDiff {
// Invariants: No duplicates, no atom that's both in `enable` and `disable`.
pub enable: Vec<CfgAtom>,
pub disable: Vec<CfgAtom>,
enable: Vec<CfgAtom>,
disable: Vec<CfgAtom>,
}

impl CfgDiff {
Expand Down
50 changes: 50 additions & 0 deletions crates/project-model/src/tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -249,3 +249,53 @@ fn crate_graph_dedup() {
crate_graph.extend(regex_crate_graph, &mut regex_proc_macros);
assert_eq!(crate_graph.iter().count(), 118);
}

#[test]
fn test_deduplicate_crate_differing_in_origin() {
let path_map = &mut Default::default();
let (mut crate_graph, _proc_macros) =
load_cargo_with_sysroot(path_map, "deduplication_crate_graph_A.json");
crate_graph.sort_deps();
let (crate_graph_1, mut _proc_macros_2) =
load_cargo_with_sysroot(path_map, "deduplication_crate_graph_B.json");

crate_graph.extend(crate_graph_1, &mut _proc_macros_2);

let mut crates_named_p1 = vec![];
for id in crate_graph.iter() {
let krate = &crate_graph[id];
if let Some(name) = krate.display_name.as_ref() {
if name.to_string() == "p1" {
crates_named_p1.push(krate);
}
}
}

assert!(crates_named_p1.len() == 1);
assert!(crates_named_p1[0].origin.is_local());
}

#[test]
fn test_deduplicate_crate_differing_in_origin_in_rev_resolution_order() {
let path_map = &mut Default::default();
let (mut crate_graph, _proc_macros) =
load_cargo_with_sysroot(path_map, "deduplication_crate_graph_B.json");
crate_graph.sort_deps();
let (crate_graph_1, mut _proc_macros_2) =
load_cargo_with_sysroot(path_map, "deduplication_crate_graph_A.json");

crate_graph.extend(crate_graph_1, &mut _proc_macros_2);

let mut crates_named_p1 = vec![];
for id in crate_graph.iter() {
let krate = &crate_graph[id];
if let Some(name) = krate.display_name.as_ref() {
if name.to_string() == "p1" {
crates_named_p1.push(krate);
}
}
}

assert!(crates_named_p1.len() == 1);
assert!(crates_named_p1[0].origin.is_local());
}
66 changes: 66 additions & 0 deletions crates/project-model/test_data/deduplication_crate_graph_A.json
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
{
"packages": [
{
"name": "p1",
"version": "0.1.0",
"id": "p1 0.1.0 (path+file:///path/to/project/projects/p1)",
"license": null,
"license_file": null,
"description": null,
"source": null,
"dependencies": [],
"targets": [
{
"kind": [
"lib"
],
"crate_types": [
"lib"
],
"name": "p1",
"src_path": "/path/to/project/projects/p1/src/lib.rs",
"edition": "2021",
"doc": true,
"doctest": true,
"test": true
}
],
"features": {},
"manifest_path": "/path/to/project/projects/p1/Cargo.toml",
"metadata": null,
"publish": null,
"authors": [],
"categories": [],
"keywords": [],
"readme": null,
"repository": null,
"homepage": null,
"documentation": null,
"edition": "2021",
"links": null,
"default_run": null,
"rust_version": null
}
],
"workspace_members": [
"p1 0.1.0 (path+file:///path/to/project/projects/p1)"
],
"workspace_default_members": [
"p1 0.1.0 (path+file:///path/to/project/projects/p1)"
],
"resolve": {
"nodes": [
{
"id": "p1 0.1.0 (path+file:///path/to/project/projects/p1)",
"dependencies": [],
"deps": [],
"features": []
}
],
"root": "p1 0.1.0 (path+file:///path/to/project/projects/p1)"
},
"target_directory": "/path/to/project/projects/p1/target",
"version": 1,
"workspace_root": "/path/to/project/projects/p1",
"metadata": null
}
140 changes: 140 additions & 0 deletions crates/project-model/test_data/deduplication_crate_graph_B.json
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
{
"packages": [
{
"name": "p1",
"version": "0.1.0",
"id": "p1 0.1.0 (path+file:///path/to/project/projects/p1)",
"license": null,
"license_file": null,
"description": null,
"source": null,
"dependencies": [],
"targets": [
{
"kind": [
"lib"
],
"crate_types": [
"lib"
],
"name": "p1",
"src_path": "/path/to/project/projects/p1/src/lib.rs",
"edition": "2021",
"doc": true,
"doctest": true,
"test": true
}
],
"features": {},
"manifest_path": "/path/to/project/projects/p1/Cargo.toml",
"metadata": null,
"publish": null,
"authors": [],
"categories": [],
"keywords": [],
"readme": null,
"repository": null,
"homepage": null,
"documentation": null,
"edition": "2021",
"links": null,
"default_run": null,
"rust_version": null
},
{
"name": "p2",
"version": "0.1.0",
"id": "p2 0.1.0 (path+file:///path/to/project/projects/p2)",
"license": null,
"license_file": null,
"description": null,
"source": null,
"dependencies": [
{
"name": "p1",
"source": null,
"req": "*",
"kind": null,
"rename": null,
"optional": false,
"uses_default_features": true,
"features": [],
"target": null,
"registry": null,
"path": "/path/to/project/projects/p1"
}
],
"targets": [
{
"kind": [
"lib"
],
"crate_types": [
"lib"
],
"name": "p2",
"src_path": "/path/to/project/projects/p2/src/lib.rs",
"edition": "2021",
"doc": true,
"doctest": true,
"test": true
}
],
"features": {},
"manifest_path": "/path/to/project/projects/p2/Cargo.toml",
"metadata": null,
"publish": null,
"authors": [],
"categories": [],
"keywords": [],
"readme": null,
"repository": null,
"homepage": null,
"documentation": null,
"edition": "2021",
"links": null,
"default_run": null,
"rust_version": null
}
],
"workspace_members": [
"p2 0.1.0 (path+file:///path/to/project/projects/p2)"
],
"workspace_default_members": [
"p2 0.1.0 (path+file:///path/to/project/projects/p2)"
],
"resolve": {
"nodes": [
{
"id": "p1 0.1.0 (path+file:///path/to/project/projects/p1)",
"dependencies": [],
"deps": [],
"features": []
},
{
"id": "p2 0.1.0 (path+file:///path/to/project/projects/p2)",
"dependencies": [
"p1 0.1.0 (path+file:///path/to/project/projects/p1)"
],
"deps": [
{
"name": "p1",
"pkg": "p1 0.1.0 (path+file:///path/to/project/projects/p1)",
"dep_kinds": [
{
"kind": null,
"target": null
}
]
}
],
"features": []
}
],
"root": "p2 0.1.0 (path+file:///path/to/project/projects/p2)"
},
"target_directory": "/path/to/project/projects/p2/target",
"version": 1,
"workspace_root": "/path/to/project/projects/p2",
"metadata": null
}
Loading

0 comments on commit 1d268fc

Please sign in to comment.