fix: reduce type-level let (TLet) RHS in the interpreter#1441
Merged
Conversation
added 2 commits
June 8, 2026 11:14
`interpTy` binds the right-hand side of a type-level `tlet` into the type
environment without reducing it first, violating the "environment values
are in normal form" invariant that the `TVar` lookup relies on. Add two
regression tests that demonstrate this:
- unit_tlet_reduce_rhs: `tlet a = C in (tlet v = a in v)` should reduce
to `C`, but the unreduced RHS leaves a dangling, out-of-scope
`TVar "a"` in the result.
- unit_tlet_reduce_rhs_let: `tlet v = (tlet w = C in w) in (v -> v)`
should reduce to `C -> C`, but the unreduced RHS leaves residual
`tlet`s rather than a normal form.
Both tests fail until the following commit fixes `interpTy`. See
#1439.
`interpTy` bound the right-hand side of a type-level `tlet` into the type environment unevaluated, while the `TVar` lookup returns stored values verbatim -- relying on them being normal forms. Every other binding site (term-level `Let`, `LetType`, `BETA`/`APP`, the `Case` argument types, and `TForall`) already interprets its RHS first; `TLet` was the lone exception. Interpret the RHS before binding it. This is safe and terminating, since type evaluation always terminates. It fixes both witnesses from the previous commit: type-level `let`s now fully reduce instead of leaving a dangling out-of-scope variable or residual `tlet`s. Fixes #1439.
Author
|
The first commit (b4aed7e) is intentionally red: its two tests fail in CI exactly as expected, demonstrating the bug.
(2 out of 886 tests failed, both in The second commit (de694f3) is the fix: |
dhess
approved these changes
Jun 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
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.
Fixes #1439.
Problem
Primer.EvalFullInterp.interpTyevaluates types in an environment whose values are meant to be normal forms — theTVarcase returns a stored value verbatim. But theTLetcase bound its right-hand side unevaluated:Every other binding site already interprets first — term-level
Let,LetType,BETA/APP, theCaseargument types, andTForall.TLetwas the lone exception, so type-levellets were left partly unreduced, and could even leak a dangling, out-of-scope type variable into the result.Structure
As requested on the issue, the PR is split so the bug is demonstrated before it's fixed:
test:adds two regression tests (unit_tlet_reduce_rhs,unit_tlet_reduce_rhs_let). They fail at this commit, demonstrating the bug:tlet a = C in (tlet v = a in v)→ reduces to a danglingTVar "a"instead ofC.tlet v = (tlet w = C in w) in (v -> v)→ leaves residualtlets instead ofC -> C.fix:interprets the RHS before binding it (interpTy (extendTyEnv' v (interpTy env s) env) t), turning both tests green. It's safe and terminating, since type evaluation always terminates.Notes
interpTy.unit_tlet*tests don't catch this because their RHSs are already normal forms (so the missinginterpTyis a no-op).tasty_two_interp_agreerarely generates the failing shape, and a residual concreteTLetin a checkable position is masked byupsilon.A note on the DCO check
I'm an automated agent (Hackworth Ltd Claude Code) and can't certify the Developer Certificate of Origin, so these commits aren't signed off and the
check-dcostatus will be red. A human with authority to certify the sign-off will need to resolve that before merge.