diff --git a/crates/rune/src/compile/mod.rs b/crates/rune/src/compile/mod.rs index d2760cdb8..c6acfec45 100644 --- a/crates/rune/src/compile/mod.rs +++ b/crates/rune/src/compile/mod.rs @@ -127,6 +127,8 @@ pub(crate) fn compile( loop { while let Some(entry) = worker.q.next_build_entry() { + tracing::trace!("next build entry: {}", entry.item.item); + let source_id = entry.location.source_id; let task = CompileBuildEntry { @@ -184,6 +186,7 @@ impl CompileBuildEntry<'_> { } } + #[tracing::instrument(skip(self, entry))] fn compile(mut self, entry: BuildEntry) -> Result<(), CompileError> { let BuildEntry { item, @@ -196,6 +199,8 @@ impl CompileBuildEntry<'_> { match build { Build::Function(f) => { + tracing::trace!("function: {}", item.item); + use self::v1::assemble; let args = @@ -221,6 +226,8 @@ impl CompileBuildEntry<'_> { } } Build::InstanceFunction(f) => { + tracing::trace!("instance function: {}", item.item); + use self::v1::assemble; let args = @@ -256,6 +263,8 @@ impl CompileBuildEntry<'_> { } } Build::Closure(closure) => { + tracing::trace!("closure: {}", item.item); + use self::v1::assemble; let span = closure.ast.span(); @@ -283,6 +292,8 @@ impl CompileBuildEntry<'_> { } } Build::AsyncBlock(b) => { + tracing::trace!("async block: {}", item.item); + use self::v1::assemble; let args = b.captures.len(); @@ -306,12 +317,16 @@ impl CompileBuildEntry<'_> { } } Build::Unused => { + tracing::trace!("unused: {}", item.item); + if !item.visibility.is_public() { self.diagnostics .not_used(location.source_id, location.span, None); } } Build::Import(import) => { + tracing::trace!("import: {}", item.item); + // Issue the import to check access. let result = self .q @@ -341,6 +356,8 @@ impl CompileBuildEntry<'_> { } } Build::ReExport => { + tracing::trace!("re-export: {}", item.item); + let import = match self .q .import(location.span, &item.module, &item.item, used)? diff --git a/crates/rune/src/diagnostics/emit_diagnostics.rs b/crates/rune/src/diagnostics/emit_diagnostics.rs index 1f0e8b866..4658b57be 100644 --- a/crates/rune/src/diagnostics/emit_diagnostics.rs +++ b/crates/rune/src/diagnostics/emit_diagnostics.rs @@ -496,22 +496,10 @@ where format_ir_error(this, sources, error_span, error, labels, notes)?; } QueryErrorKind::ImportCycle { path } => { - let mut it = path.iter(); - let last = it.next_back(); - - for (step, entry) in (1..).zip(it) { - labels.push( - d::Label::secondary(entry.location.source_id, entry.location.span.range()) - .with_message(format!("step #{} for `{}`", step, entry.item)), - ); - } - - if let Some(entry) = last { - labels.push( - d::Label::secondary(entry.location.source_id, entry.location.span.range()) - .with_message(format!("final step cycling back to `{}`", entry.item)), - ); - } + diagnose_import_path(&mut labels, path); + } + QueyrErrorKind::ImportRecursionLimit { path, .. } => { + diagnose_import_path(&mut labels, path); } QueryErrorKind::ItemConflict { other: Location { source_id, span }, @@ -594,6 +582,28 @@ where ) -> fmt::Result { Ok(()) } + + fn diagnose_import_path( + labels: &mut Vec>, + path: &[ImportStep], + ) { + let mut it = path.iter(); + let last = it.next_back(); + + for (step, entry) in (1..).zip(it) { + labels.push( + d::Label::secondary(entry.location.source_id, entry.location.span.range()) + .with_message(format!("step #{} for `{}`", step, entry.item)), + ); + } + + if let Some(entry) = last { + labels.push( + d::Label::secondary(entry.location.source_id, entry.location.span.range()) + .with_message(format!("final step cycling back to `{}`", entry.item)), + ); + } + } } impl EmitDiagnostics for FatalDiagnostic { diff --git a/crates/rune/src/indexing/index.rs b/crates/rune/src/indexing/index.rs index 2d0f86d92..60946fbb8 100644 --- a/crates/rune/src/indexing/index.rs +++ b/crates/rune/src/indexing/index.rs @@ -1132,11 +1132,11 @@ fn expr(ast: &mut ast::Expr, idx: &mut Indexer<'_>, is_used: IsUsed) -> CompileR // NB: macros have nothing to index, they don't export language // items. ast::Expr::MacroCall(macro_call) => { - // Note: There is a preprocessing step involved with statemetns - // for which the macro **might** have been expanded to a - // built-in macro if we end up here. So instead of expanding if - // the id is set, we just assert that the builtin macro has been - // added to the query engine. + // Note: There is a preprocessing step involved with statements for + // which the macro **might** have been expanded to a built-in macro + // if we end up here. So instead of expanding if the id is set, we + // just assert that the builtin macro has been added to the query + // engine. if !macro_call.id.is_set() { if !idx.try_expand_internal_macro(&mut attributes, macro_call)? { diff --git a/crates/rune/src/query/mod.rs b/crates/rune/src/query/mod.rs index 3838dec3e..b55f6d4a7 100644 --- a/crates/rune/src/query/mod.rs +++ b/crates/rune/src/query/mod.rs @@ -22,6 +22,9 @@ use std::fmt; use std::num::NonZeroUsize; use std::sync::Arc; +/// The permitted number of import recursions when constructing a path. +const IMPORT_RECURSION_LIMIT: usize = 128; + pub use self::query_error::{QueryError, QueryErrorKind}; mod query_error; @@ -808,6 +811,7 @@ impl<'a> Query<'a> { } /// Get the given import by name. + #[tracing::instrument(skip(self, span, module))] pub(crate) fn import( &mut self, span: Span, @@ -821,7 +825,18 @@ impl<'a> Query<'a> { let mut item = item.clone(); let mut any_matched = false; + let mut count = 0usize; + 'outer: loop { + if count > IMPORT_RECURSION_LIMIT { + return Err(QueryError::new( + span, + QueryErrorKind::ImportRecursionLimit { count, path }, + )); + } + + count += 1; + let mut cur = Item::new(); let mut it = item.iter(); @@ -861,6 +876,7 @@ impl<'a> Query<'a> { } /// Inner import implementation that doesn't walk the imported name. + #[tracing::instrument(skip(self, span, module, path))] fn import_step( &mut self, span: Span, diff --git a/crates/rune/src/query/query_error.rs b/crates/rune/src/query/query_error.rs index 17e305f88..63361dae8 100644 --- a/crates/rune/src/query/query_error.rs +++ b/crates/rune/src/query/query_error.rs @@ -75,6 +75,8 @@ pub enum QueryErrorKind { MissingMod { item: Item }, #[error("cycle in import")] ImportCycle { path: Vec }, + #[error("import recursion limit reached ({count})")] + ImportRecursionLimit { count: usize, path: Vec }, #[error("missing last use component")] LastUseComponent, #[error("found indexed entry for `{item}`, but was not an import")] diff --git a/crates/rune/src/worker/import.rs b/crates/rune/src/worker/import.rs index 1855f956b..fd4dc952d 100644 --- a/crates/rune/src/worker/import.rs +++ b/crates/rune/src/worker/import.rs @@ -75,6 +75,8 @@ impl Import { queue.push_back((&self.ast.path, name, first, initial)); while let Some((path, mut name, first, mut initial)) = queue.pop_front() { + tracing::trace!("process one"); + let mut it = first .into_iter() .chain(path.segments.iter().map(|(_, s)| s)); diff --git a/crates/rune/src/worker/mod.rs b/crates/rune/src/worker/mod.rs index 224378277..8e43e271c 100644 --- a/crates/rune/src/worker/mod.rs +++ b/crates/rune/src/worker/mod.rs @@ -103,7 +103,7 @@ impl<'a> Worker<'a> { LoadFileKind::Module { root } => root, }; - tracing::trace!("index: {}", mod_item.item); + tracing::trace!("load file: {}", mod_item.item); let items = Items::new(mod_item.item.clone(), self.gen); let mut indexer = Indexer { @@ -128,6 +128,8 @@ impl<'a> Worker<'a> { } } Task::ExpandImport(import) => { + tracing::trace!("expand import"); + let source_id = import.source_id; let queue = &mut self.queue; @@ -140,6 +142,8 @@ impl<'a> Worker<'a> { } } Task::ExpandWildcardImport(wildcard_import) => { + tracing::trace!("expand wildcard import"); + wildcard_imports.push(wildcard_import); } }