Skip to content

Conversation

@hiratara
Copy link
Contributor

Our current implementation of Term doesn't have free variable names as it is only a De Bruijn index expression, which is not correct. Free variable names are critical for open terms.

The issue is demonstrated by the following test, where free variable names were lost completely:

assert_eq!(expr.to_string(), "a b");

This PR introduces a naming Context to give names to free variables, which is the same approach as the TAPL book.

Notes

A key inspiration for this change is the original TAPL implementation, which uses a top-level syntax to define variables (see https://github.com/mspertus/TAPL/blob/main/untyped/test.f ). In that implementation, you first have to define a variable like x (using x/; ), which pushes x into the naming context. Then, you can use x as a free variable.

Since our implementation does not have such a top-level syntax, this PR introduces a Context type instead, which must be prepared manually by the user.

hiratara and others added 3 commits September 11, 2025 08:21
* Introduce `Context` to define free variables for parsing and display
* Add `parse_with_context` to to resolve free variables based on a `Context`
* Add `Term::with_context` for displaying terms using a given `Context`
Copy link
Owner

@ljedrz ljedrz left a comment

Choose a reason for hiding this comment

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

Yeah, I think this feature is long overdue in this repo; would you like to fix the clippy warnings and consider one suggestion I've left?

Thanks!

@hiratara
Copy link
Contributor Author

@ljedrz
Thanks for the review. I've fixed the warning in the context_methods test in commit b76efc2.

While I was at it, I also fixed a couple of other pre-existing lints in unrelated code (b8a8145, 1f59337), following the "Boy Scout Rule". These additional cleanups are not essential to this PR, so if you'd prefer a more focused commit history, I am perfectly happy to revert them. Just let me know!

Copy link
Owner

@ljedrz ljedrz left a comment

Choose a reason for hiding this comment

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

Looks good, thanks again!

@ljedrz ljedrz merged commit 4a73ed8 into ljedrz:master Sep 16, 2025
4 checks passed
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.

2 participants