feat(assembly): warn about unused constants by giwaov · Pull Request #2993 · 0xMiden/miden-vm

giwaov · 2026-04-11T23:33:08Z

Description

Adds a warning diagnostic for private constants that are declared but never referenced by any procedure or other constant in a module, helping developers catch dead definitions early.

Closes #2898

Rationale

The assembler already emits warnings for unused imports (UnusedImport), but unused constants go silently unnoticed. This change applies the same pattern to constants: during semantic analysis, the compiler tracks which constants are actually referenced (by instructions in procedure bodies or by other constant expressions), and emits an UnusedConstant warning for any private constant that is never used. Public constants are excluded because they may be consumed by external modules.

Changes

crates/assembly-syntax/src/sema/errors.rs - Added UnusedConstant variant to SemanticAnalysisError with severity(Warning)
crates/assembly-syntax/src/sema/context.rs - Added used_constants: BTreeSet<Ident> and simplifying_constant: Option<Ident> to AnalysisContext for tracking constant usage during evaluation; added is_constant_used() and mark_constant_used() helper methods
crates/assembly-syntax/src/sema/mod.rs - Added unused-constant check loop after the existing unused-import check; marked advice-map-entry constants as used
crates/assembly/src/tests.rs - Added three tests: unused_constant_warning, used_constant_no_warning, public_constant_no_warning

Test plan

unused_constant_warning - verifies the diagnostic output for a private constant that is never referenced
used_constant_no_warning - verifies no warning when a constant is consumed via push.MY_CONST
public_constant_no_warning - verifies no warning for pub const in a library module
Existing tests constant_alphanumeric_expression, enum_discriminants_can_reference_constants, and test_adv_has_map_key continue to pass (constants referenced only by other constants, and advice-map-generated constants, are correctly treated as used)

Add a warning diagnostic for private constants that are never referenced by any procedure or other constant in the module. Public constants are excluded since they may be consumed by external modules. The implementation tracks constant usage during constant simplification and instruction visiting, then emits UnusedConstant warnings for unreferenced private constants. Closes 0xMiden#2898

github-actions · 2026-04-11T23:33:20Z

Automated check (CONTRIBUTING.md)

Findings:

Ensure issue Warn about unused constants #2898 is assigned to the PR author.

Next steps:

Ensure issue Warn about unused constants #2898 is assigned to you.
See CONTRIBUTING.md for expectations.
If this is a false positive, comment: /quality-review.

Add pub proc accessors for 6 private constants in stark/constants.masm that had no accessor procedures, causing unused constant warnings with the new diagnostic.

huitseeker · 2026-04-13T08:53:50Z

    fn get(&mut self, name: &Ident) -> Result<Option<CachedConstantValue<'_>>, 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 {


Because get() runs during simplify_constants(), this marks A as used even in a dead chain like const A = 1 and const B = A where neither constant is referenced from any procedure or exported API. In that case only B warns and A is missed.

To really address this, we need to track constant-to-constant edges here and compute reachability from real roots.

Good catch - fixed in 5cc929c. Instead of marking constants as used during \get(), I now track constant-to-constant edges in a separate map and then do a BFS from real roots (procedure references + public constants) to figure out which ones are actually reachable. So \const A = 1; const B = A\ correctly warns on both if neither is referenced.

Added tests for both cases - both unused should warn, and transitive usage through a chain shouldn't.

…etection During simplify_constants(), references between constants are now recorded as dependency edges rather than marking the target as directly used. A transitive closure is computed from real roots (procedure bodies and public constants) before emitting unused constant warnings. This fixes false negatives where a constant was only referenced by another unused constant.

huitseeker · 2026-04-14T06:03:33Z

    }

+    // Check unused constants
+    analyzer.resolve_constant_usage();


I could repro a small mismatch here with a local test. A program shaped like

use lib::a const DEAD = a::BAR begin push.1 end

only emits unused constant when warnings are promoted to errors, and it skips the matching unused import warning.

The reason seems to be that visit_mut_constant_ref() increments alias.uses before this liveness pass proves the referencing constant is live, so imports reached only from dead constants stay marked as used.

Nice catch - fixed in fda2ca5. The visit_mut_constant_ref path in VerifyInvokeTargets was bumping alias.uses unconditionally, so imports reached only through dead constants slipped past the unused-import check.

Now constant exports record their import references as deferred edges in AnalysisContext instead of incrementing directly. After resolve_constant_usage() runs, only imports from live constants get credited. Moved the unused-import check after constant liveness resolution too.

Added dead_constant_does_not_mask_unused_import to cover the exact scenario you described.

…solved

huitseeker · 2026-04-14T20:36:14Z

@@ -282,10 +291,16 @@ impl VisitMut for VerifyInvokeTargets<'_> {
    fn visit_mut_constant_ref(&mut self, path: &mut Span<Arc<Path>>) -> ControlFlow<()> {
        if let Some(name) = path.as_ident() {
            self.track_used_alias(&name);


The direct-import path still bypasses the new deferral. When a dead constant uses an imported constant alias like use lib::a::BAR followed by const DEAD = BAR, this branch calls track_used_alias(&name) immediately, so the import is treated as used and the expected unused import warning never fires.

This wants the same deferral logic for identifier refs that resolve to imports, plus a regression test for direct and renamed constant imports.

huitseeker · 2026-04-14T20:36:14Z

+    /// 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) {


The liveness check here skips pub const roots because resolve_constant_usage() only pushes public constants onto the worklist, it does not insert their own names into used_constants.

That means pub const LIVE = a::BAR still leaves use lib::a looking unused even though the import is needed to evaluate an exported constant.

Marking public constants as live before applying deferred import refs, and adding a regression test for pub const LIVE = a::BAR, should close the hole.

giwaov · 2026-04-15T20:00:53Z

Addressed both review comments:

Direct-import deferral — \�isit_mut_constant_ref\ now defers \ rack_used_alias\ for identifier refs inside constant bodies (matching the existing deferral for qualified paths). \use lib::a::BAR\ + \const DEAD = BAR\ now correctly reports both unused-import and unused-constant.
Public constants marked live —
esolve_constant_usage()\ now inserts public constants into \used_constants\ (not just the worklist), so \�pply_live_constant_import_refs\ correctly credits their import references. \pub const LIVE = a::BAR\ no longer triggers a spurious unused-import warning.

Added regression tests for both cases. All 186 assembly unit tests and 104 assembly-syntax tests pass.

…constants live - Defer rack_used_alias for identifier constant refs inside constant bodies, matching the existing deferral for qualified paths. This fixes the case where use lib::a::BAR + const DEAD = BAR incorrectly kept the import marked as used. - Insert public constants into used_constants during esolve_constant_usage so that �pply_live_constant_import_refs correctly credits their import references. Without this, pub const LIVE = a::BAR would leave use lib::a looking unused. - Add regression tests for both cases.

huitseeker · 2026-04-21T10:47:49Z

-            && 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 {


Could we check that name is actually an import before recording it here?

Local constants can reuse an imported alias name: use lib::a::FOO plus const FOO = 1 makes get() resolve FOO as the local constant, but this branch still records FOO as an import ref. If a live constant references that local FOO, apply_live_constant_import_refs() increments the import use and hides the unused-import warning for the shadowed import.

Fixed in 3e09839.

The guard is in two places in verify_invoke.rs:

track_used_alias — now calls analyzer.get_constant(name) first and returns early if the name resolves to a local constant, so a same-named import is never credited as used.

visit_mut_constant_ref (identifier branch) — the deferred record_constant_import_ref call is now also skipped when the identifier resolves to a local constant, so dead-constant→shadowed-import edges don't survive into liveness resolution either.

Added a regression test (local_constant_shadowing_import_warns_unused_import) that assembles a module with both use lib::a::FOO and const FOO = 1 and asserts the import is reported unused.

…dowing When a local constant has the same name as an imported alias, track_used_alias and visit_mut_constant_ref were crediting the import as used even though the local definition shadows it. Check get_constant() before marking an alias used or recording a deferred import ref.

github-actions Bot added the quality-concern label Apr 11, 2026

giwaov added 2 commits April 12, 2026 00:38

docs: add changelog entry for unused constant warnings

69b437e

fix(core-lib): add accessor procs for unused constants in stark module

939b412

Add pub proc accessors for 6 private constants in stark/constants.masm that had no accessor procedures, causing unused constant warnings with the new diagnostic.

huitseeker requested changes Apr 13, 2026

View reviewed changes

giwaov force-pushed the feat/warn-unused-constants branch from db5d3f0 to 5cc929c Compare April 13, 2026 10:51

huitseeker reviewed Apr 14, 2026

View reviewed changes

giwaov added 2 commits April 14, 2026 08:34

fix(assembly): defer import usage from constants until liveness is re…

fda2ca5

…solved

fix: rustfmt and broken doc link

7fdaa28

huitseeker requested changes Apr 14, 2026

View reviewed changes

huitseeker reviewed Apr 14, 2026

View reviewed changes

giwaov force-pushed the feat/warn-unused-constants branch from a971b52 to 49fca2c Compare April 15, 2026 20:23

huitseeker requested changes Apr 21, 2026

View reviewed changes

giwaov force-pushed the feat/warn-unused-constants branch from 3e09839 to c3ef64b Compare April 21, 2026 17:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(assembly): warn about unused constants#2993

feat(assembly): warn about unused constants#2993
giwaov wants to merge 8 commits into0xMiden:mainfrom
giwaov:feat/warn-unused-constants

giwaov commented Apr 11, 2026

Uh oh!

github-actions Bot commented Apr 11, 2026

Uh oh!

huitseeker Apr 13, 2026

Uh oh!

giwaov Apr 13, 2026 •

edited

Loading

Uh oh!

huitseeker Apr 14, 2026

Uh oh!

giwaov Apr 14, 2026

Uh oh!

huitseeker Apr 14, 2026

Uh oh!

huitseeker Apr 14, 2026

Uh oh!

giwaov commented Apr 15, 2026

Uh oh!

huitseeker Apr 21, 2026

Uh oh!

giwaov Apr 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

giwaov commented Apr 11, 2026

Description

Rationale

Changes

Test plan

Uh oh!

github-actions Bot commented Apr 11, 2026

Uh oh!

huitseeker Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

giwaov Apr 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

huitseeker Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

giwaov Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

huitseeker Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

huitseeker Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

giwaov commented Apr 15, 2026

Uh oh!

huitseeker Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

giwaov Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

giwaov Apr 13, 2026 •

edited

Loading