From d712e529405b0ef5719c81ae620c88a97db78d93 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Wed, 25 Jan 2023 14:46:06 +0100 Subject: [PATCH] fix: Fix process-changes not deduplicating changes correctly --- crates/proc-macro-api/src/lib.rs | 2 +- crates/rust-analyzer/src/global_state.rs | 36 ++++++++++++++++-------- crates/rust-analyzer/src/reload.rs | 4 +-- crates/vfs/src/lib.rs | 3 +- 4 files changed, 30 insertions(+), 15 deletions(-) diff --git a/crates/proc-macro-api/src/lib.rs b/crates/proc-macro-api/src/lib.rs index 7921fda331e..52f976e4576 100644 --- a/crates/proc-macro-api/src/lib.rs +++ b/crates/proc-macro-api/src/lib.rs @@ -121,7 +121,7 @@ pub fn spawn( } pub fn load_dylib(&self, dylib: MacroDylib) -> Result, ServerError> { - let _p = profile::span("ProcMacroClient::by_dylib_path"); + let _p = profile::span("ProcMacroClient::load_dylib"); let macros = self.process.lock().unwrap_or_else(|e| e.into_inner()).find_proc_macros(&dylib.path)?; diff --git a/crates/rust-analyzer/src/global_state.rs b/crates/rust-analyzer/src/global_state.rs index c6f4e9ce07f..de11abdcf82 100644 --- a/crates/rust-analyzer/src/global_state.rs +++ b/crates/rust-analyzer/src/global_state.rs @@ -3,7 +3,7 @@ //! //! Each tick provides an immutable snapshot of the state as `WorldSnapshot`. -use std::{sync::Arc, time::Instant}; +use std::{mem, sync::Arc, time::Instant}; use crossbeam_channel::{unbounded, Receiver, Sender}; use flycheck::FlycheckHandle; @@ -197,29 +197,41 @@ pub(crate) fn process_changes(&mut self) -> bool { // We need to fix up the changed events a bit, if we have a create or modify for a file // id that is followed by a delete we actually no longer observe the file text from the // create or modify which may cause problems later on + let mut collapsed_create_delete = false; changed_files.dedup_by(|a, b| { use vfs::ChangeKind::*; + let has_collapsed_create_delete = mem::replace(&mut collapsed_create_delete, false); + if a.file_id != b.file_id { return false; } - match (a.change_kind, b.change_kind) { + // true => delete the second element (a), we swap them here as they are inverted by dedup_by + match (b.change_kind, a.change_kind) { // duplicate can be merged (Create, Create) | (Modify, Modify) | (Delete, Delete) => true, // just leave the create, modify is irrelevant - (Create, Modify) => { - std::mem::swap(a, b); + (Create, Modify) => true, + // modify becomes irrelevant if the file is deleted + (Modify, Delete) => { + mem::swap(a, b); + true + } + // Remove the create message, and in the following loop, also remove the delete + (Create, Delete) => { + collapsed_create_delete = true; + b.change_kind = Delete; + true + } + // trailing delete from earlier + (Delete, Create | Modify) if has_collapsed_create_delete => { + b.change_kind = Create; true } - // modify becomes irrelevant if the file is deleted - (Modify, Delete) => true, - // we should fully remove this occurrence, - // but leaving just a delete works as well - (Create, Delete) => true, // this is equivalent to a modify (Delete, Create) => { - a.change_kind = Modify; + b.change_kind = Modify; true } // can't really occur @@ -227,7 +239,9 @@ pub(crate) fn process_changes(&mut self) -> bool { (Delete, Modify) => false, } }); - + if collapsed_create_delete { + changed_files.pop(); + } for file in &changed_files { if let Some(path) = vfs.file_path(file.file_id).as_path() { let path = path.to_path_buf(); diff --git a/crates/rust-analyzer/src/reload.rs b/crates/rust-analyzer/src/reload.rs index 9bbce70ec0a..3d7342d1913 100644 --- a/crates/rust-analyzer/src/reload.rs +++ b/crates/rust-analyzer/src/reload.rs @@ -362,7 +362,7 @@ fn eq_ignore_build_data<'a>( let loader = &mut self.loader; let mem_docs = &self.mem_docs; let mut load = move |path: &AbsPath| { - let _p = profile::span("GlobalState::load"); + let _p = profile::span("switch_workspaces::load"); let vfs_path = vfs::VfsPath::from(path.to_path_buf()); if !mem_docs.contains(&vfs_path) { let contents = loader.handle.load_sync(path); @@ -584,10 +584,10 @@ pub(crate) fn load_proc_macro( path: &AbsPath, dummy_replace: &[Box], ) -> ProcMacroLoadResult { + let server = server.map_err(ToOwned::to_owned)?; let res: Result, String> = (|| { let dylib = MacroDylib::new(path.to_path_buf()) .map_err(|io| format!("Proc-macro dylib loading failed: {io}"))?; - let server = server.map_err(ToOwned::to_owned)?; let vec = server.load_dylib(dylib).map_err(|e| format!("{e}"))?; if vec.is_empty() { return Err("proc macro library returned no proc macros".to_string()); diff --git a/crates/vfs/src/lib.rs b/crates/vfs/src/lib.rs index c61f30387b7..14972d29074 100644 --- a/crates/vfs/src/lib.rs +++ b/crates/vfs/src/lib.rs @@ -75,6 +75,7 @@ pub struct Vfs { } /// Changed file in the [`Vfs`]. +#[derive(Debug)] pub struct ChangedFile { /// Id of the changed file pub file_id: FileId, @@ -161,9 +162,9 @@ pub fn set_file_contents(&mut self, path: VfsPath, contents: Option>) -> let file_id = self.alloc_file_id(path); let change_kind = match (&self.get(file_id), &contents) { (None, None) => return false, + (Some(old), Some(new)) if old == new => return false, (None, Some(_)) => ChangeKind::Create, (Some(_), None) => ChangeKind::Delete, - (Some(old), Some(new)) if old == new => return false, (Some(_), Some(_)) => ChangeKind::Modify, };