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

Fix bug in bound variables check; add regression tests #25

Merged
merged 2 commits into from
Nov 6, 2024

Conversation

ulrikrasmussen
Copy link
Contributor

The current check only works reliably when all variables are either bound or unifies with a constant, but it sometimes fails to realize that a variable is unified via binary unifiers to a bound variable.

Sometimes a possibly unbound variable ends up being the representative for the set of variables that it unifies with, in which case subst.get() just returns the possibly unbound variable.

The solution is to 1) expand the set of known bound variables to include all variables in the image of the derived substitution, and 2) apply the substitution to a possibly unbound variable and check if the result is bound if the other checks fail.

There may be a more clever way of doing this which I haven't realized.

@aaronbembenek aaronbembenek merged commit 4e25a86 into HarvardPL:master Nov 6, 2024
1 check passed
@aaronbembenek
Copy link
Collaborator

Thanks for identifying this bug and fixing it -- much appreciated! :)

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