🐛 fix: correct symbol handling, index_by test, and inlay hints logic#1421
🐛 fix: correct symbol handling, index_by test, and inlay hints logic#1421
Conversation
harehare
commented
Mar 11, 2026
- Fix symbol handling in Hir by using add_expr for arguments
- Update index_by test to use correct dict syntax
- Fix inlay hints logic to check for concrete types
- Fix symbol handling in Hir by using add_expr for arguments - Update index_by test to use correct dict syntax - Fix inlay hints logic to check for concrete types
There was a problem hiding this comment.
Pull request overview
This PR fixes a few correctness issues across the HIR builder, builtin tests, and WASM inlay-hints generation to better align with actual language semantics and type-checker behavior.
Changes:
- Update HIR foreach handling to register the iterable expression via
add_exprrather than a singleRefsymbol. - Fix the
index_bybuiltin test to use dict literal syntax. - Update WASM inlay hints to only emit hints when the inferred type is fully concrete.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| crates/mq-wasm/src/script.rs | Tightens inlay-hints emission by requiring concrete inferred types. |
| crates/mq-lang/builtin_tests.mq | Adjusts index_by test input to valid dict literal syntax. |
| crates/mq-hir/src/hir.rs | Changes foreach iterable symbol handling to use recursive expression registration (add_expr). |
| doc: node.comments(), | ||
| parent: Some(symbol_id), | ||
| }); | ||
| self.add_expr(arg, source_id, scope_id, Some(symbol_id)); |
There was a problem hiding this comment.
In foreach, the iterable expression (arg) is evaluated in the outer environment before the loop binding is introduced (see mq-lang/src/eval.rs eval_foreach: it evaluates values before creating the new loop env). Adding the iterable expression into the new loop scope can cause name resolution bugs for cases like foreach(x, x): ... where the iterable x should refer to the outer binding, not the loop variable declared on the same line. Consider building symbols for the iterable expression in the parent scope (keep the original scope_id before creating the loop scope), and only use the loop scope for the loop variable and body.
| if !type_scheme.ty.is_concrete() { | ||
| return None; |
There was a problem hiding this comment.
Type::is_concrete() currently allocates and collects all type variables via free_vars(); calling this for every symbol when generating inlay hints can add noticeable overhead for large files (especially in WASM). If you only need a boolean “contains any type var”, consider adding a short-circuiting helper (e.g., has_type_vars() / contains_var()) that walks the type tree without allocating, and use that here instead of is_concrete().