fix: nested module resolution bug

This commit is contained in:
Shunsuke Shibayama 2024-02-01 02:08:07 +09:00
parent 83aae24317
commit ded10fc3d9
7 changed files with 51 additions and 24 deletions

View file

@ -149,7 +149,7 @@ pub enum ResolveError {
}, },
} }
pub type ResolveResult<T> = Result<T, ResolveError>; pub type ResolveResult<T> = Result<T, Vec<ResolveError>>;
/// Resolve dependencies and build a package. /// Resolve dependencies and build a package.
/// This object should be a singleton. /// This object should be a singleton.
@ -387,7 +387,8 @@ impl<ASTBuilder: ASTBuildable, HIRBuilder: Buildable>
) -> Result<CompleteArtifact, IncompleteArtifact> { ) -> Result<CompleteArtifact, IncompleteArtifact> {
let cfg = self.cfg.copy(); let cfg = self.cfg.copy();
log!(info "Start dependency resolution process"); log!(info "Start dependency resolution process");
let _ = self.resolve(&mut ast, &cfg); let res = self.resolve(&mut ast, &cfg);
debug_assert!(res.is_ok(), "{:?}", res.unwrap_err());
log!(info "Dependency resolution process completed"); log!(info "Dependency resolution process completed");
log!("graph:\n{}", self.shared.graph.display()); log!("graph:\n{}", self.shared.graph.display());
if self.parse_errors.errors.is_empty() { if self.parse_errors.errors.is_empty() {
@ -411,47 +412,55 @@ impl<ASTBuilder: ASTBuildable, HIRBuilder: Buildable>
/// Analyze ASTs and make the dependencies graph. /// Analyze ASTs and make the dependencies graph.
/// If circular dependencies are found, inline submodules to eliminate the circularity. /// If circular dependencies are found, inline submodules to eliminate the circularity.
fn resolve(&mut self, ast: &mut AST, cfg: &ErgConfig) -> ResolveResult<()> { fn resolve(&mut self, ast: &mut AST, cfg: &ErgConfig) -> ResolveResult<()> {
let mut result = Ok(()); let mut errs = vec![];
for chunk in ast.module.iter_mut() { for chunk in ast.module.iter_mut() {
if let Err(err) = self.check_import(chunk, cfg) { if let Err(err) = self.check_import(chunk, cfg) {
result = Err(err); errs.extend(err);
} }
} }
result if errs.is_empty() {
Ok(())
} else {
Err(errs)
}
} }
fn check_import(&mut self, expr: &mut Expr, cfg: &ErgConfig) -> ResolveResult<()> { fn check_import(&mut self, expr: &mut Expr, cfg: &ErgConfig) -> ResolveResult<()> {
let mut result = Ok(()); let mut errs = vec![];
match expr { match expr {
Expr::Call(call) if call.additional_operation().is_some_and(|op| op.is_import()) => { Expr::Call(call) if call.additional_operation().is_some_and(|op| op.is_import()) => {
if let Err(err) = self.register(expr, cfg) { if let Err(err) = self.register(expr, cfg) {
result = Err(err); errs.extend(err);
} }
} }
Expr::Def(def) => { Expr::Def(def) => {
for expr in def.body.block.iter_mut() { for expr in def.body.block.iter_mut() {
if let Err(err) = self.check_import(expr, cfg) { if let Err(err) = self.check_import(expr, cfg) {
result = Err(err); errs.extend(err);
} }
} }
} }
Expr::Dummy(chunks) => { Expr::Dummy(chunks) => {
for chunk in chunks.iter_mut() { for chunk in chunks.iter_mut() {
if let Err(err) = self.check_import(chunk, cfg) { if let Err(err) = self.check_import(chunk, cfg) {
result = Err(err); errs.extend(err);
} }
} }
} }
Expr::Compound(chunks) => { Expr::Compound(chunks) => {
for chunk in chunks.iter_mut() { for chunk in chunks.iter_mut() {
if let Err(err) = self.check_import(chunk, cfg) { if let Err(err) = self.check_import(chunk, cfg) {
result = Err(err); errs.extend(err);
} }
} }
} }
_ => {} _ => {}
} }
result if errs.is_empty() {
Ok(())
} else {
Err(errs)
}
} }
fn analysis_in_progress(path: &Path) -> bool { fn analysis_in_progress(path: &Path) -> bool {
@ -588,10 +597,10 @@ impl<ASTBuilder: ASTBuildable, HIRBuilder: Buildable>
.is_err() .is_err()
{ {
self.submodules.push(from_path.clone()); self.submodules.push(from_path.clone());
return Err(ResolveError::CycleDetected { return Err(vec![ResolveError::CycleDetected {
path: import_path, path: import_path,
submod_input: cfg.input.clone(), submod_input: cfg.input.clone(),
}); }]);
} }
if import_path == from_path if import_path == from_path
|| self.submodules.contains(&import_path) || self.submodules.contains(&import_path)
@ -627,19 +636,20 @@ impl<ASTBuilder: ASTBuildable, HIRBuilder: Buildable>
} }
} }
}; };
if let Err(ResolveError::CycleDetected { path, submod_input }) = if let Err(mut errs) = self.resolve(&mut ast, &import_cfg) {
self.resolve(&mut ast, &import_cfg)
{
*expr = Expr::InlineModule(InlineModule::new( *expr = Expr::InlineModule(InlineModule::new(
Input::file(import_path.to_path_buf()), Input::file(import_path.to_path_buf()),
ast, ast,
call.clone(), call.clone(),
)); ));
if path != from_path { errs.retain(|err| match err {
return Err(ResolveError::CycleDetected { path, submod_input }); ResolveError::CycleDetected { path, .. } => path != &from_path,
} else { });
self.cyclic.push(path); if errs.is_empty() {
self.cyclic.push(from_path);
return Ok(()); return Ok(());
} else {
return Err(errs);
} }
} }
let prev = self.asts.insert(import_path, (__name__.clone(), ast)); let prev = self.asts.insert(import_path, (__name__.clone(), ast));

View file

@ -51,10 +51,12 @@ pub enum SubstituteResult {
} }
impl Context { impl Context {
/// Returns `true` if the module is being inspected, or has been inspected
pub(crate) fn mod_registered(&self, path: &NormalizedPathBuf) -> bool { pub(crate) fn mod_registered(&self, path: &NormalizedPathBuf) -> bool {
(self.shared.is_some() && self.promises().is_registered(path)) || self.mod_cached(path) (self.shared.is_some() && self.promises().is_registered(path)) || self.mod_cached(path)
} }
/// Returns `true` if the module has been inspected
pub(crate) fn mod_cached(&self, path: &Path) -> bool { pub(crate) fn mod_cached(&self, path: &Path) -> bool {
self.mod_cache().get(path).is_some() || self.py_mod_cache().get(path).is_some() self.mod_cache().get(path).is_some() || self.py_mod_cache().get(path).is_some()
} }

View file

@ -415,7 +415,9 @@ impl From<&Def> for ContextKind {
DefKind::Class | DefKind::Inherit => Self::Class, DefKind::Class | DefKind::Inherit => Self::Class,
DefKind::Trait | DefKind::Subsume => Self::Trait, DefKind::Trait | DefKind::Subsume => Self::Trait,
DefKind::StructuralTrait => Self::StructuralTrait, DefKind::StructuralTrait => Self::StructuralTrait,
DefKind::ErgImport | DefKind::PyImport | DefKind::RsImport => Self::Module, DefKind::ErgImport | DefKind::PyImport | DefKind::RsImport | DefKind::InlineModule => {
Self::Module
}
DefKind::Other => { DefKind::Other => {
if def.is_subr() { if def.is_subr() {
if def.sig.ident().unwrap().is_procedural() { if def.sig.ident().unwrap().is_procedural() {

View file

@ -10,7 +10,7 @@ use erg_common::dict;
use erg_common::dict::Dict; use erg_common::dict::Dict;
use erg_common::error::{Location, MultiErrorDisplay}; use erg_common::error::{Location, MultiErrorDisplay};
use erg_common::fresh::FreshNameGenerator; use erg_common::fresh::FreshNameGenerator;
use erg_common::pathutil::mod_name; use erg_common::pathutil::{mod_name, NormalizedPathBuf};
use erg_common::set; use erg_common::set;
use erg_common::set::Set; use erg_common::set::Set;
use erg_common::traits::{ExitStatus, Locational, NoTypeDisplay, Runnable, Stream}; use erg_common::traits::{ExitStatus, Locational, NoTypeDisplay, Runnable, Stream};
@ -2981,7 +2981,10 @@ impl<A: ASTBuildable> GenericASTLowerer<A> {
expect: Option<&Type>, expect: Option<&Type>,
) -> hir::Call { ) -> hir::Call {
log!(info "entered {}", fn_name!()); log!(info "entered {}", fn_name!());
let path = inline.input.path().to_path_buf(); let path = NormalizedPathBuf::from(inline.input.path().to_path_buf());
if self.module.context.mod_cached(&path) {
return self.lower_call(inline.import, expect);
}
let parent = self.get_mod_ctx().context.get_module().unwrap().clone(); let parent = self.get_mod_ctx().context.get_module().unwrap().clone();
let mod_ctx = ModuleContext::new(parent, dict! {}); let mod_ctx = ModuleContext::new(parent, dict! {});
let mod_name = mod_name(&path); let mod_name = mod_name(&path);

View file

@ -277,6 +277,10 @@ impl SharedModuleGraph {
self.0.borrow_mut().sort() self.0.borrow_mut().sort()
} }
pub fn entries(&self) -> Set<NormalizedPathBuf> {
self.0.borrow().iter().map(|n| n.id.clone()).collect()
}
pub fn initialize(&self) { pub fn initialize(&self) {
self.0.borrow_mut().initialize(); self.0.borrow_mut().initialize();
} }

View file

@ -1236,7 +1236,7 @@ impl LimitedDisplay for Type {
} }
Self::Poly { name, params } => { Self::Poly { name, params } => {
write!(f, "{name}(")?; write!(f, "{name}(")?;
if self.is_module() { if !DEBUG_MODE && self.is_module() {
// Module("path/to/module.er") -> Module("module.er") // Module("path/to/module.er") -> Module("module.er")
let name = params.first().unwrap().to_string_unabbreviated(); let name = params.first().unwrap().to_string_unabbreviated();
let name = name.replace("__init__.d.er", "").replace("__init__.er", ""); let name = name.replace("__init__.d.er", "").replace("__init__.er", "");

View file

@ -5522,6 +5522,7 @@ pub enum DefKind {
PyImport, PyImport,
RsImport, RsImport,
Patch, Patch,
InlineModule,
/// type alias included /// type alias included
Other, Other,
} }
@ -5556,6 +5557,10 @@ impl DefKind {
self.is_erg_import() || self.is_py_import() || self.is_rs_import() self.is_erg_import() || self.is_py_import() || self.is_rs_import()
} }
pub fn is_inline_module(&self) -> bool {
matches!(self, Self::InlineModule)
}
pub const fn is_other(&self) -> bool { pub const fn is_other(&self) -> bool {
matches!(self, Self::Other) matches!(self, Self::Other)
} }
@ -5607,6 +5612,7 @@ impl DefBody {
Some("rsimport") => DefKind::RsImport, Some("rsimport") => DefKind::RsImport,
_ => DefKind::Other, _ => DefKind::Other,
}, },
Expr::InlineModule(_) => DefKind::InlineModule,
_ => DefKind::Other, _ => DefKind::Other,
} }
} }