fix: Fix process-changes not deduplicating changes correctly

This commit is contained in:
Lukas Wirth 2023-01-25 14:46:06 +01:00
parent e6ad8a2edc
commit d712e52940
4 changed files with 30 additions and 15 deletions

View File

@ -121,7 +121,7 @@ pub fn spawn(
} }
pub fn load_dylib(&self, dylib: MacroDylib) -> Result<Vec<ProcMacro>, ServerError> { pub fn load_dylib(&self, dylib: MacroDylib) -> Result<Vec<ProcMacro>, ServerError> {
let _p = profile::span("ProcMacroClient::by_dylib_path"); let _p = profile::span("ProcMacroClient::load_dylib");
let macros = let macros =
self.process.lock().unwrap_or_else(|e| e.into_inner()).find_proc_macros(&dylib.path)?; self.process.lock().unwrap_or_else(|e| e.into_inner()).find_proc_macros(&dylib.path)?;

View File

@ -3,7 +3,7 @@
//! //!
//! Each tick provides an immutable snapshot of the state as `WorldSnapshot`. //! 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 crossbeam_channel::{unbounded, Receiver, Sender};
use flycheck::FlycheckHandle; 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 // 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 // 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 // create or modify which may cause problems later on
let mut collapsed_create_delete = false;
changed_files.dedup_by(|a, b| { changed_files.dedup_by(|a, b| {
use vfs::ChangeKind::*; use vfs::ChangeKind::*;
let has_collapsed_create_delete = mem::replace(&mut collapsed_create_delete, false);
if a.file_id != b.file_id { if a.file_id != b.file_id {
return false; 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 // duplicate can be merged
(Create, Create) | (Modify, Modify) | (Delete, Delete) => true, (Create, Create) | (Modify, Modify) | (Delete, Delete) => true,
// just leave the create, modify is irrelevant // just leave the create, modify is irrelevant
(Create, Modify) => { (Create, Modify) => true,
std::mem::swap(a, b); // 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 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 // this is equivalent to a modify
(Delete, Create) => { (Delete, Create) => {
a.change_kind = Modify; b.change_kind = Modify;
true true
} }
// can't really occur // can't really occur
@ -227,7 +239,9 @@ pub(crate) fn process_changes(&mut self) -> bool {
(Delete, Modify) => false, (Delete, Modify) => false,
} }
}); });
if collapsed_create_delete {
changed_files.pop();
}
for file in &changed_files { for file in &changed_files {
if let Some(path) = vfs.file_path(file.file_id).as_path() { if let Some(path) = vfs.file_path(file.file_id).as_path() {
let path = path.to_path_buf(); let path = path.to_path_buf();

View File

@ -362,7 +362,7 @@ fn eq_ignore_build_data<'a>(
let loader = &mut self.loader; let loader = &mut self.loader;
let mem_docs = &self.mem_docs; let mem_docs = &self.mem_docs;
let mut load = move |path: &AbsPath| { 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()); let vfs_path = vfs::VfsPath::from(path.to_path_buf());
if !mem_docs.contains(&vfs_path) { if !mem_docs.contains(&vfs_path) {
let contents = loader.handle.load_sync(path); let contents = loader.handle.load_sync(path);
@ -584,10 +584,10 @@ pub(crate) fn load_proc_macro(
path: &AbsPath, path: &AbsPath,
dummy_replace: &[Box<str>], dummy_replace: &[Box<str>],
) -> ProcMacroLoadResult { ) -> ProcMacroLoadResult {
let server = server.map_err(ToOwned::to_owned)?;
let res: Result<Vec<_>, String> = (|| { let res: Result<Vec<_>, String> = (|| {
let dylib = MacroDylib::new(path.to_path_buf()) let dylib = MacroDylib::new(path.to_path_buf())
.map_err(|io| format!("Proc-macro dylib loading failed: {io}"))?; .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}"))?; let vec = server.load_dylib(dylib).map_err(|e| format!("{e}"))?;
if vec.is_empty() { if vec.is_empty() {
return Err("proc macro library returned no proc macros".to_string()); return Err("proc macro library returned no proc macros".to_string());

View File

@ -75,6 +75,7 @@ pub struct Vfs {
} }
/// Changed file in the [`Vfs`]. /// Changed file in the [`Vfs`].
#[derive(Debug)]
pub struct ChangedFile { pub struct ChangedFile {
/// Id of the changed file /// Id of the changed file
pub file_id: FileId, pub file_id: FileId,
@ -161,9 +162,9 @@ pub fn set_file_contents(&mut self, path: VfsPath, contents: Option<Vec<u8>>) ->
let file_id = self.alloc_file_id(path); let file_id = self.alloc_file_id(path);
let change_kind = match (&self.get(file_id), &contents) { let change_kind = match (&self.get(file_id), &contents) {
(None, None) => return false, (None, None) => return false,
(Some(old), Some(new)) if old == new => return false,
(None, Some(_)) => ChangeKind::Create, (None, Some(_)) => ChangeKind::Create,
(Some(_), None) => ChangeKind::Delete, (Some(_), None) => ChangeKind::Delete,
(Some(old), Some(new)) if old == new => return false,
(Some(_), Some(_)) => ChangeKind::Modify, (Some(_), Some(_)) => ChangeKind::Modify,
}; };