diff --git a/CHANGELOG.md b/CHANGELOG.md index ca81458e01..0a7b8bee20 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ - Implemented project assembly ([#2877](https://github.com/0xMiden/miden-vm/pull/2877)). - Added `FastProcessor::into_parts()` to extract advice provider, memory, and precompile transcript after step-based execution ([#2901](https://github.com/0xMiden/miden-vm/pull/2901)). +- Added warning diagnostic for unused private constants in modules ([#2993](https://github.com/0xMiden/miden-vm/pull/2993)). ## 0.22.0 (2026-03-18) diff --git a/crates/assembly-syntax/src/sema/context.rs b/crates/assembly-syntax/src/sema/context.rs index 86e2415159..fdb3a69281 100644 --- a/crates/assembly-syntax/src/sema/context.rs +++ b/crates/assembly-syntax/src/sema/context.rs @@ -1,6 +1,6 @@ use alloc::{ boxed::Box, - collections::{BTreeMap, BTreeSet}, + collections::{BTreeMap, BTreeSet, VecDeque}, sync::Arc, vec::Vec, }; @@ -17,6 +17,16 @@ use crate::ast::{ /// This maintains the state for semantic analysis of a single [Module]. pub struct AnalysisContext { constants: BTreeMap, + used_constants: BTreeSet, + /// Edges from a constant to the constants it references. + /// Used to compute transitive usage from real roots. + constant_deps: BTreeMap>, + /// Edges from a constant to the imports it references. + /// Used to avoid marking imports as used when only dead constants reference them. + constant_import_refs: BTreeMap>, + /// When set, references are recorded as constant-to-constant edges + /// rather than marking the target as directly used. + simplifying_constant: Option, imported: BTreeSet, procedures: BTreeSet, errors: Vec, @@ -38,6 +48,14 @@ impl constants::ConstEnvironment for AnalysisContext { #[inline] fn get(&mut self, name: &Ident) -> Result>, Self::Error> { if let Some(constant) = self.constants.get(name) { + let is_self_ref = self.simplifying_constant.as_ref() == Some(name); + if !is_self_ref { + if let Some(ref parent) = self.simplifying_constant { + self.constant_deps.entry(parent.clone()).or_default().insert(name.clone()); + } else { + self.used_constants.insert(name.clone()); + } + } Ok(Some(CachedConstantValue::Miss(&constant.value))) } else if self.imported.contains(name) { // We don't have the definition available yet @@ -67,6 +85,10 @@ impl AnalysisContext { pub fn new(source_file: Arc, source_manager: Arc) -> Self { Self { constants: Default::default(), + used_constants: Default::default(), + constant_deps: Default::default(), + constant_import_refs: Default::default(), + simplifying_constant: None, imported: Default::default(), procedures: Default::default(), errors: Default::default(), @@ -98,6 +120,76 @@ impl AnalysisContext { self.imported.insert(name); } + /// Returns true if the constant has been referenced by another constant or + /// by a procedure body, or is publicly visible. + pub fn is_constant_used(&self, constant: &Constant) -> bool { + constant.visibility.is_public() || self.used_constants.contains(&constant.name) + } + + /// Mark a constant as used. + /// + /// This is used for constants created as a side effect of other declarations + /// (e.g. advice map entries) that should not trigger unused constant warnings. + pub fn mark_constant_used(&mut self, name: &Ident) { + self.used_constants.insert(name.clone()); + } + + /// Record that constant `constant_name` references import `import_name`. + /// + /// These edges are used to defer import-usage bookkeeping until after + /// constant liveness has been resolved, so that imports reached only from + /// dead constants are correctly reported as unused. + pub fn record_constant_import_ref( + &mut self, + constant_name: &Ident, + import_name: alloc::string::String, + ) { + self.constant_import_refs + .entry(constant_name.clone()) + .or_default() + .insert(import_name); + } + + /// Increment `alias.uses` only for imports that are referenced by live + /// constants. Must be called after `resolve_constant_usage`. + pub fn apply_live_constant_import_refs(&self, module: &mut Module) { + for (constant_name, import_names) in &self.constant_import_refs { + if !self.used_constants.contains(constant_name) { + continue; + } + for import_name in import_names { + if let Some(alias) = + module.aliases_mut().find(|a| a.name().as_str() == import_name.as_str()) + { + alias.uses += 1; + } + } + } + } + + /// Propagate usage from real roots through constant-to-constant edges. + /// + /// A constant is considered truly used if it is public, directly referenced + /// by a procedure body, or transitively referenced by such a constant. + /// This must be called before checking for unused constants. + pub fn resolve_constant_usage(&mut self) { + let mut worklist: VecDeque = self.used_constants.iter().cloned().collect(); + for (name, constant) in &self.constants { + if constant.visibility.is_public() && self.used_constants.insert(name.clone()) { + worklist.push_back(name.clone()); + } + } + while let Some(name) = worklist.pop_front() { + if let Some(deps) = self.constant_deps.get(&name) { + for dep in deps { + if self.used_constants.insert(dep.clone()) { + worklist.push_back(dep.clone()); + } + } + } + } + } + /// Define a new constant `constant` /// /// Returns `Err` if a constant with the same name is already defined @@ -133,6 +225,7 @@ impl AnalysisContext { let constants = self.constants.keys().cloned().collect::>(); for constant in constants.iter() { + self.simplifying_constant = Some(constant.clone()); let expr = ConstantExpr::Var(Span::new( constant.span(), PathBuf::from(constant.clone()).into(), @@ -145,6 +238,7 @@ impl AnalysisContext { self.errors.push(err); }, } + self.simplifying_constant = None; } } diff --git a/crates/assembly-syntax/src/sema/errors.rs b/crates/assembly-syntax/src/sema/errors.rs index 75ef8a0e37..7bc1eb9968 100644 --- a/crates/assembly-syntax/src/sema/errors.rs +++ b/crates/assembly-syntax/src/sema/errors.rs @@ -114,6 +114,12 @@ pub enum SemanticAnalysisError { #[label] span: SourceSpan, }, + #[error("unused constant")] + #[diagnostic(severity(Warning), help("this constant is never used and can be safely removed"))] + UnusedConstant { + #[label] + span: SourceSpan, + }, #[error("missing import: the referenced module has not been imported")] #[diagnostic()] MissingImport { diff --git a/crates/assembly-syntax/src/sema/mod.rs b/crates/assembly-syntax/src/sema/mod.rs index 832a88c82d..d267ea8da7 100644 --- a/crates/assembly-syntax/src/sema/mod.rs +++ b/crates/assembly-syntax/src/sema/mod.rs @@ -164,13 +164,25 @@ pub fn analyze( // Run item checks visit_items(&mut module, &mut analyzer)?; - // Check unused imports + // Check unused constants + analyzer.resolve_constant_usage(); + + // Apply deferred import references from live constants, then check unused + // imports. This must happen after constant liveness is resolved so that + // imports reached only from dead constants are correctly reported as unused. + analyzer.apply_live_constant_import_refs(&mut module); for import in module.aliases() { if !import.is_used() { analyzer.error(SemanticAnalysisError::UnusedImport { span: import.span() }); } } + for constant in module.constants() { + if !analyzer.is_constant_used(constant) { + analyzer.error(SemanticAnalysisError::UnusedConstant { span: constant.span }); + } + } + analyzer.into_result().map(move |_| module) } @@ -240,6 +252,7 @@ fn visit_items(module: &mut Module, analyzer: &mut AnalysisContext) -> Result<() log::debug!(target: "verify-invoke", "visiting constant {}", constant.name()); { let mut visitor = VerifyInvokeTargets::new(analyzer, module, &locals, None); + visitor.set_current_constant(Some(constant.name.clone())); let _ = visitor.visit_mut_constant(&mut constant); } module.items.push(Export::Constant(constant)); @@ -328,6 +341,7 @@ fn add_advice_map_entry( ConstantExpr::Word(Span::new(entry.span, WordValue(*key))), ); context.define_constant(module, cst); + context.mark_constant_used(&entry.name); match module.advice_map.get(&key) { Some(_) => { context.error(SemanticAnalysisError::AdvMapKeyAlreadyDefined { span: entry.span }); diff --git a/crates/assembly-syntax/src/sema/passes/verify_invoke.rs b/crates/assembly-syntax/src/sema/passes/verify_invoke.rs index b9a741f289..e9df47d7be 100644 --- a/crates/assembly-syntax/src/sema/passes/verify_invoke.rs +++ b/crates/assembly-syntax/src/sema/passes/verify_invoke.rs @@ -24,6 +24,9 @@ pub struct VerifyInvokeTargets<'a> { module: &'a mut Module, procedures: &'a BTreeSet, current_procedure: Option, + /// When visiting a constant export, holds the constant's name so that + /// import references can be deferred until constant liveness is resolved. + current_constant: Option, invoked: BTreeSet, } @@ -39,9 +42,15 @@ impl<'a> VerifyInvokeTargets<'a> { module, procedures, current_procedure, + current_constant: None, invoked: Default::default(), } } + + /// Set the constant name whose export is currently being visited. + pub fn set_current_constant(&mut self, name: Option) { + self.current_constant = name; + } } impl VerifyInvokeTargets<'_> { @@ -110,6 +119,11 @@ impl VerifyInvokeTargets<'_> { } } fn track_used_alias(&mut self, name: &Ident) { + // A locally-defined constant with the same name shadows any import of that name, + // so the import should not be credited as used in that case. + if self.analyzer.get_constant(name).is_ok() { + return; + } if let Some(alias) = self.module.aliases_mut().find(|a| a.name() == name) { alias.uses += 1; } @@ -281,11 +295,25 @@ impl VisitMut for VerifyInvokeTargets<'_> { } fn visit_mut_constant_ref(&mut self, path: &mut Span>) -> ControlFlow<()> { if let Some(name) = path.as_ident() { - self.track_used_alias(&name); - } else if let Some((module, _)) = path.split_first() - && let Some(alias) = self.module.aliases_mut().find(|a| a.name().as_str() == module) - { - alias.uses += 1; + if let Some(ref const_name) = self.current_constant { + // Only defer as an import ref if this identifier is not a local constant. + // A local constant shadows any same-named import, so the import gets no credit. + if self.analyzer.get_constant(&name).is_err() { + self.analyzer.record_constant_import_ref(const_name, name.as_str().into()); + } + } else { + self.track_used_alias(&name); + } + } else if let Some((module, _)) = path.split_first() { + if let Some(ref const_name) = self.current_constant { + // Defer: record the edge so we only credit the import when this + // constant is proven live. + self.analyzer.record_constant_import_ref(const_name, module.into()); + } else if let Some(alias) = + self.module.aliases_mut().find(|a| a.name().as_str() == module) + { + alias.uses += 1; + } } ControlFlow::Continue(()) } diff --git a/crates/assembly/src/tests.rs b/crates/assembly/src/tests.rs index 2c443bf035..590f7a4627 100644 --- a/crates/assembly/src/tests.rs +++ b/crates/assembly/src/tests.rs @@ -5789,3 +5789,285 @@ fn test_linking_recursive_expansion_via_renamed_aliases() -> TestResult { Ok(()) } + +// UNUSED CONSTANTS +// ================================================================================================ + +#[test] +fn unused_constant_warning() { + let context = TestContext::default(); + let source = source_file!( + &context, + " + const UNUSED = 42 + + begin + push.1 + end" + ); + + assert_assembler_diagnostic!( + context, + source, + "syntax error", + "help: see emitted diagnostics for details", + "unused constant", + regex!(r#",-\[test[\d]+:2:9\]"#), + "1 |", + "2 | const UNUSED = 42", + " : ^^^^^^^^^^^^^^^^^", + "3 |", + " `----", + " help: this constant is never used and can be safely removed" + ); +} + +#[test] +fn used_constant_no_warning() -> TestResult { + let context = TestContext::default(); + let source = source_file!( + &context, + " + const MY_CONST = 42 + + begin + push.MY_CONST + end" + ); + + let _program = context.assemble(source)?; + Ok(()) +} + +#[test] +fn public_constant_no_warning() -> TestResult { + let context = TestContext::default(); + let source = source_file!( + &context, + " + pub const EXPORTED = 42 + + pub proc foo + push.1 + end" + ); + + let module = context.parse_module_with_path("test::lib", source)?; + let _library = context.assemble_library([module])?; + Ok(()) +} + +#[test] +fn chained_unused_constants_both_warn() { + let context = TestContext::default(); + let source = source_file!( + &context, + " + const A = 1 + const B = A + + begin + push.1 + end" + ); + + assert_assembler_diagnostic!( + context, + source, + "syntax error", + "help: see emitted diagnostics for details", + "unused constant", + regex!(r#",-\[test[\d]+:2:9\]"#), + "1 |", + "2 | const A = 1", + " : ^^^^^^^^^^^", + "3 |", + " `----", + " help: this constant is never used and can be safely removed", + "unused constant", + regex!(r#",-\[test[\d]+:3:9\]"#), + "2 |", + "3 | const B = A", + " : ^^^^^^^^^^^", + "4 |", + " `----", + " help: this constant is never used and can be safely removed" + ); +} + +#[test] +fn chained_constant_used_transitively() -> TestResult { + let context = TestContext::default(); + let source = source_file!( + &context, + " + const A = 1 + const B = A + + begin + push.B + end" + ); + + let _program = context.assemble(source)?; + Ok(()) +} + +#[test] +fn dead_constant_does_not_mask_unused_import() { + let context = TestContext::default(); + let a = "pub const BAR = 42\n\npub proc noop\n push.1 drop\nend\n"; + let a = parse_module!(&context, "lib::a", a); + let lib = Assembler::new(context.source_manager()).assemble_library([a]).unwrap(); + + let mut context = TestContext::default(); + context.add_library(&lib).unwrap(); + + let source = source_file!( + &context, + " + use lib::a + + const DEAD = a::BAR + + begin + push.1 + end" + ); + + assert_assembler_diagnostic!( + context, + source, + "syntax error", + "help: see emitted diagnostics for details", + "unused import", + regex!(r#",-\[test[\d]+:2:13\]"#), + "1 |", + "2 | use lib::a", + " : ^^^^^^", + "3 |", + " `----", + " help: this import is never used and can be safely removed", + "unused constant", + regex!(r#",-\[test[\d]+:4:9\]"#), + "3 |", + "4 | const DEAD = a::BAR", + " : ^^^^^^^^^^^^^^^^^^^", + "5 |", + " `----", + " help: this constant is never used and can be safely removed" + ); +} + +#[test] +fn dead_constant_direct_import_warns_unused_import() { + let context = TestContext::default(); + let a = "pub const BAR = 42\n\npub proc noop\n push.1 drop\nend\n"; + let a = parse_module!(&context, "lib::a", a); + let lib = Assembler::new(context.source_manager()).assemble_library([a]).unwrap(); + + let mut context = TestContext::default(); + context.add_library(&lib).unwrap(); + + let source = source_file!( + &context, + " + use lib::a::BAR + + const DEAD = BAR + + begin + push.1 + end" + ); + + assert_assembler_diagnostic!( + context, + source, + "syntax error", + "help: see emitted diagnostics for details", + "unused import", + regex!(r#",-\[test[\d]+:2:13\]"#), + "1 |", + "2 | use lib::a::BAR", + " : ^^^^^^^^^^", + "3 |", + " `----", + " help: this import is never used and can be safely removed", + "unused constant", + regex!(r#",-\[test[\d]+:4:9\]"#), + "3 |", + "4 | const DEAD = BAR", + " : ^^^^^^^^^^^^^^^^", + "5 |", + " `----", + " help: this constant is never used and can be safely removed" + ); +} + +#[test] +fn pub_constant_import_not_unused() -> TestResult { + let context = TestContext::default(); + let a = "pub const BAR = 42\n\npub proc noop\n push.1 drop\nend\n"; + let a = parse_module!(&context, "lib::a", a); + let lib = Assembler::new(context.source_manager()).assemble_library([a]).unwrap(); + + let mut context = TestContext::default(); + context.add_library(&lib).unwrap(); + + let source = source_file!( + &context, + " + use lib::a + + pub const LIVE = a::BAR + + pub proc foo + push.1 + end" + ); + + let module = context.parse_module_with_path("test::lib", source)?; + let _library = context.assemble_library([module])?; + Ok(()) +} + +#[test] +fn local_constant_shadowing_import_warns_unused_import() { + let context = TestContext::default(); + let a = "pub const FOO = 99\n\npub proc noop\n push.1 drop\nend\n"; + let a = parse_module!(&context, "lib::a", a); + let lib = Assembler::new(context.source_manager()).assemble_library([a]).unwrap(); + + let mut context = TestContext::default(); + context.add_library(&lib).unwrap(); + + // `use lib::a::FOO` imports FOO, but a local `const FOO = 1` shadows it. + // The import should be reported as unused because the local constant takes precedence. + let source = source_file!( + &context, + " + use lib::a::FOO + + const FOO = 1 + + begin + push.FOO + end" + ); + + assert_assembler_diagnostic!( + context, + source, + "syntax error", + "help: see emitted diagnostics for details", + "unused import", + regex!(r#",-\[test[\d]+:2:13\]"#), + "1 |", + "2 | use lib::a::FOO", + " : ^^^^^^^^^^^", + "3 |", + " `----", + " help: this import is never used and can be safely removed" + ); +} diff --git a/crates/lib/core/asm/stark/constants.masm b/crates/lib/core/asm/stark/constants.masm index 770942651d..780d90b9c7 100644 --- a/crates/lib/core/asm/stark/constants.masm +++ b/crates/lib/core/asm/stark/constants.masm @@ -450,6 +450,30 @@ pub proc get_procedure_digest_process_public_inputs_ptr push.DYNAMIC_PROCEDURE_3_PTR end +pub proc get_num_fixed_len_public_inputs + push.NUM_FIXED_LEN_PUBLIC_INPUTS +end + +pub proc get_kernel_op_label + push.KERNEL_OP_LABEL +end + +pub proc num_fixed_len_public_inputs_ptr + push.NUM_FIXED_LEN_PUBLIC_INPUTS_PTR +end + +pub proc num_ace_inputs_ptr + push.NUM_ACE_INPUTS_PTR +end + +pub proc num_ace_gates_ptr + push.NUM_ACE_GATES_PTR +end + +pub proc max_cycle_len_log_ptr + push.MAX_CYCLE_LEN_LOG_PTR +end + # HELPER # =================================================================================================