Skip to content

drule.ml: fail term_match early when a substitution would let a bound variable escape - #191

Open
aqjune-aws wants to merge 2 commits into
jrh13:masterfrom
aqjune-aws:term_pmatch-earlyfail
Open

aqjune-aws wants to merge 2 commits into
jrh13:masterfrom
aqjune-aws:term_pmatch-earlyfail

Conversation

@aqjune-aws

@aqjune-aws aqjune-aws commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

In term_pmatch, a pattern variable that is in neither "env" nor lconsts was
assigned whatever subterm it met, including one with a free occurrence of a
variable bound over it on the target side. Such an assignment can never
reproduce the target, since INSTANTIATE renames the pattern's own binder rather
than capturing the variable, so PART_MATCH's closing alpha check rejected it --
but only after a full INSTANTIATE had already built a theorem.

That is what makes the basic rewrites EXISTS_SIMP and FORALL_SIMP
((?x. t) <=> t, (!x. t) <=> t) so expensive on s2n-bignum-shaped goals
(awslabs/s2n-bignum#209): each failing attempt
against a ?y. body[y] first ran a capture-avoiding substitution over the body
-- a "variant" scan, a vsubst traversal, a kernel INST and a failing ALPHA,
several traversals of the body in all. A vfree_in check against env in the
inserter replaces that with a single vfree_in scan, which stops at the first
occurrence of the bound variable: effectively constant time when the body
mentions y early, and still linear in the body but much cheaper when y occurs
only at the very end. Where the check does not fire it costs one extra scan of
the matched subterm, so a match that does succeed against a large subterm gets
somewhat slower.

term_match itself keeps its permissive behavior, which its documentation
already warns about, so the check is exposed as a separate term_match_checked
and only PART_MATCH and GEN_PART_MATCH are switched over. Every other caller is
unaffected; in particular net_of_thm still classifies permutative rewrites with
the permissive matcher, so rewrite nets come out exactly as before. The check
covers the first order case only -- the higher order paths in term_homatch can
still build an escaping instantiation -- and PART_MATCH's alpha check is
retained as the real guarantee. Which matches succeed should be unchanged.

UnitTests/basic_tests.ml gains five checks: term_match still accepts the
escaping match, term_match_checked rejects it, the two agree on a first order
match, REWR_CONV EXISTS_SIMP still fails on ?y. P y, and it still succeeds on
?y. T. Help/term_match_checked.hlp documents the new function, and
Help/term_match.hlp points at it.

s2n-bignum proofs speedup (arm): comparison.csv
holtest speedup: holtest_result.txt

@aqjune-aws
aqjune-aws force-pushed the term_pmatch-earlyfail branch 2 times, most recently from 82a138b to 7a3a0ca Compare July 12, 2026 23:21
@aqjune-aws
aqjune-aws marked this pull request as ready for review July 13, 2026 02:37
@aqjune-aws

Copy link
Copy Markdown
Contributor Author

"Hide whitespace" from the diff display option will make review significantly easier :)
image

@aqjune-aws
aqjune-aws force-pushed the term_pmatch-earlyfail branch from 7a3a0ca to 41cb9b0 Compare July 29, 2026 17:49
…ariable escape

In term_pmatch, a pattern variable that is in neither "env" nor lconsts was
assigned whatever subterm it met, including one with a free occurrence of a
variable bound over it on the target side. Such an assignment can never
reproduce the target, since INSTANTIATE renames the pattern's own binder rather
than capturing the variable, so PART_MATCH's closing alpha check rejected it --
but only after a full INSTANTIATE had already built a theorem.

That is what makes the basic rewrites EXISTS_SIMP and FORALL_SIMP
(`(?x. t) <=> t`, `(!x. t) <=> t`) so expensive on s2n-bignum-shaped goals
(awslabs/s2n-bignum#209): each failing attempt
against a `?y. body[y]` first ran a capture-avoiding substitution over the body
-- a "variant" scan, a vsubst traversal, a kernel INST and a failing ALPHA,
several traversals of the body in all. A vfree_in check against env in the
inserter replaces that with a single vfree_in scan, which stops at the first
occurrence of the bound variable: effectively constant time when the body
mentions y early, and still linear in the body but much cheaper when y occurs
only at the very end. Where the check does not fire it costs one extra scan of
the matched subterm, so a match that does succeed against a large subterm gets
somewhat slower.

term_match itself keeps its permissive behavior, which its documentation
already warns about, so the check is exposed as a separate term_match_checked
and only PART_MATCH and GEN_PART_MATCH are switched over. Every other caller is
unaffected; in particular net_of_thm still classifies permutative rewrites with
the permissive matcher, so rewrite nets come out exactly as before. The check
covers the first order case only -- the higher order paths in term_homatch can
still build an escaping instantiation -- and PART_MATCH's alpha check is
retained as the real guarantee. Which matches succeed should be unchanged.

UnitTests/basic_tests.ml gains five checks: term_match still accepts the
escaping match, term_match_checked rejects it, the two agree on a first order
match, REWR_CONV EXISTS_SIMP still fails on `?y. P y`, and it still succeeds on
`?y. T`. Help/term_match_checked.hlp documents the new function, and
Help/term_match.hlp points at it.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@aqjune-aws
aqjune-aws force-pushed the term_pmatch-earlyfail branch from 41cb9b0 to 10d32dd Compare August 19, 2026 03:30
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.

1 participant