Skip to content

Commit ab0149d

Browse files
cormacrelfcormacrelf
committed
fix config invalidation
1 parent 69bdf4a commit ab0149d

File tree

1 file changed

+74
-41
lines changed
  • crates/rust-analyzer/src/config

1 file changed

+74
-41
lines changed

‎crates/rust-analyzer/src/config/tree.rs‎

Lines changed: 74 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ pub struct ConfigChanges {
4141
/// - `Some(None)` => the client config was removed / reset or something
4242
/// - `Some(Some(...))` => the client config was updated
4343
client_change: Option<Option<Arc<ConfigInput>>>,
44-
parent_changes: Vec<ConfigParentChange>,
44+
parent_changes: FxHashMap<FileId, ConfigParent>,
4545
}
4646

4747
#[derive(Debug)]
@@ -103,7 +103,7 @@ impl ConcurrentConfigTree {
103103

104104
#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)]
105105
enum ConfigSource {
106-
ClientConfig,
106+
XdgConfig(FileId),
107107
RaToml(FileId),
108108
}
109109

@@ -115,12 +115,13 @@ slotmap::new_key_type! {
115115
struct ConfigNode {
116116
src: ConfigSource,
117117
input: Option<Arc<ConfigInput>>,
118-
computed: ComputedIdx,
118+
computed_idx: ComputedIdx,
119119
}
120120

121121
struct ConfigTree {
122122
tree: indextree::Arena<ConfigNode>,
123123
client_config: Option<Arc<ConfigInput>>,
124+
xdg_config_file_id: FileId,
124125
xdg_config_node_id: NodeId,
125126
ra_file_id_map: FxHashMap<FileId, NodeId>,
126127
computed: SlotMap<ComputedIdx, Option<Arc<LocalConfigData>>>,
@@ -164,14 +165,21 @@ impl ConfigTree {
164165
let mut tree = indextree::Arena::new();
165166
let mut computed = SlotMap::default();
166167
let mut ra_file_id_map = FxHashMap::default();
167-
let xdg_config = tree.new_node(ConfigNode {
168-
src: ConfigSource::RaToml(xdg_config_file_id),
168+
let xdg_config_node_id = tree.new_node(ConfigNode {
169+
src: ConfigSource::XdgConfig(xdg_config_file_id),
169170
input: None,
170-
computed: computed.insert(Option::<Arc<LocalConfigData>>::None),
171+
computed_idx: computed.insert(Option::<Arc<LocalConfigData>>::None),
171172
});
172-
ra_file_id_map.insert(xdg_config_file_id, xdg_config);
173+
ra_file_id_map.insert(xdg_config_file_id, xdg_config_node_id);
173174

174-
Self { client_config: None, xdg_config_node_id: xdg_config, ra_file_id_map, tree, computed }
175+
Self {
176+
client_config: None,
177+
xdg_config_file_id,
178+
xdg_config_node_id,
179+
ra_file_id_map,
180+
tree,
181+
computed,
182+
}
175183
}
176184

177185
fn read_only(&self, file_id: FileId) -> Result<Option<Arc<LocalConfigData>>, ConfigTreeError> {
@@ -196,7 +204,12 @@ impl ConfigTree {
196204
return Err(ConfigTreeError::Removed);
197205
}
198206
let node = self.tree.get(node_id).ok_or(ConfigTreeError::NonExistent)?.get();
199-
let stored = self.computed[node.computed].clone();
207+
let stored = self.computed[node.computed_idx].clone();
208+
tracing::trace!(
209+
"read_only_inner on {:?} got {:?}",
210+
node.src,
211+
stored.as_ref().map(|_| "some stored value")
212+
);
200213
Ok(stored)
201214
}
202215

@@ -214,7 +227,8 @@ impl ConfigTree {
214227
return Err(ConfigTreeError::Removed);
215228
}
216229
let node = self.tree.get(node_id).ok_or(ConfigTreeError::NonExistent)?.get();
217-
let idx = node.computed;
230+
tracing::trace!("compute_inner on {:?}", node.src);
231+
let idx = node.computed_idx;
218232
let slot = &mut self.computed[idx];
219233
if let Some(slot) = slot {
220234
Ok(slot.clone())
@@ -250,12 +264,17 @@ impl ConfigTree {
250264

251265
fn insert_toml(&mut self, file_id: FileId, input: Option<Arc<ConfigInput>>) -> NodeId {
252266
let computed = self.computed.insert(None);
253-
let node =
254-
self.tree.new_node(ConfigNode { src: ConfigSource::RaToml(file_id), input, computed });
255-
if let Some(_removed) = self.ra_file_id_map.insert(file_id, node) {
267+
let node_id = self.tree.new_node(ConfigNode {
268+
src: ConfigSource::RaToml(file_id),
269+
input,
270+
computed_idx: computed,
271+
});
272+
if let Some(_removed) = self.ra_file_id_map.insert(file_id, node_id) {
256273
panic!("ERROR: node should not have existed for {file_id:?} but it did");
257274
}
258-
node
275+
// By default, everything is under the xdg_config_node_id
276+
self.xdg_config_node_id.append(node_id, &mut self.tree);
277+
node_id
259278
}
260279

261280
fn update_toml(
@@ -274,6 +293,7 @@ impl ConfigTree {
274293
node.get_mut().input = input;
275294

276295
self.invalidate_subtree(node_id);
296+
// tracing::trace!("invalidated subtree:\n{:#?}", node_id.debug_pretty_print(&self.tree));
277297
Ok(node_id)
278298
}
279299

@@ -292,7 +312,16 @@ impl ConfigTree {
292312
let Some(desc) = self.tree.get(x) else {
293313
return;
294314
};
295-
self.computed.get_mut(desc.get().computed).take();
315+
let desc = desc.get();
316+
let Some(slot) = self.computed.get_mut(desc.computed_idx) else {
317+
tracing::error!(
318+
"computed_idx missing from computed local config slotmap: {:?}",
319+
desc.computed_idx
320+
);
321+
return;
322+
};
323+
tracing::trace!("invalidating {x:?} / {:?}", desc.src);
324+
slot.take();
296325
});
297326
}
298327

@@ -314,32 +343,22 @@ impl ConfigTree {
314343
errors: &mut Vec<ConfigTreeError>,
315344
) {
316345
let mut scratch_errors = Vec::new();
317-
let ConfigChanges { client_change, ra_toml_changes, parent_changes } = changes;
346+
let ConfigChanges { client_change, ra_toml_changes, mut parent_changes } = changes;
318347

319348
if let Some(change) = client_change {
320349
self.client_config = change;
321350
}
322351

323-
for ConfigParentChange { file_id, parent } in parent_changes {
324-
let node_id = self.ensure_node(file_id);
325-
let parent_node_id = match parent {
326-
ConfigParent::UserDefault => self.xdg_config_node_id,
327-
ConfigParent::Parent(parent_file_id) => self.ensure_node(parent_file_id),
328-
};
329-
// order of children within the parent node does not matter
330-
tracing::trace!("appending child {node_id:?} to {parent_node_id:?}");
331-
parent_node_id.append(node_id, &mut self.tree);
332-
}
333-
334352
for change in ra_toml_changes {
335353
// turn and face the strain
336354
match change.change_kind {
337-
vfs::ChangeKind::Create => {
338-
let input = parse_toml(change.file_id, vfs, &mut scratch_errors, errors);
339-
let _new_node = self.update_toml(change.file_id, input);
340-
}
341-
vfs::ChangeKind::Modify => {
355+
vfs::ChangeKind::Create | vfs::ChangeKind::Modify => {
356+
if change.change_kind == vfs::ChangeKind::Create {
357+
parent_changes.entry(change.file_id).or_insert(ConfigParent::UserDefault);
358+
}
342359
let input = parse_toml(change.file_id, vfs, &mut scratch_errors, errors);
360+
tracing::trace!("updating toml for {:?} to {:?}", change.file_id, input);
361+
343362
if let Err(e) = self.update_toml(change.file_id, input) {
344363
errors.push(e);
345364
}
@@ -349,6 +368,18 @@ impl ConfigTree {
349368
}
350369
}
351370
}
371+
372+
for (file_id, parent) in parent_changes {
373+
let node_id = self.ensure_node(file_id);
374+
let parent_node_id = match parent {
375+
ConfigParent::Parent(parent_file_id) => self.ensure_node(parent_file_id),
376+
ConfigParent::UserDefault if file_id == self.xdg_config_file_id => continue,
377+
ConfigParent::UserDefault => self.xdg_config_node_id,
378+
};
379+
// order of children within the parent node does not matter
380+
tracing::trace!("appending child {node_id:?} to {parent_node_id:?}");
381+
parent_node_id.append(node_id, &mut self.tree);
382+
}
352383
}
353384
}
354385

@@ -365,6 +396,7 @@ mod tests {
365396
use vfs::{AbsPathBuf, VfsPath};
366397

367398
fn alloc_file_id(vfs: &mut Vfs, s: &str) -> FileId {
399+
tracing_subscriber::fmt().init();
368400
let abs_path = AbsPathBuf::try_from(PathBuf::new().join(s)).unwrap();
369401

370402
let vfs_path = VfsPath::from(abs_path);
@@ -391,7 +423,7 @@ mod tests {
391423
fn basic() {
392424
let mut vfs = Vfs::default();
393425
let xdg_config_file_id =
394-
alloc_file_id(&mut vfs, "/home/.config/rust-analyzer/rust-analyzer.toml");
426+
alloc_file_id(&mut vfs, "/home/username/.config/rust-analyzer/rust-analyzer.toml");
395427
let config_tree = ConcurrentConfigTree::new(ConfigTree::new(xdg_config_file_id));
396428

397429
let root = alloc_config(
@@ -415,8 +447,8 @@ mod tests {
415447
"#,
416448
);
417449

418-
let parent_changes =
419-
vec![ConfigParentChange { file_id: crate_a, parent: ConfigParent::Parent(root) }];
450+
let mut parent_changes = FxHashMap::default();
451+
parent_changes.insert(crate_a, ConfigParent::Parent(root));
420452

421453
let changes = ConfigChanges {
422454
// Normally you will filter these!
@@ -432,7 +464,6 @@ mod tests {
432464
};
433465

434466
dbg!(config_tree.apply_changes(changes, &vfs));
435-
dbg!(&config_tree);
436467

437468
let local = config_tree.read_config(crate_a).unwrap();
438469
// from root
@@ -442,6 +473,9 @@ mod tests {
442473
// from client
443474
assert_eq!(local.semanticHighlighting_strings_enable, false);
444475

476+
// --------------------------------------------------------
477+
478+
// Now let's modify the xdg_config_file_id, which should invalidate everything else
445479
vfs.set_file_id_contents(
446480
xdg_config_file_id,
447481
Some(
@@ -456,15 +490,14 @@ mod tests {
456490
);
457491

458492
let changes = ConfigChanges {
459-
ra_toml_changes: vfs.take_changes(),
460-
parent_changes: vec![],
493+
ra_toml_changes: dbg!(vfs.take_changes()),
494+
parent_changes: Default::default(),
461495
client_change: None,
462496
};
463497
dbg!(config_tree.apply_changes(changes, &vfs));
464498

465-
let local2 = config_tree.read_config(crate_a).unwrap();
466-
// should have recomputed
467-
assert!(!Arc::ptr_eq(&local, &local2));
499+
let prev = local;
500+
let local = config_tree.read_config(crate_a).unwrap();
468501

469502
assert_eq!(
470503
local.inlayHints_discriminantHints_enable,

0 commit comments

Comments
 (0)