diff --git a/crates/els/diagnostics.rs b/crates/els/diagnostics.rs index 327d21cad..b172bc1c7 100644 --- a/crates/els/diagnostics.rs +++ b/crates/els/diagnostics.rs @@ -34,10 +34,13 @@ impl Server { } else { "exec" }; - let mut checker = self.get_checker(path); - match checker.build(code.into(), mode) { + let mut checker = self.get_checker(path.clone()); + let artifact = match checker.build(code.into(), mode) { Ok(artifact) => { - send_log(format!("checking {uri} passed"))?; + send_log(format!( + "checking {uri} passed, found warns: {}", + artifact.warns.len() + ))?; let uri_and_diags = self.make_uri_and_diags(uri.clone(), artifact.warns.clone()); // clear previous diagnostics self.send_diagnostics(uri.clone().raw(), vec![])?; @@ -45,10 +48,7 @@ impl Server { send_log(format!("{uri}, warns: {}", diags.len()))?; self.send_diagnostics(uri, diags)?; } - if let Some(module) = self.get_ast(&uri) { - self.analysis_result - .insert(uri.clone(), AnalysisResult::new(module, artifact.into())); - } + artifact.into() } Err(artifact) => { send_log(format!("found errors: {}", artifact.errors.len()))?; @@ -67,12 +67,28 @@ impl Server { send_log(format!("{uri}, errs & warns: {}", diags.len()))?; self.send_diagnostics(uri, diags)?; } - if let Some(module) = self.get_ast(&uri) { - self.analysis_result - .insert(uri.clone(), AnalysisResult::new(module, artifact)); - } + artifact + } + }; + if let Some(shared) = self.get_shared() { + if mode == "declare" { + shared.py_mod_cache.register( + path, + artifact.object.clone(), + checker.get_context().unwrap().clone(), + ); + } else { + shared.mod_cache.register( + path, + artifact.object.clone(), + checker.get_context().unwrap().clone(), + ); } } + if let Some(module) = self.get_ast(&uri) { + self.analysis_result + .insert(uri.clone(), AnalysisResult::new(module, artifact)); + } if let Some(module) = checker.pop_context() { send_log(format!("{uri}: {}", module.context.name))?; self.modules.insert(uri.clone(), module); @@ -118,11 +134,12 @@ impl Server { for err in errors.into_iter() { let loc = err.core.get_loc_with_fallback(); let res_uri = if let Some(path) = err.input.path() { - Url::from_file_path(path) + Url::from_file_path(path.canonicalize().unwrap()) } else { Ok(uri.clone().raw()) }; let Ok(err_uri) = res_uri else { + crate::_log!("failed to get uri: {}", err.input.path().unwrap().display()); continue; }; let mut message = remove_style(&err.core.main_message); diff --git a/crates/els/server.rs b/crates/els/server.rs index 97182b129..454d32a41 100644 --- a/crates/els/server.rs +++ b/crates/els/server.rs @@ -314,6 +314,7 @@ pub struct Server, pub(crate) file_cache: FileCache, pub(crate) comp_cache: CompletionCache, + // TODO: remove modules, analysis_result, and add `shared: SharedCompilerResource` pub(crate) modules: ModuleCache, pub(crate) analysis_result: AnalysisResultCache, pub(crate) current_sig: Option, @@ -723,8 +724,7 @@ impl Server { pub(crate) fn get_checker(&self, path: PathBuf) -> Checker { if let Some(shared) = self.get_shared() { let shared = shared.clone(); - shared.mod_cache.remove(&path); - shared.py_mod_cache.remove(&path); + shared.clear(&path); Checker::inherit(self.cfg.inherit(path), shared) } else { Checker::new(self.cfg.inherit(path)) diff --git a/crates/erg_common/set.rs b/crates/erg_common/set.rs index 64bd95b4c..7aaf0ef6f 100644 --- a/crates/erg_common/set.rs +++ b/crates/erg_common/set.rs @@ -192,6 +192,12 @@ impl Set { self.elems.extend(other.elems); self } + + /// remove all elements for which the predicate returns false + #[inline] + pub fn retain(&mut self, f: impl FnMut(&T) -> bool) { + self.elems.retain(f); + } } impl Set { diff --git a/crates/erg_compiler/context/register.rs b/crates/erg_compiler/context/register.rs index 8dce4e04d..b67e885de 100644 --- a/crates/erg_compiler/context/register.rs +++ b/crates/erg_compiler/context/register.rs @@ -27,7 +27,6 @@ use ast::{ }; use erg_parser::ast; -use crate::artifact::ErrorArtifact; use crate::ty::constructors::{ free_var, func, func0, func1, proc, ref_, ref_mut, tp_enum, unknown_len_array_t, v_enum, }; @@ -225,7 +224,7 @@ impl Context { py_name, self.absolutize(ident.name.loc()), ); - self.index().register(&vi); + self.index().register(ident.inspect().clone(), &vi); self.future_defined_locals.insert(ident.name.clone(), vi); Ok(()) } @@ -271,7 +270,7 @@ impl Context { py_name, self.absolutize(sig.ident.name.loc()), ); - self.index().register(&vi); + self.index().register(sig.ident.inspect().clone(), &vi); if self .remove_class_attr(name) .is_some_and(|(_, decl)| !decl.kind.is_auto()) @@ -505,7 +504,7 @@ impl Context { None, self.absolutize(name.loc()), ); - self.index().register(&vi); + self.index().register(name.inspect().clone(), &vi); sig.vi = vi.clone(); self.params.push((Some(name.clone()), vi)); if errs.is_empty() { @@ -1272,7 +1271,7 @@ impl Context { None, self.absolutize(ident.name.loc()), ); - self.index().register(&vi); + self.index().register(ident.inspect().clone(), &vi); self.decls.insert(ident.name.clone(), vi); self.consts.insert(ident.name.clone(), other); Ok(()) @@ -1619,7 +1618,7 @@ impl Context { None, self.absolutize(name.loc()), ); - self.index().register(&vi); + self.index().register(name.inspect().clone(), &vi); self.decls.insert(name.clone(), vi); self.consts.insert(name.clone(), val); Ok(()) @@ -1667,7 +1666,7 @@ impl Context { None, self.absolutize(name.loc()), ); - self.index().register(&vi); + self.index().register(name.inspect().clone(), &vi); self.decls.insert(name.clone(), vi); self.consts.insert(name.clone(), val); self.register_methods(&t, &ctx); @@ -1727,7 +1726,7 @@ impl Context { None, self.absolutize(name.loc()), ); - self.index().register(&vi); + self.index().register(name.inspect().clone(), &vi); self.decls.insert(name.clone(), vi); self.consts.insert(name.clone(), val); self.register_methods(&t, &ctx); @@ -1940,8 +1939,7 @@ impl Context { Some(artifact.object), builder.pop_mod_ctx().unwrap(), ); - // shared.warns.extend(artifact.warns); - Ok(()) + shared.warns.extend(artifact.warns); } Err(artifact) => { if let Some(hir) = artifact.object { @@ -1949,9 +1947,8 @@ impl Context { .mod_cache .register(_path, Some(hir), builder.pop_mod_ctx().unwrap()); } - // shared.warns.extend(artifact.warns); - // shared.errors.extend(artifact.errors); - Err(ErrorArtifact::new(artifact.errors, artifact.warns)) + shared.warns.extend(artifact.warns); + shared.errors.extend(artifact.errors); } } }; @@ -2233,9 +2230,15 @@ impl Context { Ok(()) } - pub(crate) fn inc_ref(&self, vi: &VarInfo, name: &L, namespace: &Context) { + pub(crate) fn inc_ref( + &self, + name: &Str, + vi: &VarInfo, + loc: &L, + namespace: &Context, + ) { if let Some(index) = self.opt_index() { - index.inc_ref(vi, namespace.absolutize(name.loc())); + index.inc_ref(name, vi, namespace.absolutize(loc.loc())); } } @@ -2288,7 +2291,7 @@ impl Context { &self.cfg.input, self, ) { - self.inc_ref(&vi, &ident.name, namespace); + self.inc_ref(ident.inspect(), &vi, &ident.name, namespace); true } else { false @@ -2307,7 +2310,7 @@ impl Context { &self.cfg.input, self, ) { - self.inc_ref(&vi, &local.name, namespace); + self.inc_ref(local.inspect(), &vi, &local.name, namespace); true } else { &local.inspect()[..] == "module" || &local.inspect()[..] == "global" diff --git a/crates/erg_compiler/lint.rs b/crates/erg_compiler/lint.rs index 2a19633c1..99fc5ae71 100644 --- a/crates/erg_compiler/lint.rs +++ b/crates/erg_compiler/lint.rs @@ -161,19 +161,23 @@ impl ASTLowerer { } } - pub(crate) fn inc_ref(&self, vi: &VarInfo, name: &L) { - self.module.context.inc_ref(vi, name, &self.module.context); + pub(crate) fn inc_ref(&self, name: &Str, vi: &VarInfo, loc: &L) { + self.module + .context + .inc_ref(name, vi, loc, &self.module.context); } - pub(crate) fn warn_unused_vars(&mut self, mode: &str) { + pub(crate) fn warn_unused_local_vars(&mut self, mode: &str) { if mode == "eval" { return; } + let self_path = self.module.context.module_path(); for (referee, value) in self.module.context.index().members().iter() { - let code = referee.code(); - let name = code.as_ref().map(|s| &s[..]).unwrap_or(""); - let name_is_auto = - name == "_" || !Lexer::is_valid_start_symbol_ch(name.chars().next().unwrap_or(' ')); + if referee.module.as_deref() != self_path { + continue; + } + let name_is_auto = &value.name[..] == "_" + || !Lexer::is_valid_start_symbol_ch(value.name.chars().next().unwrap_or(' ')); if value.referrers.is_empty() && value.vi.vis.is_private() && !name_is_auto { let input = referee .module @@ -183,7 +187,7 @@ impl ASTLowerer { input, line!() as usize, referee.loc, - name, + &value.name, self.module.context.caused_by(), ); self.warns.push(warn); diff --git a/crates/erg_compiler/lower.rs b/crates/erg_compiler/lower.rs index 5af84f34e..1adc7af97 100644 --- a/crates/erg_compiler/lower.rs +++ b/crates/erg_compiler/lower.rs @@ -4,7 +4,7 @@ use std::mem; use erg_common::config::{ErgConfig, ErgMode}; -use erg_common::consts::{ERG_MODE, PYTHON_MODE}; +use erg_common::consts::{ELS, ERG_MODE, PYTHON_MODE}; use erg_common::dict; use erg_common::dict::Dict; use erg_common::error::{Location, MultiErrorDisplay}; @@ -659,7 +659,7 @@ impl ASTLowerer { VarInfo::ILLEGAL } }; - self.inc_ref(&vi, &attr.ident.name); + self.inc_ref(attr.ident.inspect(), &vi, &attr.ident.name); let ident = hir::Identifier::new(attr.ident, None, vi); let acc = hir::Accessor::Attr(hir::Attribute::new(obj, ident)); Ok(acc) @@ -728,7 +728,7 @@ impl ASTLowerer { .map(|ctx| ctx.first().unwrap().name.clone()), ) }; - self.inc_ref(&vi, &ident.name); + self.inc_ref(ident.inspect(), &vi, &ident.name); let ident = hir::Identifier::new(ident, __name__, vi); Ok(ident) } @@ -998,7 +998,7 @@ impl ASTLowerer { } } let attr_name = if let Some(attr_name) = call.attr_name { - self.inc_ref(&vi, &attr_name.name); + self.inc_ref(attr_name.inspect(), &vi, &attr_name.name); Some(hir::Identifier::new(attr_name, None, vi)) } else { if let hir::Expr::Call(call) = &obj { @@ -1251,13 +1251,21 @@ impl ASTLowerer { // suppress warns of lambda types, e.g. `(x: Int, y: Int) -> Int` if self.module.context.subtype_of(body.ref_t(), &Type::Type) { for param in params.non_defaults.iter() { - self.inc_ref(¶m.vi, param); + self.inc_ref(param.inspect().unwrap_or(&"_".into()), ¶m.vi, param); } if let Some(var_param) = params.var_params.as_deref() { - self.inc_ref(&var_param.vi, var_param); + self.inc_ref( + var_param.inspect().unwrap_or(&"_".into()), + &var_param.vi, + var_param, + ); } for default in params.defaults.iter() { - self.inc_ref(&default.sig.vi, &default.sig); + self.inc_ref( + default.sig.inspect().unwrap_or(&"_".into()), + &default.sig.vi, + &default.sig, + ); } } let (non_default_params, default_params): (Vec<_>, Vec<_>) = self @@ -2456,7 +2464,7 @@ impl ASTLowerer { log!(info "the AST lowering process has started."); log!(info "the type-checking process has started."); if let Some(path) = self.cfg.input.path() { - let graph = &self.module.context.shared.as_ref().unwrap().graph; + let graph = &self.module.context.shared().graph; graph.add_node_if_none(path); } let ast = ASTLinker::new(self.cfg.clone()) @@ -2511,8 +2519,19 @@ impl ASTLowerer { }; self.warn_implicit_union(&hir); self.warn_unused_expr(&hir.module, mode); - self.warn_unused_vars(mode); self.check_doc_comments(&hir); + if &self.module.context.name[..] == "" || ELS { + self.warn_unused_local_vars(mode); + if ELS { + self.module.context.shared().promises.join_children(); + } else { + self.module.context.shared().promises.join_all(); + } + let errs = self.module.context.shared().errors.take(); + let warns = self.module.context.shared().warns.take(); + self.errs.extend(errs); + self.warns.extend(warns); + } if self.errs.is_empty() { log!(info "the AST lowering process has completed."); Ok(CompleteArtifact::new( diff --git a/crates/erg_compiler/module/errors.rs b/crates/erg_compiler/module/errors.rs index 89c53ad86..de45fe29e 100644 --- a/crates/erg_compiler/module/errors.rs +++ b/crates/erg_compiler/module/errors.rs @@ -18,4 +18,10 @@ impl SharedCompileErrors { pub fn take(&self) -> CompileErrors { self.0.borrow_mut().take_all().into() } + + pub fn clear(&self) { + self.0.borrow_mut().clear(); + } } + +pub type SharedCompileWarnings = SharedCompileErrors; diff --git a/crates/erg_compiler/module/global.rs b/crates/erg_compiler/module/global.rs index 21428677a..bbff25a19 100644 --- a/crates/erg_compiler/module/global.rs +++ b/crates/erg_compiler/module/global.rs @@ -1,8 +1,11 @@ +use std::path::Path; + use erg_common::config::ErgConfig; use crate::context::Context; use super::cache::SharedModuleCache; +use super::errors::{SharedCompileErrors, SharedCompileWarnings}; use super::graph::SharedModuleGraph; use super::impls::SharedTraitImpls; use super::index::SharedModuleIndex; @@ -19,6 +22,8 @@ pub struct SharedCompilerResource { /// e.g. { "Named": [(Type, Named), (Func, Named), ...], "Add": [(Nat, Add(Nat)), (Int, Add(Int)), ...], ... } pub trait_impls: SharedTraitImpls, pub promises: SharedPromises, + pub errors: SharedCompileErrors, + pub warns: SharedCompileWarnings, } impl SharedCompilerResource { @@ -32,6 +37,8 @@ impl SharedCompilerResource { graph: SharedModuleGraph::new(), trait_impls: SharedTraitImpls::new(), promises: SharedPromises::new(), + errors: SharedCompileErrors::new(), + warns: SharedCompileWarnings::new(), }; Context::init_builtins(cfg, self_.clone()); self_ @@ -43,5 +50,14 @@ impl SharedCompilerResource { self.index.initialize(); self.graph.initialize(); self.trait_impls.initialize(); + self.errors.clear(); + self.warns.clear(); + } + + pub fn clear(&self, path: &Path) { + self.mod_cache.remove(path); + self.py_mod_cache.remove(path); + self.index.remove_path(path); + self.graph.remove(path); } } diff --git a/crates/erg_compiler/module/index.rs b/crates/erg_compiler/module/index.rs index 294bab940..0df12c152 100644 --- a/crates/erg_compiler/module/index.rs +++ b/crates/erg_compiler/module/index.rs @@ -6,6 +6,7 @@ use erg_common::dict::Dict; use erg_common::set; use erg_common::set::Set; use erg_common::shared::{MappedRwLockReadGuard, RwLockReadGuard, Shared}; +use erg_common::Str; use crate::varinfo::{AbsLocation, VarInfo}; @@ -25,8 +26,9 @@ impl<'a> Members<'a> { } } -#[derive(Debug, Clone, Default)] +#[derive(Debug, Clone)] pub struct ModuleIndexValue { + pub name: Str, pub vi: VarInfo, pub referrers: Set, } @@ -38,8 +40,12 @@ impl fmt::Display for ModuleIndexValue { } impl ModuleIndexValue { - pub const fn new(vi: VarInfo, referrers: Set) -> Self { - Self { vi, referrers } + pub const fn new(name: Str, vi: VarInfo, referrers: Set) -> Self { + Self { + name, + vi, + referrers, + } } pub fn push_ref(&mut self, referrer: AbsLocation) { @@ -65,19 +71,19 @@ impl ModuleIndex { } } - pub fn inc_ref(&mut self, vi: &VarInfo, referrer: AbsLocation) { + pub fn inc_ref(&mut self, name: &Str, vi: &VarInfo, referrer: AbsLocation) { let referee = vi.def_loc.clone(); if let Some(referrers) = self.members.get_mut(&referee) { referrers.push_ref(referrer); } else { - let value = ModuleIndexValue::new(vi.clone(), set! {referrer}); + let value = ModuleIndexValue::new(name.clone(), vi.clone(), set! {referrer}); self.members.insert(referee, value); } } - pub fn register(&mut self, vi: &VarInfo) { + pub fn register(&mut self, name: Str, vi: &VarInfo) { let referee = vi.def_loc.clone(); - let value = ModuleIndexValue::new(vi.clone(), set! {}); + let value = ModuleIndexValue::new(name, vi.clone(), set! {}); self.members.insert(referee, value); } @@ -90,8 +96,12 @@ impl ModuleIndex { } pub fn remove_path(&mut self, path: &Path) { - self.members - .retain(|loc, _| loc.module.as_deref() != Some(path)); + self.members.retain(|loc, value| { + value + .referrers + .retain(|ref_loc| ref_loc.module.as_deref() != Some(path)); + loc.module.as_deref() != Some(path) + }); } } @@ -109,12 +119,12 @@ impl SharedModuleIndex { Self(Shared::new(ModuleIndex::new())) } - pub fn inc_ref(&self, vi: &VarInfo, referrer: AbsLocation) { - self.0.borrow_mut().inc_ref(vi, referrer); + pub fn inc_ref(&self, name: &Str, vi: &VarInfo, referrer: AbsLocation) { + self.0.borrow_mut().inc_ref(name, vi, referrer); } - pub fn register(&self, vi: &VarInfo) { - self.0.borrow_mut().register(vi); + pub fn register(&self, name: Str, vi: &VarInfo) { + self.0.borrow_mut().register(name, vi); } pub fn get_refs( diff --git a/crates/erg_compiler/module/mod.rs b/crates/erg_compiler/module/mod.rs index 7ff725422..8b7a86736 100644 --- a/crates/erg_compiler/module/mod.rs +++ b/crates/erg_compiler/module/mod.rs @@ -1,4 +1,5 @@ pub mod cache; +pub mod errors; pub mod global; pub mod graph; pub mod impls; @@ -6,6 +7,7 @@ pub mod index; pub mod promise; pub use cache::*; +pub use errors::*; pub use global::*; pub use graph::*; pub use impls::*; diff --git a/crates/erg_compiler/module/promise.rs b/crates/erg_compiler/module/promise.rs index 1af69dab1..1d96f16be 100644 --- a/crates/erg_compiler/module/promise.rs +++ b/crates/erg_compiler/module/promise.rs @@ -1,22 +1,44 @@ use std::path::{Path, PathBuf}; -use std::thread::JoinHandle; +use std::thread::{current, JoinHandle, ThreadId}; use erg_common::dict::Dict; use erg_common::shared::Shared; -use crate::artifact::ErrorArtifact; - #[derive(Debug)] pub enum Promise { - Running(JoinHandle>), + Running { + parent: ThreadId, + handle: JoinHandle<()>, + }, Finished, } impl Promise { + pub fn running(handle: JoinHandle<()>) -> Self { + Self::Running { + parent: current().id(), + handle, + } + } + pub fn is_finished(&self) -> bool { match self { Self::Finished => true, - Self::Running(handle) => handle.is_finished(), + Self::Running { handle, .. } => handle.is_finished(), + } + } + + pub fn thread_id(&self) -> Option { + match self { + Self::Finished => None, + Self::Running { handle, .. } => Some(handle.thread().id()), + } + } + + pub fn parent_thread_id(&self) -> Option { + match self { + Self::Finished => None, + Self::Running { parent, .. } => Some(*parent), } } @@ -33,24 +55,52 @@ impl SharedPromises { Self::default() } - pub fn insert(&self, path: PathBuf, handle: JoinHandle>) { - self.0.borrow_mut().insert(path, Promise::Running(handle)); + pub fn insert(&self, path: PathBuf, handle: JoinHandle<()>) { + self.0.borrow_mut().insert(path, Promise::running(handle)); } pub fn is_registered(&self, path: &Path) -> bool { - self.0.borrow_mut().get(path).is_some() + self.0.borrow().get(path).is_some() } pub fn is_finished(&self, path: &Path) -> bool { self.0 - .borrow_mut() + .borrow() .get(path) .is_some_and(|promise| promise.is_finished()) } - pub fn join(&self, path: &Path) -> Result<(), ErrorArtifact> { + pub fn join(&self, path: &Path) -> std::thread::Result<()> { let promise = self.0.borrow_mut().get_mut(path).unwrap().take(); - let Promise::Running(handle) = promise else { todo!() }; - handle.join().unwrap() + let Promise::Running{ handle, .. } = promise else { todo!() }; + handle.join() + } + + pub fn join_children(&self) { + let cur_id = std::thread::current().id(); + let mut handles = vec![]; + for (_path, promise) in self.0.borrow_mut().iter_mut() { + if promise.parent_thread_id() != Some(cur_id) { + continue; + } + if let Promise::Running { handle, .. } = promise.take() { + handles.push(handle); + } + } + for handle in handles { + let _result = handle.join(); + } + } + + pub fn join_all(&self) { + let mut handles = vec![]; + for (_path, promise) in self.0.borrow_mut().iter_mut() { + if let Promise::Running { handle, .. } = promise.take() { + handles.push(handle); + } + } + for handle in handles { + let _result = handle.join(); + } } } diff --git a/tests/should_err/err_import.er b/tests/should_err/err_import.er new file mode 100644 index 000000000..8f0666047 --- /dev/null +++ b/tests/should_err/err_import.er @@ -0,0 +1 @@ +_ = import "addition" diff --git a/tests/test.rs b/tests/test.rs index e3facd442..f6f351c32 100644 --- a/tests/test.rs +++ b/tests/test.rs @@ -99,8 +99,8 @@ fn exec_impl() -> Result<(), ()> { #[test] fn exec_import() -> Result<(), ()> { - // 1 warn: a11y - expect_success("examples/import.er", 1) + // 2 warns: a11y + expect_success("examples/import.er", 2) } #[test] @@ -303,6 +303,11 @@ fn exec_dependent_err() -> Result<(), ()> { expect_failure("tests/should_err/dependent.er", 0, 5) } +#[test] +fn exec_err_import() -> Result<(), ()> { + expect_failure("tests/should_err/err_import.er", 0, 9) +} + /// This file compiles successfully, but causes a run-time error due to incomplete method dispatching #[test] fn exec_tests_impl() -> Result<(), ()> {