Skip to content

Commit b1404d3

Browse files
VeykrilVeykril
committed
fix: Split toolchain and datalayout out of CrateData
1 parent a016555 commit b1404d3

File tree

22 files changed

+247
-346
lines changed

22 files changed

+247
-346
lines changed

‎crates/base-db/src/input.rs

Lines changed: 21 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ use std::{fmt, mem, ops, str::FromStr};
1111
use cfg::CfgOptions;
1212
use la_arena::{Arena, Idx, RawIdx};
1313
use rustc_hash::{FxHashMap, FxHashSet};
14-
use semver::Version;
1514
use syntax::SmolStr;
1615
use triomphe::Arc;
1716
use vfs::{file_set::FileSet, AbsPathBuf, AnchoredPath, FileId, VfsPath};
@@ -292,16 +291,11 @@ pub struct CrateData {
292291
pub dependencies: Vec<Dependency>,
293292
pub origin: CrateOrigin,
294293
pub is_proc_macro: bool,
295-
// FIXME: These things should not be per crate! These are more per workspace crate graph level
296-
// things. This info does need to be somewhat present though as to prevent deduplication from
297-
// happening across different workspaces with different layouts.
298-
pub target_layout: TargetLayoutLoadResult,
299-
pub toolchain: Option<Version>,
300294
}
301295

302296
impl CrateData {
303297
/// Check if [`other`] is almost equal to [`self`] ignoring `CrateOrigin` value.
304-
pub fn eq_ignoring_origin_and_deps(&self, other: &CrateData, ignore_dev_deps: bool) -> bool {
298+
fn eq_ignoring_origin_and_deps(&self, other: &CrateData, ignore_dev_deps: bool) -> bool {
305299
// This method has some obscure bits. These are mostly there to be compliant with
306300
// some patches. References to the patches are given.
307301
if self.root_file_id != other.root_file_id {
@@ -353,10 +347,6 @@ impl CrateData {
353347

354348
slf_deps.eq(other_deps)
355349
}
356-
357-
pub fn channel(&self) -> Option<ReleaseChannel> {
358-
self.toolchain.as_ref().and_then(|v| ReleaseChannel::from_str(&v.pre))
359-
}
360350
}
361351

362352
#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)]
@@ -439,8 +429,6 @@ impl CrateGraph {
439429
env: Env,
440430
is_proc_macro: bool,
441431
origin: CrateOrigin,
442-
target_layout: Result<Arc<str>, Arc<str>>,
443-
toolchain: Option<Version>,
444432
) -> CrateId {
445433
let data = CrateData {
446434
root_file_id,
@@ -452,9 +440,7 @@ impl CrateGraph {
452440
env,
453441
dependencies: Vec::new(),
454442
origin,
455-
target_layout,
456443
is_proc_macro,
457-
toolchain,
458444
};
459445
self.arena.alloc(data)
460446
}
@@ -524,6 +510,10 @@ impl CrateGraph {
524510
self.arena.is_empty()
525511
}
526512

513+
pub fn len(&self) -> usize {
514+
self.arena.len()
515+
}
516+
527517
pub fn iter(&self) -> impl Iterator<Item = CrateId> + '_ {
528518
self.arena.iter().map(|(idx, _)| idx)
529519
}
@@ -624,13 +614,16 @@ impl CrateGraph {
624614
///
625615
/// This will deduplicate the crates of the graph where possible.
626616
/// Note that for deduplication to fully work, `self`'s crate dependencies must be sorted by crate id.
627-
/// If the crate dependencies were sorted, the resulting graph from this `extend` call will also have the crate dependencies sorted.
617+
/// If the crate dependencies were sorted, the resulting graph from this `extend` call will also
618+
/// have the crate dependencies sorted.
619+
///
620+
/// Returns a mapping from `other`'s crate ids to the new crate ids in `self`.
628621
pub fn extend(
629622
&mut self,
630623
mut other: CrateGraph,
631624
proc_macros: &mut ProcMacroPaths,
632-
on_finished: impl FnOnce(&FxHashMap<CrateId, CrateId>),
633-
) {
625+
may_merge: impl Fn((CrateId, &CrateData), (CrateId, &CrateData)) -> bool,
626+
) -> FxHashMap<CrateId, CrateId> {
634627
let topo = other.crates_in_topological_order();
635628
let mut id_map: FxHashMap<CrateId, CrateId> = FxHashMap::default();
636629
for topo in topo {
@@ -639,6 +632,10 @@ impl CrateGraph {
639632
crate_data.dependencies.iter_mut().for_each(|dep| dep.crate_id = id_map[&dep.crate_id]);
640633
crate_data.dependencies.sort_by_key(|dep| dep.crate_id);
641634
let res = self.arena.iter().find_map(|(id, data)| {
635+
if !may_merge((id, &data), (topo, &crate_data)) {
636+
return None;
637+
}
638+
642639
match (&data.origin, &crate_data.origin) {
643640
(a, b) if a == b => {
644641
if data.eq_ignoring_origin_and_deps(crate_data, false) {
@@ -663,8 +660,7 @@ impl CrateGraph {
663660
None
664661
});
665662

666-
if let Some((res, should_update_lib_to_local)) = res {
667-
id_map.insert(topo, res);
663+
let new_id = if let Some((res, should_update_lib_to_local)) = res {
668664
if should_update_lib_to_local {
669665
assert!(self.arena[res].origin.is_lib());
670666
assert!(crate_data.origin.is_local());
@@ -673,16 +669,17 @@ impl CrateGraph {
673669
// Move local's dev dependencies into the newly-local-formerly-lib crate.
674670
self.arena[res].dependencies = crate_data.dependencies.clone();
675671
}
672+
res
676673
} else {
677-
let id = self.arena.alloc(crate_data.clone());
678-
id_map.insert(topo, id);
679-
}
674+
self.arena.alloc(crate_data.clone())
675+
};
676+
id_map.insert(topo, new_id);
680677
}
681678

682679
*proc_macros =
683680
mem::take(proc_macros).into_iter().map(|(id, macros)| (id_map[&id], macros)).collect();
684681

685-
on_finished(&id_map);
682+
id_map
686683
}
687684

688685
fn find_path(
@@ -889,8 +886,6 @@ mod tests {
889886
Env::default(),
890887
false,
891888
CrateOrigin::Local { repo: None, name: None },
892-
Err("".into()),
893-
None,
894889
);
895890
let crate2 = graph.add_crate_root(
896891
FileId::from_raw(2u32),
@@ -902,8 +897,6 @@ mod tests {
902897
Env::default(),
903898
false,
904899
CrateOrigin::Local { repo: None, name: None },
905-
Err("".into()),
906-
None,
907900
);
908901
let crate3 = graph.add_crate_root(
909902
FileId::from_raw(3u32),
@@ -915,8 +908,6 @@ mod tests {
915908
Env::default(),
916909
false,
917910
CrateOrigin::Local { repo: None, name: None },
918-
Err("".into()),
919-
None,
920911
);
921912
assert!(graph
922913
.add_dep(
@@ -951,8 +942,6 @@ mod tests {
951942
Env::default(),
952943
false,
953944
CrateOrigin::Local { repo: None, name: None },
954-
Err("".into()),
955-
None,
956945
);
957946
let crate2 = graph.add_crate_root(
958947
FileId::from_raw(2u32),
@@ -964,8 +953,6 @@ mod tests {
964953
Env::default(),
965954
false,
966955
CrateOrigin::Local { repo: None, name: None },
967-
Err("".into()),
968-
None,
969956
);
970957
assert!(graph
971958
.add_dep(
@@ -994,8 +981,6 @@ mod tests {
994981
Env::default(),
995982
false,
996983
CrateOrigin::Local { repo: None, name: None },
997-
Err("".into()),
998-
None,
999984
);
1000985
let crate2 = graph.add_crate_root(
1001986
FileId::from_raw(2u32),
@@ -1007,8 +992,6 @@ mod tests {
1007992
Env::default(),
1008993
false,
1009994
CrateOrigin::Local { repo: None, name: None },
1010-
Err("".into()),
1011-
None,
1012995
);
1013996
let crate3 = graph.add_crate_root(
1014997
FileId::from_raw(3u32),
@@ -1020,8 +1003,6 @@ mod tests {
10201003
Env::default(),
10211004
false,
10221005
CrateOrigin::Local { repo: None, name: None },
1023-
Err("".into()),
1024-
None,
10251006
);
10261007
assert!(graph
10271008
.add_dep(
@@ -1050,8 +1031,6 @@ mod tests {
10501031
Env::default(),
10511032
false,
10521033
CrateOrigin::Local { repo: None, name: None },
1053-
Err("".into()),
1054-
None,
10551034
);
10561035
let crate2 = graph.add_crate_root(
10571036
FileId::from_raw(2u32),
@@ -1063,8 +1042,6 @@ mod tests {
10631042
Env::default(),
10641043
false,
10651044
CrateOrigin::Local { repo: None, name: None },
1066-
Err("".into()),
1067-
None,
10681045
);
10691046
assert!(graph
10701047
.add_dep(

‎crates/base-db/src/lib.rs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,20 @@ pub trait SourceDatabase: FileLoader + std::fmt::Debug {
6262
/// The crate graph.
6363
#[salsa::input]
6464
fn crate_graph(&self) -> Arc<CrateGraph>;
65+
66+
// FIXME: Consider removing this, making HirDatabase::target_data_layout an input query
67+
#[salsa::input]
68+
fn data_layout(&self, krate: CrateId) -> TargetLayoutLoadResult;
69+
70+
#[salsa::input]
71+
fn toolchain(&self, krate: CrateId) -> Option<Version>;
72+
73+
#[salsa::transparent]
74+
fn toolchain_channel(&self, krate: CrateId) -> Option<ReleaseChannel>;
75+
}
76+
77+
fn toolchain_channel(db: &dyn SourceDatabase, krate: CrateId) -> Option<ReleaseChannel> {
78+
db.toolchain(krate).as_ref().and_then(|v| ReleaseChannel::from_str(&v.pre))
6579
}
6680

6781
fn parse(db: &dyn SourceDatabase, file_id: FileId) -> Parse<ast::SourceFile> {

‎crates/hir-expand/src/change.rs

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
11
//! Defines a unit of change that can applied to the database to get the next
22
//! state. Changes are transactional.
3-
use base_db::{salsa::Durability, CrateGraph, FileChange, SourceDatabaseExt, SourceRoot};
3+
use base_db::{
4+
salsa::Durability, CrateGraph, CrateId, FileChange, SourceDatabaseExt, SourceRoot,
5+
TargetLayoutLoadResult, Version,
6+
};
7+
use la_arena::RawIdx;
48
use span::FileId;
59
use triomphe::Arc;
610

@@ -10,6 +14,8 @@ use crate::{db::ExpandDatabase, proc_macro::ProcMacros};
1014
pub struct Change {
1115
pub source_change: FileChange,
1216
pub proc_macros: Option<ProcMacros>,
17+
pub toolchains: Option<Vec<Option<Version>>>,
18+
pub target_data_layouts: Option<Vec<TargetLayoutLoadResult>>,
1319
}
1420

1521
impl Change {
@@ -22,6 +28,24 @@ impl Change {
2228
if let Some(proc_macros) = self.proc_macros {
2329
db.set_proc_macros_with_durability(Arc::new(proc_macros), Durability::HIGH);
2430
}
31+
if let Some(target_data_layouts) = self.target_data_layouts {
32+
for (id, val) in target_data_layouts.into_iter().enumerate() {
33+
db.set_data_layout_with_durability(
34+
CrateId::from_raw(RawIdx::from(id as u32)),
35+
val,
36+
Durability::HIGH,
37+
);
38+
}
39+
}
40+
if let Some(toolchains) = self.toolchains {
41+
for (id, val) in toolchains.into_iter().enumerate() {
42+
db.set_toolchain_with_durability(
43+
CrateId::from_raw(RawIdx::from(id as u32)),
44+
val,
45+
Durability::HIGH,
46+
);
47+
}
48+
}
2549
}
2650

2751
pub fn change_file(&mut self, file_id: FileId, new_text: Option<Arc<str>>) {
@@ -36,6 +60,14 @@ impl Change {
3660
self.proc_macros = Some(proc_macros);
3761
}
3862

63+
pub fn set_toolchains(&mut self, toolchains: Vec<Option<Version>>) {
64+
self.toolchains = Some(toolchains);
65+
}
66+
67+
pub fn set_target_data_layouts(&mut self, target_data_layouts: Vec<TargetLayoutLoadResult>) {
68+
self.target_data_layouts = Some(target_data_layouts);
69+
}
70+
3971
pub fn set_roots(&mut self, roots: Vec<SourceRoot>) {
4072
self.source_change.set_roots(roots)
4173
}

‎crates/hir-expand/src/declarative.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ impl DeclarativeMacroExpander {
3131
call_id: MacroCallId,
3232
) -> ExpandResult<tt::Subtree> {
3333
let loc = db.lookup_intern_macro_call(call_id);
34-
let toolchain = &db.crate_graph()[loc.def.krate].toolchain;
34+
let toolchain = db.toolchain(loc.def.krate);
3535
let new_meta_vars = toolchain.as_ref().map_or(false, |version| {
3636
REQUIREMENT.get_or_init(|| VersionReq::parse(">=1.76").unwrap()).matches(
3737
&base_db::Version {
@@ -67,7 +67,7 @@ impl DeclarativeMacroExpander {
6767
krate: CrateId,
6868
call_site: Span,
6969
) -> ExpandResult<tt::Subtree> {
70-
let toolchain = &db.crate_graph()[krate].toolchain;
70+
let toolchain = db.toolchain(krate);
7171
let new_meta_vars = toolchain.as_ref().map_or(false, |version| {
7272
REQUIREMENT.get_or_init(|| VersionReq::parse(">=1.76").unwrap()).matches(
7373
&base_db::Version {
@@ -119,7 +119,7 @@ impl DeclarativeMacroExpander {
119119
_ => None,
120120
}
121121
};
122-
let toolchain = crate_data.toolchain.as_ref();
122+
let toolchain = db.toolchain(def_crate);
123123
let new_meta_vars = toolchain.as_ref().map_or(false, |version| {
124124
REQUIREMENT.get_or_init(|| VersionReq::parse(">=1.76").unwrap()).matches(
125125
&base_db::Version {

‎crates/hir-ty/src/layout/target.rs

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,8 @@ pub fn target_data_layout_query(
1111
db: &dyn HirDatabase,
1212
krate: CrateId,
1313
) -> Result<Arc<TargetDataLayout>, Arc<str>> {
14-
let crate_graph = db.crate_graph();
15-
let res = crate_graph[krate].target_layout.as_deref();
16-
match res {
17-
Ok(it) => match TargetDataLayout::parse_from_llvm_datalayout_string(it) {
14+
match db.data_layout(krate) {
15+
Ok(it) => match TargetDataLayout::parse_from_llvm_datalayout_string(&it) {
1816
Ok(it) => Ok(Arc::new(it)),
1917
Err(e) => {
2018
Err(match e {
@@ -44,6 +42,6 @@ pub fn target_data_layout_query(
4442
}.into())
4543
}
4644
},
47-
Err(e) => Err(Arc::from(&**e)),
45+
Err(e) => Err(e),
4846
}
4947
}

‎crates/hir-ty/src/layout/tests.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ fn current_machine_data_layout() -> String {
2727
fn eval_goal(ra_fixture: &str, minicore: &str) -> Result<Arc<Layout>, LayoutError> {
2828
let target_data_layout = current_machine_data_layout();
2929
let ra_fixture = format!(
30-
"{minicore}//- /main.rs crate:test target_data_layout:{target_data_layout}\n{ra_fixture}",
30+
"//- target_data_layout: {target_data_layout}\n{minicore}//- /main.rs crate:test\n{ra_fixture}",
3131
);
3232

3333
let (db, file_ids) = TestDB::with_many_files(&ra_fixture);
@@ -76,7 +76,7 @@ fn eval_goal(ra_fixture: &str, minicore: &str) -> Result<Arc<Layout>, LayoutErro
7676
fn eval_expr(ra_fixture: &str, minicore: &str) -> Result<Arc<Layout>, LayoutError> {
7777
let target_data_layout = current_machine_data_layout();
7878
let ra_fixture = format!(
79-
"{minicore}//- /main.rs crate:test target_data_layout:{target_data_layout}\nfn main(){{let goal = {{{ra_fixture}}};}}",
79+
"//- target_data_layout: {target_data_layout}\n{minicore}//- /main.rs crate:test\nfn main(){{let goal = {{{ra_fixture}}};}}",
8080
);
8181

8282
let (db, file_id) = TestDB::with_single_file(&ra_fixture);

‎crates/ide-completion/src/context.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -717,7 +717,7 @@ impl<'a> CompletionContext<'a> {
717717
let krate = scope.krate();
718718
let module = scope.module();
719719

720-
let toolchain = db.crate_graph()[krate.into()].channel();
720+
let toolchain = db.toolchain_channel(krate.into());
721721
// `toolchain == None` means we're in some detached files. Since we have no information on
722722
// the toolchain being used, let's just allow unstable items to be listed.
723723
let is_nightly = matches!(toolchain, Some(base_db::ReleaseChannel::Nightly) | None);

‎crates/ide/src/doc_links.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -501,7 +501,7 @@ fn get_doc_base_urls(
501501
let Some(krate) = def.krate(db) else { return Default::default() };
502502
let Some(display_name) = krate.display_name(db) else { return Default::default() };
503503
let crate_data = &db.crate_graph()[krate.into()];
504-
let channel = crate_data.channel().unwrap_or(ReleaseChannel::Nightly).as_str();
504+
let channel = db.toolchain_channel(krate.into()).unwrap_or(ReleaseChannel::Nightly).as_str();
505505

506506
let (web_base, local_base) = match &crate_data.origin {
507507
// std and co do not specify `html_root_url` any longer so we gotta handwrite this ourself.

‎crates/ide/src/lib.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -253,11 +253,11 @@ impl Analysis {
253253
Env::default(),
254254
false,
255255
CrateOrigin::Local { repo: None, name: None },
256-
Err("Analysis::from_single_file has no target layout".into()),
257-
None,
258256
);
259257
change.change_file(file_id, Some(Arc::from(text)));
260258
change.set_crate_graph(crate_graph);
259+
change.set_target_data_layouts(vec![Err("fixture has no layout".into())]);
260+
change.set_toolchains(vec![None]);
261261
host.apply_change(change);
262262
(host.analysis(), file_id)
263263
}

0 commit comments

Comments
 (0)