Skip to content

internal: Remove crate graph deduplication logic #18080

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Sep 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion crates/base-db/src/change.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,23 @@

use std::fmt;

use rustc_hash::FxHashMap;
use salsa::Durability;
use triomphe::Arc;
use vfs::FileId;

use crate::{CrateGraph, SourceDatabaseFileInputExt, SourceRoot, SourceRootDatabase, SourceRootId};
use crate::{
CrateGraph, CrateId, CrateWorkspaceData, SourceDatabaseFileInputExt, SourceRoot,
SourceRootDatabase, SourceRootId,
};

/// Encapsulate a bunch of raw `.set` calls on the database.
#[derive(Default)]
pub struct FileChange {
pub roots: Option<Vec<SourceRoot>>,
pub files_changed: Vec<(FileId, Option<String>)>,
pub crate_graph: Option<CrateGraph>,
pub ws_data: Option<FxHashMap<CrateId, Arc<CrateWorkspaceData>>>,
}

impl fmt::Debug for FileChange {
Expand Down Expand Up @@ -50,6 +55,10 @@ impl FileChange {
self.crate_graph = Some(graph);
}

pub fn set_ws_data(&mut self, data: FxHashMap<CrateId, Arc<CrateWorkspaceData>>) {
self.ws_data = Some(data);
}

pub fn apply(self, db: &mut dyn SourceRootDatabase) {
let _p = tracing::info_span!("FileChange::apply").entered();
if let Some(roots) = self.roots {
Expand All @@ -74,6 +83,9 @@ impl FileChange {
if let Some(crate_graph) = self.crate_graph {
db.set_crate_graph_with_durability(Arc::new(crate_graph), Durability::HIGH);
}
if let Some(data) = self.ws_data {
db.set_crate_workspace_data_with_durability(Arc::new(data), Durability::HIGH);
}
}
}

Expand Down
22 changes: 4 additions & 18 deletions crates/base-db/src/input.rs
Original file line number Diff line number Diff line change
Expand Up @@ -491,43 +491,29 @@ impl CrateGraph {
.for_each(|(_, data)| data.dependencies.sort_by_key(|dep| dep.crate_id));
}

/// Extends this crate graph by adding a complete disjoint second crate
/// Extends this crate graph by adding a complete second crate
/// graph and adjust the ids in the [`ProcMacroPaths`] accordingly.
///
/// This will deduplicate the crates of the graph where possible.
/// 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.
///
/// Returns a mapping from `other`'s crate ids to the new crate ids in `self`.
/// Returns a map mapping `other`'s IDs to the new IDs in `self`.
pub fn extend(
&mut self,
mut other: CrateGraph,
proc_macros: &mut ProcMacroPaths,
merge: impl Fn((CrateId, &mut CrateData), (CrateId, &CrateData)) -> bool,
) -> FxHashMap<CrateId, CrateId> {
let m = self.len();
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_mut()
.take(m)
.find_map(|(id, data)| merge((id, data), (topo, crate_data)).then_some(id));

let new_id =
if let Some(res) = res { res } else { self.arena.alloc(crate_data.clone()) };

let new_id = self.arena.alloc(crate_data.clone());
id_map.insert(topo, new_id);
}

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

id_map
}

Expand Down
26 changes: 19 additions & 7 deletions crates/base-db/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,12 @@ mod input;

use std::panic;

use rustc_hash::FxHashMap;
use salsa::Durability;
use span::EditionedFileId;
use syntax::{ast, Parse, SourceFile, SyntaxError};
use triomphe::Arc;
use vfs::FileId;
use vfs::{AbsPathBuf, FileId};

pub use crate::{
change::FileChange,
Expand Down Expand Up @@ -74,19 +75,30 @@ pub trait SourceDatabase: FileLoader + std::fmt::Debug {
#[salsa::input]
fn crate_graph(&self) -> Arc<CrateGraph>;

// FIXME: Consider removing this, making HirDatabase::target_data_layout an input query
#[salsa::input]
fn data_layout(&self, krate: CrateId) -> TargetLayoutLoadResult;

#[salsa::input]
fn toolchain(&self, krate: CrateId) -> Option<Version>;
fn crate_workspace_data(&self) -> Arc<FxHashMap<CrateId, Arc<CrateWorkspaceData>>>;

#[salsa::transparent]
fn toolchain_channel(&self, krate: CrateId) -> Option<ReleaseChannel>;
}

/// Crate related data shared by the whole workspace.
#[derive(Debug, PartialEq, Eq, Hash, Clone)]
pub struct CrateWorkspaceData {
/// The working directory to run proc-macros in. This is usually the workspace root of cargo workspaces.
pub proc_macro_cwd: Option<AbsPathBuf>,
// FIXME: Consider removing this, making HirDatabase::target_data_layout an input query
pub data_layout: TargetLayoutLoadResult,
/// Toolchain version used to compile the crate.
pub toolchain: Option<Version>,
}

fn toolchain_channel(db: &dyn SourceDatabase, krate: CrateId) -> Option<ReleaseChannel> {
db.toolchain(krate).as_ref().and_then(|v| ReleaseChannel::from_str(&v.pre))
db.crate_workspace_data()
.get(&krate)?
.toolchain
.as_ref()
.and_then(|v| ReleaseChannel::from_str(&v.pre))
}

fn parse(db: &dyn SourceDatabase, file_id: EditionedFileId) -> Parse<ast::SourceFile> {
Expand Down
4 changes: 2 additions & 2 deletions crates/hir-def/src/attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ impl Attrs {
}

impl Attrs {
pub fn by_key<'attrs>(&'attrs self, key: &'attrs Symbol) -> AttrQuery<'_> {
pub fn by_key<'attrs>(&'attrs self, key: &'attrs Symbol) -> AttrQuery<'attrs> {
AttrQuery { attrs: self, key }
}

Expand Down Expand Up @@ -594,7 +594,7 @@ impl<'attr> AttrQuery<'attr> {
/// #[doc(html_root_url = "url")]
/// ^^^^^^^^^^^^^ key
/// ```
pub fn find_string_value_in_tt(self, key: &'attr Symbol) -> Option<&str> {
pub fn find_string_value_in_tt(self, key: &'attr Symbol) -> Option<&'attr str> {
self.tt_values().find_map(|tt| {
let name = tt.token_trees.iter()
.skip_while(|tt| !matches!(tt, tt::TokenTree::Leaf(tt::Leaf::Ident(tt::Ident { sym, ..} )) if *sym == *key))
Expand Down
2 changes: 1 addition & 1 deletion crates/hir-def/src/body.rs
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ impl Body {
pub fn blocks<'a>(
&'a self,
db: &'a dyn DefDatabase,
) -> impl Iterator<Item = (BlockId, Arc<DefMap>)> + '_ {
) -> impl Iterator<Item = (BlockId, Arc<DefMap>)> + 'a {
self.block_scopes.iter().map(move |&block| (block, db.block_def_map(block)))
}

Expand Down
1 change: 1 addition & 0 deletions crates/hir-def/src/macro_expansion_tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,7 @@ impl ProcMacroExpander for IdentityWhenValidProcMacroExpander {
_: Span,
_: Span,
_: Span,
_: Option<String>,
) -> Result<Subtree, ProcMacroExpansionError> {
let (parse, _) = syntax_bridge::token_tree_to_syntax_node(
subtree,
Expand Down
43 changes: 10 additions & 33 deletions crates/hir-expand/src/change.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
//! Defines a unit of change that can applied to the database to get the next
//! state. Changes are transactional.
use base_db::{
salsa::Durability, CrateGraph, CrateId, FileChange, SourceRoot, SourceRootDatabase,
TargetLayoutLoadResult, Version,
salsa::Durability, CrateGraph, CrateId, CrateWorkspaceData, FileChange, SourceRoot,
SourceRootDatabase,
};
use la_arena::RawIdx;
use rustc_hash::FxHashMap;
use span::FileId;
use triomphe::Arc;

Expand All @@ -14,8 +14,6 @@ use crate::{db::ExpandDatabase, proc_macro::ProcMacros};
pub struct ChangeWithProcMacros {
pub source_change: FileChange,
pub proc_macros: Option<ProcMacros>,
pub toolchains: Option<Vec<Option<Version>>>,
pub target_data_layouts: Option<Vec<TargetLayoutLoadResult>>,
}

impl ChangeWithProcMacros {
Expand All @@ -28,46 +26,25 @@ impl ChangeWithProcMacros {
if let Some(proc_macros) = self.proc_macros {
db.set_proc_macros_with_durability(Arc::new(proc_macros), Durability::HIGH);
}
if let Some(target_data_layouts) = self.target_data_layouts {
for (id, val) in target_data_layouts.into_iter().enumerate() {
db.set_data_layout_with_durability(
CrateId::from_raw(RawIdx::from(id as u32)),
val,
Durability::HIGH,
);
}
}
if let Some(toolchains) = self.toolchains {
for (id, val) in toolchains.into_iter().enumerate() {
db.set_toolchain_with_durability(
CrateId::from_raw(RawIdx::from(id as u32)),
val,
Durability::HIGH,
);
}
}
}

pub fn change_file(&mut self, file_id: FileId, new_text: Option<String>) {
self.source_change.change_file(file_id, new_text)
}

pub fn set_crate_graph(&mut self, graph: CrateGraph) {
self.source_change.set_crate_graph(graph)
pub fn set_crate_graph(
&mut self,
graph: CrateGraph,
ws_data: FxHashMap<CrateId, Arc<CrateWorkspaceData>>,
) {
self.source_change.set_crate_graph(graph);
self.source_change.set_ws_data(ws_data);
}

pub fn set_proc_macros(&mut self, proc_macros: ProcMacros) {
self.proc_macros = Some(proc_macros);
}

pub fn set_toolchains(&mut self, toolchains: Vec<Option<Version>>) {
self.toolchains = Some(toolchains);
}

pub fn set_target_data_layouts(&mut self, target_data_layouts: Vec<TargetLayoutLoadResult>) {
self.target_data_layouts = Some(target_data_layouts);
}

pub fn set_roots(&mut self, roots: Vec<SourceRoot>) {
self.source_change.set_roots(roots)
}
Expand Down
15 changes: 13 additions & 2 deletions crates/hir-expand/src/proc_macro.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ pub trait ProcMacroExpander: fmt::Debug + Send + Sync + RefUnwindSafe {
def_site: Span,
call_site: Span,
mixed_site: Span,
current_dir: Option<String>,
) -> Result<tt::Subtree, ProcMacroExpansionError>;
}

Expand Down Expand Up @@ -234,8 +235,18 @@ impl CustomProcMacroExpander {
let krate_graph = db.crate_graph();
// Proc macros have access to the environment variables of the invoking crate.
let env = &krate_graph[calling_crate].env;
match proc_macro.expander.expand(tt, attr_arg, env, def_site, call_site, mixed_site)
{
match proc_macro.expander.expand(
tt,
attr_arg,
env,
def_site,
call_site,
mixed_site,
db.crate_workspace_data()[&calling_crate]
.proc_macro_cwd
.as_ref()
.map(ToString::to_string),
) {
Ok(t) => ExpandResult::ok(t),
Err(err) => match err {
// Don't discard the item in case something unexpected happened while expanding attributes
Expand Down
6 changes: 3 additions & 3 deletions crates/hir-ty/src/layout/target.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ pub fn target_data_layout_query(
db: &dyn HirDatabase,
krate: CrateId,
) -> Result<Arc<TargetDataLayout>, Arc<str>> {
match db.data_layout(krate) {
Ok(it) => match TargetDataLayout::parse_from_llvm_datalayout_string(&it) {
match &db.crate_workspace_data()[&krate].data_layout {
Ok(it) => match TargetDataLayout::parse_from_llvm_datalayout_string(it) {
Ok(it) => Ok(Arc::new(it)),
Err(e) => {
Err(match e {
Expand Down Expand Up @@ -42,6 +42,6 @@ pub fn target_data_layout_query(
}.into())
}
},
Err(e) => Err(e),
Err(e) => Err(e.clone()),
}
}
18 changes: 13 additions & 5 deletions crates/ide/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,15 +57,16 @@ mod view_item_tree;
mod view_memory_layout;
mod view_mir;

use std::panic::UnwindSafe;
use std::{iter, panic::UnwindSafe};

use cfg::CfgOptions;
use fetch_crates::CrateInfo;
use hir::{sym, ChangeWithProcMacros};
use ide_db::{
base_db::{
salsa::{self, ParallelDatabase},
CrateOrigin, Env, FileLoader, FileSet, SourceDatabase, SourceRootDatabase, VfsPath,
CrateOrigin, CrateWorkspaceData, Env, FileLoader, FileSet, SourceDatabase,
SourceRootDatabase, VfsPath,
},
prime_caches, symbol_index, FxHashMap, FxIndexSet, LineIndexDatabase,
};
Expand Down Expand Up @@ -256,9 +257,16 @@ impl Analysis {
CrateOrigin::Local { repo: None, name: None },
);
change.change_file(file_id, Some(text));
change.set_crate_graph(crate_graph);
change.set_target_data_layouts(vec![Err("fixture has no layout".into())]);
change.set_toolchains(vec![None]);
let ws_data = crate_graph
.iter()
.zip(iter::repeat(Arc::new(CrateWorkspaceData {
proc_macro_cwd: None,
data_layout: Err("fixture has no layout".into()),
toolchain: None,
})))
.collect();
change.set_crate_graph(crate_graph, ws_data);

host.apply_change(change);
(host.analysis(), file_id)
}
Expand Down
27 changes: 20 additions & 7 deletions crates/load-cargo/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use hir_expand::proc_macro::{
ProcMacros,
};
use ide_db::{
base_db::{CrateGraph, Env, SourceRoot, SourceRootId},
base_db::{CrateGraph, CrateWorkspaceData, Env, SourceRoot, SourceRootId},
prime_caches, ChangeWithProcMacros, FxHashMap, RootDatabase,
};
use itertools::Itertools;
Expand Down Expand Up @@ -447,12 +447,16 @@ fn load_crate_graph(
let source_roots = source_root_config.partition(vfs);
analysis_change.set_roots(source_roots);

let num_crates = crate_graph.len();
analysis_change.set_crate_graph(crate_graph);
let ws_data = crate_graph
.iter()
.zip(iter::repeat(From::from(CrateWorkspaceData {
proc_macro_cwd: None,
data_layout: target_layout.clone(),
toolchain: toolchain.clone(),
})))
.collect();
analysis_change.set_crate_graph(crate_graph, ws_data);
analysis_change.set_proc_macros(proc_macros);
analysis_change
.set_target_data_layouts(iter::repeat(target_layout.clone()).take(num_crates).collect());
analysis_change.set_toolchains(iter::repeat(toolchain.clone()).take(num_crates).collect());

db.apply_change(analysis_change);
db
Expand Down Expand Up @@ -489,8 +493,17 @@ impl ProcMacroExpander for Expander {
def_site: Span,
call_site: Span,
mixed_site: Span,
current_dir: Option<String>,
) -> Result<tt::Subtree<Span>, ProcMacroExpansionError> {
match self.0.expand(subtree, attrs, env.clone(), def_site, call_site, mixed_site) {
match self.0.expand(
subtree,
attrs,
env.clone(),
def_site,
call_site,
mixed_site,
current_dir,
) {
Ok(Ok(subtree)) => Ok(subtree),
Ok(Err(err)) => Err(ProcMacroExpansionError::Panic(err.0)),
Err(err) => Err(ProcMacroExpansionError::System(err.to_string())),
Expand Down
Loading