From 4740c6e4038599de6c11967414d4885dad5d25d6 Mon Sep 17 00:00:00 2001 From: Agent Smith Date: Fri, 18 Oct 2024 10:45:51 +0800 Subject: [PATCH] fix(lsp): cycle recovery panic issue & await context check - Avoid cycle recovery logic when cycle deps was detected by adding a thread local input cache for input of `compile_dry_file`. This help lsp to be more efficient and avoid a bug(?) in salsa causing panic after cycle recovery from time to time. - Updated salsa to the latest version to fix changing input causing panic bug (https://github.com/salsa-rs/salsa/issues/590). - Added an async helper function `spawn` which can start the task without directly await operation. This function return a new task which can be await later to get the result. - Update parser to support function def with inplicit return type. Those function's ret type will be treated as `void` - Add check logic for await context, now awaiting in none async function will report error as expected - Add finalizer support for gc, using gc finalizer to implicitly destroy the mutex (TODO: update condvar to use finalizer) --- Cargo.lock | 6 +-- immix | 2 +- planglib/std/chan.pi | 17 ++++---- planglib/std/mutex.pi | 31 ++++++++++--- planglib/std/task/delay.pi | 2 +- planglib/std/task/helper.pi | 63 +++++++++++++++++++++++++++ src/ast/compiler.rs | 7 ++- src/ast/diag.rs | 1 + src/ast/expects/test_diag.pi.expect | 16 +++---- src/ast/node/operator.rs | 11 +++++ src/ast/node/pkg.rs | 1 + src/ast/node/primary.rs | 1 + src/ast/node/program.rs | 30 +++++++++++-- src/ast/node/program/salsa_structs.rs | 3 +- src/lsp/mem_docs.rs | 33 +++++++++++++- src/nomparser/function.rs | 21 ++++++++- src/utils/read_config.rs | 2 +- test/lsp_diag/test_diag.pi | 2 +- test/main.pi | 5 +-- test/test/std_test.pi | 16 +++++-- vm/src/mutex/mod.rs | 13 +++--- 21 files changed, 230 insertions(+), 53 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 86599b5ed..e8a805941 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1758,7 +1758,7 @@ checksum = "f3cb5ba0dc43242ce17de99c180e96db90b235b8a9fdc9543c96d2209116bd9f" [[package]] name = "salsa" version = "0.18.0" -source = "git+https://github.com/salsa-rs/salsa?branch=master#2af849b9a911a72da699df21c48e2f12c211d0e7" +source = "git+https://github.com/salsa-rs/salsa?branch=master#254c749b02cde2fd29852a7463a33e800b771758" dependencies = [ "append-only-vec", "arc-swap", @@ -1778,12 +1778,12 @@ dependencies = [ [[package]] name = "salsa-macro-rules" version = "0.1.0" -source = "git+https://github.com/salsa-rs/salsa?branch=master#2af849b9a911a72da699df21c48e2f12c211d0e7" +source = "git+https://github.com/salsa-rs/salsa?branch=master#254c749b02cde2fd29852a7463a33e800b771758" [[package]] name = "salsa-macros" version = "0.18.0" -source = "git+https://github.com/salsa-rs/salsa?branch=master#2af849b9a911a72da699df21c48e2f12c211d0e7" +source = "git+https://github.com/salsa-rs/salsa?branch=master#254c749b02cde2fd29852a7463a33e800b771758" dependencies = [ "heck", "proc-macro2", diff --git a/immix b/immix index dfcab4278..1d862e2da 160000 --- a/immix +++ b/immix @@ -1 +1 @@ -Subproject commit dfcab42780725417ff1d4aaa1fb03db96428bfc4 +Subproject commit 1d862e2da97be9754ea96ed7e4318aaa7e3eb58c diff --git a/planglib/std/chan.pi b/planglib/std/chan.pi index de09908be..eaddccb12 100644 --- a/planglib/std/chan.pi +++ b/planglib/std/chan.pi @@ -3,7 +3,7 @@ pub struct Chan { buffer: Queue; count: u64; capacity: u64; - mtx: *Mutex; + mtx: **MutexHandle; condvar: *CondVar; } struct Node { @@ -47,16 +47,17 @@ pub fn channel(sz: u64) Chan { ch.buffer = Queue{}; ch.count = 0; ch.capacity = sz; - create_mutex(&ch.mtx); + ch.mtx = &&MutexHandle{}; + create_mutex(ch.mtx); create_condvar(&ch.condvar); return ch; } use std::io; impl Chan { pub fn send(s: S) void { - lock_mutex(self.mtx); + lock_mutex(*self.mtx); while self.count == self.capacity { - condvar_wait(self.condvar, self.mtx); + condvar_wait(self.condvar, *self.mtx); } self.buffer.push(s); // io::printi64ln(self.count as i64); @@ -64,13 +65,13 @@ impl Chan { if self.count == 1 { condvar_notify(self.condvar); } - unlock_mutex(self.mtx); + unlock_mutex(*self.mtx); return; } pub fn recv() S { - lock_mutex(self.mtx); + lock_mutex(*self.mtx); while self.count == 0 { - condvar_wait(self.condvar, self.mtx); + condvar_wait(self.condvar, *self.mtx); } let s = self.buffer.pop(); self.count = self.count - 1; @@ -79,7 +80,7 @@ impl Chan { condvar_notify(self.condvar); } - unlock_mutex(self.mtx); + unlock_mutex(*self.mtx); return s; } pub fn len() u64 { diff --git a/planglib/std/mutex.pi b/planglib/std/mutex.pi index ee3c64cb7..00145b56b 100644 --- a/planglib/std/mutex.pi +++ b/planglib/std/mutex.pi @@ -1,21 +1,40 @@ -pub struct Mutex{} +pub struct MutexHandle{} -pub fn create_mutex(mutex: **Mutex) u64; -pub fn lock_mutex(mutex: *Mutex) u64; +pub struct Mutex{ + handle: **MutexHandle; +} +pub fn create_mutex(mutex: **MutexHandle) u64; +pub fn lock_mutex(mutex: *MutexHandle) u64; +pub fn new_mutex() Mutex{ + let m = Mutex{}; + m.handle = &&MutexHandle{}; + create_mutex(m.handle); + return m; +} -pub fn unlock_mutex(mutex: *Mutex) u64; +impl Mutex{ + pub fn lock() u64{ + return lock_mutex(*self.handle); + } + + pub fn unlock() u64{ + return unlock_mutex(*self.handle); + } +} + + +pub fn unlock_mutex(mutex: *MutexHandle) u64; -pub fn drop_mutex(mutex: *Mutex) u64; pub struct CondVar{} pub fn create_condvar(condvar: **CondVar) u64; -pub fn condvar_wait(condvar: *CondVar, mutex: *Mutex) u64; +pub fn condvar_wait(condvar: *CondVar, mutex: *MutexHandle) u64; pub fn condvar_notify(condvar: *CondVar) u64; diff --git a/planglib/std/task/delay.pi b/planglib/std/task/delay.pi index 8665de0ba..d729d2a90 100644 --- a/planglib/std/task/delay.pi +++ b/planglib/std/task/delay.pi @@ -4,7 +4,7 @@ use std::task::executor; -pub struct DelayTask { +struct DelayTask { first:bool; ready:bool; delay:u64; diff --git a/planglib/std/task/helper.pi b/planglib/std/task/helper.pi index 332feb12a..5eea1d331 100644 --- a/planglib/std/task/helper.pi +++ b/planglib/std/task/helper.pi @@ -2,6 +2,7 @@ use std::task::Task; use std::task::executor; use std::task::reactor; use std::thread; +use std::mutex; pub fn spawn_async_main(t:Task) void { @@ -19,3 +20,65 @@ pub fn spawn_async_main(t:Task) void { reactor::start_global_reactor(); return; } + + +struct AdapterTask { + task:Task; + wk:||=>void; + ready:bool; + mtx:mutex::Mutex; +} + +impl Task for AdapterTask { + fn poll(wk:||=>void) Option { + if self.ready { + let r = self.task.poll(|| => { + return; + }); + return r; + } + self.mtx.lock(); + if !self.ready { + self.wk = ||=>{ + self.mtx.unlock(); + wk(); + return; + }; + } else { + self.mtx.unlock(); + wk(); + let r = self.task.poll(|| => { + return; + }); + return r; + } + self.mtx.unlock(); + let r = self.task.poll(|| => { + return; + }); + return r; + } +} + + +pub fn spawn(tk:Task) Task { + let t = AdapterTask { + task:tk, + ready:false, + mtx:mutex::new_mutex(), + }; + t.wk = ||=>{ + t.mtx.unlock(); + return; + }; + tk.poll(||=>{ + t.mtx.lock(); + t.ready = true; + t.wk(); + + return; + }); + return t as Task; +} + + diff --git a/src/ast/compiler.rs b/src/ast/compiler.rs index 92897c016..64ad32678 100644 --- a/src/ast/compiler.rs +++ b/src/ast/compiler.rs @@ -3,6 +3,7 @@ use super::node::program::ModWrapper; use super::node::program::ASSET_PATH; #[cfg(feature = "llvm")] use crate::ast::jit_config::IS_JIT; +use crate::lsp::mem_docs::COMPILE_INPUT_CACHE; use crate::{ ast::{accumulators::ModBuffer, node::program::Program}, lsp::mem_docs::{FileCompileInput, MemDocsInput}, @@ -122,8 +123,11 @@ pub fn compile_dry<'db>(db: &'db dyn Db, docs: MemDocsInput) -> Result( parser_entry.docs(db), parser_entry.config(db), parser_entry.opt(db), + parser_entry.parent_mods(db), ); log::trace!("entering emit"); Some(program.emit(db)) diff --git a/src/ast/diag.rs b/src/ast/diag.rs index 6c5353d04..59878fc18 100644 --- a/src/ast/diag.rs +++ b/src/ast/diag.rs @@ -181,6 +181,7 @@ define_diag!( GENERATOR_FUNCTION_CANNOT_RETURN_VOID = "generator function cannot return void", MACRO_EXPANSION_FAILED = "macro expansion failed", SYNTAX_ERROR_FUNC_PARAM = "syntax error: function parameter", + ONLY_AWAIT_IN_ASYNC_FN = "await is only expected in async functions", ); define_diag! { diff --git a/src/ast/expects/test_diag.pi.expect b/src/ast/expects/test_diag.pi.expect index 9ae7291d6..edfffd908 100644 --- a/src/ast/expects/test_diag.pi.expect +++ b/src/ast/expects/test_diag.pi.expect @@ -529,12 +529,12 @@ start: Pos { line: 163, column: 17, - offset: 1709, + offset: 1704, }, end: Pos { line: 163, column: 18, - offset: 1710, + offset: 1705, }, }, }, @@ -901,8 +901,8 @@ }, end: Pos { line: 1, - column: 13, - offset: 12, + column: 20, + offset: 19, }, }, }, @@ -1527,12 +1527,12 @@ start: Pos { line: 163, column: 20, - offset: 1712, + offset: 1707, }, end: Pos { line: 163, column: 21, - offset: 1713, + offset: 1708, }, }, }, @@ -1542,12 +1542,12 @@ start: Pos { line: 163, column: 20, - offset: 1712, + offset: 1707, }, end: Pos { line: 163, column: 21, - offset: 1713, + offset: 1708, }, }, }, diff --git a/src/ast/node/operator.rs b/src/ast/node/operator.rs index 224f43390..c3be9af75 100644 --- a/src/ast/node/operator.rs +++ b/src/ast/node/operator.rs @@ -74,6 +74,17 @@ impl Node for UnaryOpNode { .new_output(pltype.clone()), (_, TokenType::BIT_NOT) => builder.build_bit_not(exp).new_output(pltype.clone()), (_, TokenType::AWAIT) => { + if !ctx + .generator_data + .as_ref() + .map(|x| x.borrow().generator_type == GeneratorType::Async) + .unwrap_or_default() + { + self.range + .new_err(ErrorCode::ONLY_AWAIT_IN_ASYNC_FN) + .add_label(exp_range, ctx.get_file(), None) + .add_to_ctx(ctx); + } let poll_ty = match &*pltype.borrow() { PLType::Trait(st) => { if !st.name.starts_with("Task") { diff --git a/src/ast/node/pkg.rs b/src/ast/node/pkg.rs index e62cbea6c..2a6a861a2 100644 --- a/src/ast/node/pkg.rs +++ b/src/ast/node/pkg.rs @@ -213,6 +213,7 @@ impl Node for UseNode { /// /// TODO: 区分该节点与ExternTypeName节点，该节点不生成类型，只生成函数与变量/常量 #[node] +#[derive(Default)] pub struct ExternIdNode { /// namespace refers to the namespace of an identifier /// it might be empty diff --git a/src/ast/node/primary.rs b/src/ast/node/primary.rs index e114a473f..1d62bf2bb 100644 --- a/src/ast/node/primary.rs +++ b/src/ast/node/primary.rs @@ -115,6 +115,7 @@ impl Node for NumNode { } #[node] +#[derive(Default)] pub struct VarNode { /// identifier name of a symbol, which could be either a variable or a type pub name: Ustr, diff --git a/src/ast/node/program.rs b/src/ast/node/program.rs index 400994a79..2f3bf9770 100644 --- a/src/ast/node/program.rs +++ b/src/ast/node/program.rs @@ -21,6 +21,7 @@ use crate::ast::pltype::add_primitive_types; use crate::ast::pltype::FNValue; use crate::ast::tokens::TokenType; use crate::flow::display::Dot; +use crate::format_label; use crate::lsp::semantic_tokens::SemanticTokensBuilder; use crate::lsp::text; #[cfg(feature = "repl")] @@ -274,8 +275,13 @@ impl<'db> Program<'db> { } else { ("检查", &CHECK_PROGRESS as &ProgressBar) }; + let parents = self.parent_mods(db); // parse all dependencies into modules and process symbols into the main module symbol table for (i, u) in entry_node.uses.iter().enumerate() { + let mut parents = parents.clone(); + parents + .0 + .insert(self.params(db).file(db).to_string(), u.range()); #[cfg(not(target_arch = "wasm32"))] pb.set_message(format!( "正在{}包{}的依赖项{}/{}", @@ -312,9 +318,23 @@ impl<'db> Program<'db> { let mut mod_id = wrapper.use_node(db).get_last_id(); let dep_path_str = dep_path.to_str().unwrap().to_string(); - let mut dep_parser_entry = - self.docs(db) - .finalize_parser_input(db, dep_path_str.clone(), false); + if parents.0.contains_key(&dep_path_str) { + let mut diag = u.range().new_err(ErrorCode::CYCLE_DEPENDENCY); + diag.set_source(&dep_path_str); + for (f, r) in parents.0.iter() { + let msg = "import in cycle here"; + diag.add_label(*r, ustr(f), format_label!(msg)); + } + Diagnostics((dep_path_str.clone(), vec![diag])).accumulate(db); + continue; + } + parents.0.insert(dep_path_str.clone(), u.range()); + let mut dep_parser_entry = self.docs(db).finalize_parser_input( + db, + dep_path_str.clone(), + false, + parents.clone(), + ); #[cfg(target_arch = "wasm32")] if dep_path_str.starts_with("core") || dep_path_str.starts_with("std") { @@ -329,7 +349,9 @@ impl<'db> Program<'db> { if let Some(p) = dep_path.parent() { mod_id = Some(p.file_name().unwrap().to_str().unwrap().to_string().into()); let file = p.with_extension("pi").to_str().unwrap().to_string(); - dep_parser_entry = self.docs(db).finalize_parser_input(db, file, false); + dep_parser_entry = self + .docs(db) + .finalize_parser_input(db, file, false, parents); symbol_opt = Some( dep_path .with_extension("") diff --git a/src/ast/node/program/salsa_structs.rs b/src/ast/node/program/salsa_structs.rs index ec09b5340..414751a3d 100644 --- a/src/ast/node/program/salsa_structs.rs +++ b/src/ast/node/program/salsa_structs.rs @@ -7,7 +7,7 @@ use crate::utils::read_config::Config; use crate::ast::plmod::{GlobalType, Mod}; -use crate::lsp::mem_docs::{EmitParams, MemDocsInput}; +use crate::lsp::mem_docs::{EmitParams, MemDocsInput, ParentMods}; use rustc_hash::FxHashMap; use std::cell::RefCell; use std::sync::Arc; @@ -108,4 +108,5 @@ pub struct Program<'db> { /// config is all neccessary information to represent a program pub config: Config, pub opt: HashOptimizationLevel, + pub parent_mods: ParentMods, } diff --git a/src/lsp/mem_docs.rs b/src/lsp/mem_docs.rs index cd81499cb..096e9d7f7 100644 --- a/src/lsp/mem_docs.rs +++ b/src/lsp/mem_docs.rs @@ -1,16 +1,18 @@ use std::{ + cell::RefCell, fs::read_to_string, path::PathBuf, sync::{Arc, Mutex}, }; +use linked_hash_map::LinkedHashMap; use log::debug; use rustc_hash::FxHashMap; use crate::{ ast::{ compiler::{ActionType, HashOptimizationLevel, Options}, - range::Pos, + range::{Pos, Range}, }, nomparser::SourceProgram, utils::read_config::{prepare_build_envs, search_config_file, Config}, @@ -51,6 +53,9 @@ pub struct MemDocsInput { pub edit_pos: Option, } +#[derive(Debug, Clone, Default, PartialEq, Eq, Hash)] +pub struct ParentMods(pub LinkedHashMap); + /// FileCompileInput holds the information to parse a program through the entry of file, /// with processed dependencies required by the file according to its kagari.toml #[salsa::tracked] @@ -65,13 +70,14 @@ pub struct FileCompileInput<'db> { pub docs: MemDocsInput, pub config: Config, pub opt: HashOptimizationLevel, + pub parent_mods: ParentMods, } #[salsa::tracked] impl<'db> FileCompileInput<'db> { #[salsa::tracked] // get_file_content gets the file content from cache or reads from file with the self.file - pub fn get_file_content(self, db: &dyn Db) -> Option { + pub fn get_file_content(self, db: &'db dyn Db) -> Option { // let f = self.file(db); // eprintln!("get_file_content {}", f); let re = self @@ -125,6 +131,10 @@ impl<'db> FileCompileInput<'db> { } } +thread_local! { + pub static COMPILE_INPUT_CACHE: RefCell> = Default::default(); +} + #[salsa::tracked] impl MemDocsInput { #[salsa::tracked] @@ -155,6 +165,7 @@ impl MemDocsInput { db: &dyn Db, entry_file: String, override_with_kagari_entry: bool, + parent_mods: ParentMods, ) -> Option> { let mut final_entry_file: String; let re_entry_file = crate::utils::canonicalize(entry_file); @@ -194,6 +205,23 @@ impl MemDocsInput { if override_with_kagari_entry { final_entry_file = config.entry.clone(); } + if let Some(cached) = + COMPILE_INPUT_CACHE.with(|cache| cache.borrow().get(&final_entry_file).cloned()) + { + if cached != parent_mods { + return self.finalize_parser_input( + db, + final_entry_file, + override_with_kagari_entry, + cached, + ); + } + } + COMPILE_INPUT_CACHE.with(|cache| { + cache + .borrow_mut() + .insert(final_entry_file.clone(), parent_mods.clone()); + }); let buf = crate::utils::canonicalize(PathBuf::from(kagari_path.clone())).unwrap(); let parant = buf.parent().unwrap(); Some(FileCompileInput::new( @@ -203,6 +231,7 @@ impl MemDocsInput { self, config, self.op(db).optimization, + parent_mods, )) } } diff --git a/src/nomparser/function.rs b/src/nomparser/function.rs index a924f72cd..52bfceea9 100644 --- a/src/nomparser/function.rs +++ b/src/nomparser/function.rs @@ -7,7 +7,11 @@ use nom::{ IResult, }; -use crate::ast::{diag::ErrorCode, node::function::FuncDefNode, tokens::TokenType}; +use crate::ast::{ + diag::ErrorCode, + node::{function::FuncDefNode, pkg::ExternIdNode, types::TypeNameNode}, + tokens::TokenType, +}; use crate::{ast::node::function::GeneratorType, nomparser::Span}; use internal_macro::{test_parser, test_parser_error}; @@ -111,7 +115,7 @@ pub fn function_def(input: Span) -> IResult> { ), )), tag_token_symbol(TokenType::RPAREN), - type_name, + opt(type_name), opt(del_newline_or_space!(preceded( tag_token_symbol(TokenType::WHERE), err_tolerable_seplist0( @@ -151,6 +155,19 @@ pub fn function_def(input: Span) -> IResult> { precoms.push(Box::new(NodeEnum::Comment(com))); } } + let ret = ret.unwrap_or(Box::new(TypeNodeEnum::Basic(TypeNameNode { + id: Some(ExternIdNode { + id: Box::new(VarNode { + name: "void".into(), + ..Default::default() + }), + complete: true, + ..Default::default() + }), + range: Range::default(), + generic_params: None, + generic_infer: None, + }))); let node = FuncDefNode { id: function_identifier, paralist: paras, diff --git a/src/utils/read_config.rs b/src/utils/read_config.rs index 92a1bff8f..824c248b8 100644 --- a/src/utils/read_config.rs +++ b/src/utils/read_config.rs @@ -107,7 +107,7 @@ impl<'db> ConfigWrapper<'db> { /// ``` /// we will use `project1` to resolve the path of the project #[salsa::tracked] - pub(crate) fn resolve_dep_path(self, db: &dyn Db) -> PathBuf { + pub(crate) fn resolve_dep_path(self, db: &'db dyn Db) -> PathBuf { let u = self.use_node(db); let mut path = PathBuf::from(self.config(db).root.clone()); if let Some(cm) = &self.config(db).deps { diff --git a/test/lsp_diag/test_diag.pi b/test/lsp_diag/test_diag.pi index fa1674340..4fd90d718 100644 --- a/test/lsp_diag/test_diag.pi +++ b/test/lsp_diag/test_diag.pi @@ -155,7 +155,7 @@ fn test_f_diag1(a,) void { return; } -fn test_f_diag2(,) void { +fn test_f_diag2(,) { return; } diff --git a/test/main.pi b/test/main.pi index b458e6afc..b59269808 100644 --- a/test/main.pi +++ b/test/main.pi @@ -35,7 +35,6 @@ use project1::test::_hashtable; use project1::test::_io; use project1::test::future_test; use project1::test::std_test; -use std::task::delay; @@ -74,13 +73,11 @@ async fn main() Task<()> { _io::test_io(); future_test::test_future(); std_test::test_std(); - println!("async 1"); - await delay::delay(2000 as u64); - println!("async 1 end"); await std_test::test_nested_async_closure(); await std_test::test_nested_async_closure_in_normal_f(); await std_test::test_nested_async_closure_in_normal_f2(); + await std_test::test_delay(); return (); } diff --git a/test/test/std_test.pi b/test/test/std_test.pi index 6a6437591..65cc26431 100644 --- a/test/test/std_test.pi +++ b/test/test/std_test.pi @@ -4,6 +4,7 @@ use std::math::F64Ext; use std::task::Task; use std::io; use project1::test::iter; +use std::task::delay; @@ -101,8 +102,7 @@ pub fn test_std() void { return; }); panic::assert(b == 100); - fn1(test{a:9999}); - + fn1(test{a:9999}); return; } @@ -191,4 +191,14 @@ struct test { fn fn1(t:test) void { panic::assert(t.a == 9999); return; -} \ No newline at end of file +} + +pub async fn test_delay() Task<()> { + println!("async 1"); + let d = spawn(delay::delay(2000 as u64)); + await delay::delay(2000 as u64); + println!("async 1 end"); + await d; + println!("async 2 end"); + return (); +} diff --git a/vm/src/mutex/mod.rs b/vm/src/mutex/mod.rs index fba5634d3..a2afe11a0 100644 --- a/vm/src/mutex/mod.rs +++ b/vm/src/mutex/mod.rs @@ -21,6 +21,12 @@ fn create_mutex(mutex: *mut *mut OpaqueMutex) -> u64 { guard: Cell::new(None), })) .cast(); + fn drop_mutex_f(mutex: *mut u8) { + unsafe { + drop(Box::from_raw(mutex.cast::())); + } + } + immix::gc_register_finalizer(mutex as _, (*mutex) as _, drop_mutex_f); 0 } @@ -45,13 +51,6 @@ fn unlock_mutex(mutex: *mut OpaqueMutex) -> u64 { 0 } -#[is_runtime] -fn drop_mutex(mutex: *mut OpaqueMutex) -> u64 { - unlock_mutex(mutex); - drop(Box::from_raw(mutex.cast::())); - 0 -} - #[is_runtime] fn create_condvar(cv: *mut *mut Condvar) -> u64 { let condvar = Box::leak(Box::new(Condvar::new()));