[spectec] Fix prose of the rule instr_const#2161
Merged
Conversation
rossberg
reviewed
May 8, 2026
| (* HARDCODE: Prevent falsely unifying mixture of instr and val into `__unify:val` *) | ||
| and generalize_unified_wasm_val env u e = | ||
| if Il2al_util.is_val u && not (Il2al_util.is_val e) then ( | ||
| let new_var = VarE ("instr" $ no_region) $$ no_region % e.note in |
Member
There was a problem hiding this comment.
Would it be possible to avoid hard-coding the variable name? Could it invoke gen_new_unified without a prefix here right away?
(Btw, looking at gen_new_unified just now, I noticed that
let var =
match prefix with
| Some prefix -> introduce_fresh_variable ~prefix env.frees ty
| _ -> introduce_fresh_variable env.frees ty in
can be simplified to
let var = introduce_fresh_variable ?prefix env.frees ty
)
Collaborator
Author
There was a problem hiding this comment.
Makes sense. Fixed it accordingly.
rossberg
approved these changes
May 8, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR fixes the prose for the
instr_construle, where the variablevalwas incorrectly used instead ofinstr, resolving #2144.The root cause was that the rule unification logic only considered the first two rules (
CONSTandVCONST) and incorrectly assumed the unified variable type should beval. It now considers all cases and correctly generates the unified variable with typeinstr.