Skip to content

fix: add missing Mod case to the typed-AST JSON emitter - #227

Merged
anjapetkovic merged 3 commits into
argotorg:mainfrom
mijovic:fix/json-ast-mod
Jul 30, 2026
Merged

fix: add missing Mod case to the typed-AST JSON emitter#227
anjapetkovic merged 3 commits into
argotorg:mainfrom
mijovic:fix/json-ast-mod

Conversation

@mijovic

@mijovic mijovic commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

act type crashed with todo: json ast for: Mod ... after successfully typechecking any spec that uses the % operator: the ToJSON instance for typed expressions covered every constructor except Mod, falling through to the error catch-all.

Add the Mod case (mirroring Div) and delete the catch-all, which the exhaustiveness checker now proves dead (Mod was the only missing constructor; keeping it trips -Werror=overlapping-patterns).

Regression test: a minimal %-using spec under tests/typing/pass, which exercises act type | jq . via the .type.pass make target. The existing %-specs under tests/hevm never hit this path.

mijovic and others added 2 commits July 29, 2026 12:32
`act type` crashed with `todo: json ast for: Mod ...` after successfully
typechecking any spec that uses the % operator: the ToJSON instance for
typed expressions covered every constructor except Mod, falling through
to the error catch-all.

Add the Mod case (mirroring Div) and delete the catch-all, which the
exhaustiveness checker now proves dead (Mod was the only missing
constructor; keeping it trips -Werror=overlapping-patterns).

Regression test: a minimal %-using spec under tests/typing/pass, which
exercises `act type | jq .` via the .type.pass make target. The existing
%-specs under tests/hevm never hit this path.
@lefterislazar

Copy link
Copy Markdown
Collaborator

Thank you for the fix. I have added the missing test output. Once the CI passes we will accept and merge.

@anjapetkovic anjapetkovic left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good and the checks now pass.

@anjapetkovic
anjapetkovic merged commit 7cf80f0 into argotorg:main Jul 30, 2026
2 checks passed
@mijovic
mijovic deleted the fix/json-ast-mod branch July 30, 2026 10:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants