Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test the reference implementation #120

Merged
merged 11 commits into from
Mar 8, 2025

Conversation

zerbina
Copy link
Collaborator

@zerbina zerbina commented Mar 4, 2025

Summary

Also run the expr tests using the source language reference
implementation languages/source.nim, to make sure that the
reference/specification implements/describes the desired semantics.

Details

As a temporary solution, a standalone test orchestrator is used that
scans the expr directory for test files. For each test file, the
spec is extracted and (the relevant bits of it) parsed, with the test
content then parsed into a representation understood by the meta-
language interpreter.

The processing pipeline is: desugar -> type-check -> evaluate. Stages
are enabled/disabled based on the test specification. Failing tests
don't result in an orchestrator error, so that they can be fixed
incrementally; the list of known issues is hard-coded and needs to be
updated manually once more tests start to succeed.

There are many issues with the reference implementation as is, and
therefore not all tests can be run successfully at the moment.

A few small changes/additions are made to source, so that at least
some tests work:

  • the cstep relation is added to source, which is the transitive
    closure of step. It's used for fully reducing (i.e., evaluating)
    an expression
  • an infinite recursion with the <:= relation is fixed
  • a catch-call branch is added to desugar so that the function
    doesn't fail for unhandled input

Finally, a dangling reference caused by a parameter aliasing violation
with the mitems's iterator usage in interpretRelation is fixed.

zerbina added 8 commits March 4, 2025 23:10
This makes `desugar` at least work not fail for currently unhandled
syntax.
The exception type is needed for catching failures escaping
`interpret`.
The module is now compiled as part of the `spectest` program.
The interpreter doesn't detect infinite recursion and simply crashes
due to a stack overflow, making it a necessity to fix the issue
already.
The step is integrated into the existing job, as it's too small to
warrant its own job.
@zerbina zerbina added the enhancement New feature or request label Mar 4, 2025
@zerbina
Copy link
Collaborator Author

zerbina commented Mar 4, 2025

The current plan going forward is as follows:

  1. incrementally fix all reference implementation issues
  2. once all issues are resolved (and thus all tests succeed), replace the orchestrator with a proper test runner
  3. make writing tests easier by grouping multiple related tests into a single test file

Work on new language features can continue in parallel. All tests not included in the issues array are required to succeed, which is going to prevent reference implementation regressions during this time.

@zerbina zerbina requested a review from saem March 4, 2025 23:39
@zerbina zerbina mentioned this pull request Mar 4, 2025
10 tasks
The `mut` qualifier is an implementation detail not relevant to outside
use.
zerbina added 2 commits March 8, 2025 15:05
The cache may be modified by `interpret`, which can result in the
storage location of the iterated-over `seq` changing, thus causing the
reference passed to `mitems` becoming stale.
@zerbina
Copy link
Collaborator Author

zerbina commented Mar 8, 2025

#121 introduced a parameter aliasing violation leading to the previous CI failure. I've fixed it and updated the PR message accordingly.

@zerbina zerbina merged commit f4938a3 into nim-works:main Mar 8, 2025
6 checks passed
@zerbina zerbina deleted the reference-impl-tests branch March 8, 2025 14:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants