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 sub-derivations failure reporting in test-judgment-holds #205

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

wilbowma
Copy link
Collaborator

@wilbowma wilbowma commented Oct 5, 2019

closes #204

When a test-judgment-holds test fails for a modeless judgment, the error message
will incorrectly blame any sub-derivation that is not from the same judgment as
the top-level judgment.
@wilbowma
Copy link
Collaborator Author

wilbowma commented Oct 5, 2019

Rebased on master

@wilbowma wilbowma changed the title Only report proper sub-derivations from test-judgment-holds Fix sub-derivations failure reporting in test-judgment-holds Oct 5, 2019
@wilbowma
Copy link
Collaborator Author

wilbowma commented Oct 5, 2019

Added a new optional argument for test-judgment-holds to support reporting sub-derivations from other judgments.

@wilbowma
Copy link
Collaborator Author

wilbowma commented Oct 5, 2019

Not sure #:mutuals is the right name for that keyword... and I want something that will make sense when I scale this to moded judgments.

@rfindler
Copy link
Member

rfindler commented Oct 5, 2019 via email

@wilbowma
Copy link
Collaborator Author

wilbowma commented Oct 5, 2019 via email

@wilbowma
Copy link
Collaborator Author

wilbowma commented Oct 7, 2019

If we merge up to c3bfe1d, we'll get sound but incomplete information without a change in interface. This could be a good compromise for the time being, until I can figure out call-modeless-judgment.

@rfindler
Copy link
Member

rfindler commented Oct 8, 2019

Lets just leave it off master for now; people who are savvy enough to get the improvement outside of a release are savvy enough to get the commits from the pull request, I'd say.

@wilbowma
Copy link
Collaborator Author

For context, all of this was born out of this tutorial I wrote:
https://www.williamjbowman.com/doc/experimenting-with-redex/sec_judgment.html#%28part._.Testing_.Judgments%29

The hacks I wrote there work okay in userland code, but are painfully inefficient and rely on eval.

So far, looking over call-modeless-judgment, it seems quite hard to isolate exactly which premise fails, since many of the premises and sub-derivations get checked "all at once". For example, modeless-jf-process-rule-candidates checks all the patterns for all the sub-derivations at one time. I'm sure there's a reason for this that I don't understand yet. But it means I can't report which "one" failed to match without significant and possibly inefficient refactoring.

Also, collecting lists of derivations is making the code ugly and unmaintainable so

@rfindler
Copy link
Member

rfindler commented Oct 11, 2019

It checks things one rule at a time. So it checks the conclusion of the rule and then it does a single match to check all the premises of the rules. If that match worked, it moves on, recursively, to try to check the rules for the subderivations.

The reason it has to do that is you can write an ellipsis "between" premises and we need redex's matcher's functionality to actually do that matching properly. So we take the premises and build up a single pattern for the "above the rule part" (with some new names added in to be able to pull out things later), do that match, and then pull out what are supposed to be conclusions (using those new names) for the next layer in the derivation. (At least I think that's the strategy I settled on. I tried a few before that that didn't work out.)

Does this make sense?

[ edit was to fix the end of the first paragraph ]

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.

test-judgment-holds incorrectly blames sub-derivations from other judgments
2 participants