Skip to content

Commit

Permalink
Relaxation for crate graph mergin
Browse files Browse the repository at this point in the history 
Partially fixes rust-lang#15656 . When a crate graph is extended which is the case when new workspaces are added to the project
the rules for deduplication were too strict. One problem that arises from this is that in certain conditions
when we see the same crate having different `CrateOrigin`s the first form would be maintained. This approach however
results in some unwanted results such as making renaming forbidden as this has been recently only made available for
local crates. The given example in rust-lang#15656 can still not be resolved with this PR as that involves taking inconsistencies
between dependencies into consideration. This will be addressed in a future PR.
  • Loading branch information
@alibektas
alibektas committed Nov 8, 2023
1 parent 7059ae2 commit d7b72ce
Show file tree
Hide file tree
Showing 11 changed files with 515 additions and 54 deletions.
28 changes: 22 additions & 6 deletions crates/base-db/src/fixture.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ use vfs::{file_set::FileSet, VfsPath};

use crate::{
input::{CrateName, CrateOrigin, LangCrateOrigin},
Change, CrateDisplayName, CrateGraph, CrateId, Dependency, Edition, Env, FileId, FilePosition,
FileRange, ProcMacro, ProcMacroExpander, ProcMacroExpansionError, ProcMacros, ReleaseChannel,
SourceDatabaseExt, SourceRoot, SourceRootId,
Change, CrateDisplayName, CrateGraph, CrateId, Dependency, DependencyKind, Edition, Env,
FileId, FilePosition, FileRange, ProcMacro, ProcMacroExpander, ProcMacroExpansionError,
ProcMacros, ReleaseChannel, SourceDatabaseExt, SourceRoot, SourceRootId,
};

pub const WORKSPACE: SourceRootId = SourceRootId(0);
Expand Down Expand Up @@ -237,7 +237,12 @@ impl ChangeFixture {
crate_graph
.add_dep(
from_id,
Dependency::with_prelude(CrateName::new(&to).unwrap(), to_id, prelude),
Dependency::with_prelude(
CrateName::new(&to).unwrap(),
to_id,
prelude,
DependencyKind::Normal,
),
)
.unwrap();
}
Expand Down Expand Up @@ -275,7 +280,14 @@ impl ChangeFixture {

for krate in all_crates {
crate_graph
.add_dep(krate, Dependency::new(CrateName::new("core").unwrap(), core_crate))
.add_dep(
krate,
Dependency::new(
CrateName::new("core").unwrap(),
core_crate,
DependencyKind::Normal,
),
)
.unwrap();
}
}
Expand Down Expand Up @@ -317,7 +329,11 @@ impl ChangeFixture {
crate_graph
.add_dep(
krate,
Dependency::new(CrateName::new("proc_macros").unwrap(), proc_macros_crate),
Dependency::new(
CrateName::new("proc_macros").unwrap(),
proc_macros_crate,
DependencyKind::Normal,
),
)
.unwrap();
}
Expand Down
221 changes: 191 additions & 30 deletions crates/base-db/src/input.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

use std::{fmt, mem, ops, panic::RefUnwindSafe, str::FromStr, sync};

use cfg::CfgOptions;
use cfg::{CfgDiff, CfgOptions};
use la_arena::{Arena, Idx};
use rustc_hash::{FxHashMap, FxHashSet};
use syntax::SmolStr;
Expand Down Expand Up @@ -155,6 +155,10 @@ impl CrateOrigin {
pub fn is_local(&self) -> bool {
matches!(self, CrateOrigin::Local { .. })
}

pub fn is_lib(&self) -> bool {
matches!(self, CrateOrigin::Library { .. })
}
}

#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
Expand Down Expand Up @@ -324,6 +328,99 @@ pub struct CrateData {
pub channel: Option<ReleaseChannel>,
}

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 {
if self.root_file_id != other.root_file_id {
return false;
}

if self.display_name != other.display_name {
return false;
}

if self.is_proc_macro != other.is_proc_macro {
return false;
}

if self.edition != other.edition {
return false;
}

if self.version != other.version {
return false;
}

let mut opts = self.cfg_options.clone();
opts.apply_diff(CfgDiff {
disable: other.cfg_options.clone().into_iter().collect(),
enable: vec![],
});

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" {
return false;
}
}

let mut itself = self.dependencies.iter();
let mut otself = other.dependencies.iter();
let (mut anx, mut bnx) = (itself.next(), otself.next());
loop {
match (anx, bnx) {
(None, None) => {
break;
}
(None, Some(b)) => {
if b.kind != DependencyKind::Normal {
bnx = otself.next();
} else {
break;
}
}
(Some(a), None) => {
if a.kind != DependencyKind::Normal {
anx = itself.next();
} else {
break;
}
}
(Some(a), Some(b)) => {
if a.kind != DependencyKind::Normal {
anx = itself.next();
continue;
}

if b.kind != DependencyKind::Normal {
bnx = otself.next();
continue;
}

if a != b {
return false;
}

anx = itself.next();
bnx = otself.next();
}
}
}

if self.env != other.env {
return false;
}

true
}
}

#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)]
pub enum Edition {
Edition2015,
Expand Down Expand Up @@ -351,26 +448,43 @@ impl Env {
}
}

#[derive(Debug, Clone, PartialEq, Eq, Hash)]
pub enum DependencyKind {
Normal,
Dev,
Build,
}

#[derive(Debug, Clone, PartialEq, Eq, Hash)]
pub struct Dependency {
pub crate_id: CrateId,
pub name: CrateName,
kind: DependencyKind,
prelude: bool,
}

impl Dependency {
pub fn new(name: CrateName, crate_id: CrateId) -> Self {
Self { name, crate_id, prelude: true }
pub fn new(name: CrateName, crate_id: CrateId, kind: DependencyKind) -> Self {
Self { name, crate_id, prelude: true, kind }
}

pub fn with_prelude(name: CrateName, crate_id: CrateId, prelude: bool) -> Self {
Self { name, crate_id, prelude }
pub fn with_prelude(
name: CrateName,
crate_id: CrateId,
prelude: bool,
kind: DependencyKind,
) -> Self {
Self { name, crate_id, prelude, kind }
}

/// Whether this dependency is to be added to the depending crate's extern prelude.
pub fn is_prelude(&self) -> bool {
self.prelude
}

pub fn kind(&self) -> &DependencyKind {
&self.kind
}
}

impl CrateGraph {
Expand Down Expand Up @@ -572,25 +686,41 @@ 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.sort_by_key(|dep| dep.crate_id);

let res = self.arena.iter().find_map(
|(id, data)| {
if data == crate_data {
Some(id)
} else {
None
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.origin.is_lib() && crate_data.origin.is_local() {
// See #15656 for a relevant example.
return Some(ExtendStrategy::Replace(id));
}
},
);

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

if let Some(res) = res {
id_map.insert(topo, 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)
}
};
} else {
let id = self.arena.alloc(crate_data.clone());
id_map.insert(topo, id);
Expand Down Expand Up @@ -636,9 +766,11 @@ impl CrateGraph {
match (cfg_if, std) {
(Some(cfg_if), Some(std)) => {
self.arena[cfg_if].dependencies.clear();
self.arena[std]
.dependencies
.push(Dependency::new(CrateName::new("cfg_if").unwrap(), cfg_if));
self.arena[std].dependencies.push(Dependency::new(
CrateName::new("cfg_if").unwrap(),
cfg_if,
DependencyKind::Normal,
));
true
}
_ => false,
Expand Down Expand Up @@ -759,7 +891,7 @@ impl fmt::Display for CyclicDependenciesError {

#[cfg(test)]
mod tests {
use crate::CrateOrigin;
use crate::{CrateOrigin, DependencyKind};

use super::{CrateGraph, CrateName, Dependency, Edition::Edition2018, Env, FileId};

Expand Down Expand Up @@ -806,13 +938,22 @@ mod tests {
None,
);
assert!(graph
.add_dep(crate1, Dependency::new(CrateName::new("crate2").unwrap(), crate2))
.add_dep(
crate1,
Dependency::new(CrateName::new("crate2").unwrap(), crate2, DependencyKind::Normal)
)
.is_ok());
assert!(graph
.add_dep(crate2, Dependency::new(CrateName::new("crate3").unwrap(), crate3))
.add_dep(
crate2,
Dependency::new(CrateName::new("crate3").unwrap(), crate3, DependencyKind::Normal)
)
.is_ok());
assert!(graph
.add_dep(crate3, Dependency::new(CrateName::new("crate1").unwrap(), crate1))
.add_dep(
crate3,
Dependency::new(CrateName::new("crate1").unwrap(), crate1, DependencyKind::Normal)
)
.is_err());
}

Expand Down Expand Up @@ -846,10 +987,16 @@ mod tests {
None,
);
assert!(graph
.add_dep(crate1, Dependency::new(CrateName::new("crate2").unwrap(), crate2))
.add_dep(
crate1,
Dependency::new(CrateName::new("crate2").unwrap(), crate2, DependencyKind::Normal)
)
.is_ok());
assert!(graph
.add_dep(crate2, Dependency::new(CrateName::new("crate2").unwrap(), crate2))
.add_dep(
crate2,
Dependency::new(CrateName::new("crate2").unwrap(), crate2, DependencyKind::Normal)
)
.is_err());
}

Expand Down Expand Up @@ -896,10 +1043,16 @@ mod tests {
None,
);
assert!(graph
.add_dep(crate1, Dependency::new(CrateName::new("crate2").unwrap(), crate2))
.add_dep(
crate1,
Dependency::new(CrateName::new("crate2").unwrap(), crate2, DependencyKind::Normal)
)
.is_ok());
assert!(graph
.add_dep(crate2, Dependency::new(CrateName::new("crate3").unwrap(), crate3))
.add_dep(
crate2,
Dependency::new(CrateName::new("crate3").unwrap(), crate3, DependencyKind::Normal)
)
.is_ok());
}

Expand Down Expand Up @@ -935,12 +1088,20 @@ mod tests {
assert!(graph
.add_dep(
crate1,
Dependency::new(CrateName::normalize_dashes("crate-name-with-dashes"), crate2)
Dependency::new(
CrateName::normalize_dashes("crate-name-with-dashes"),
crate2,
DependencyKind::Normal
)
)
.is_ok());
assert_eq!(
graph[crate1].dependencies,
vec![Dependency::new(CrateName::new("crate_name_with_dashes").unwrap(), crate2)]
vec![Dependency::new(
CrateName::new("crate_name_with_dashes").unwrap(),
crate2,
DependencyKind::Normal
)]
);
}
}
1 change: 1 addition & 0 deletions crates/base-db/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use rustc_hash::FxHashSet;
use syntax::{ast, Parse, SourceFile, TextRange, TextSize};
use triomphe::Arc;

pub use crate::input::DependencyKind;
pub use crate::{
change::Change,
input::{
Expand Down
Loading

0 comments on commit d7b72ce

Please sign in to comment.