fix(fixpoint): matchfix termination check must descend into all call arguments - #1144
Open
namasikanam wants to merge 1 commit into
Open
namasikanam wants to merge 1 commit into
namasikanam wants to merge 1 commit into
Conversation
…arguments
`trans_matchfix`'s `check_body` (ecHiInductive.ml) verified that a recursive
call's argument in the decreasing position is a structural subterm, but then
returned without inspecting the call's OTHER arguments. A non-decreasing
recursive call hidden in a sibling argument therefore escaped the check, e.g.
op f (n : t) (b : bool) : bool =
with n = Z => b
with n = S k => f k (! (f n b)).
Here the outer `f k (...)` decreases on `k`, so the check accepted it and never
looked at the second argument `! (f n b)`, which recurses on the whole input
`n`. This definition satisfies `f (S Z) b = ! (f (S Z) b)`, i.e. `f x = not f x`,
which has no total-function solution -- the theory becomes inconsistent.
Descend into every argument of an accepted recursive call (`List.iter
check_body args`) so nested non-decreasing calls are also checked. Legitimate
recursion (including nested decreasing calls like `f k (f k b)`) is unaffected;
the standard library still builds.
Regression: tests/matchfix-termination.ec (the bad fixpoint must be rejected).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The structural-recursion (termination) check for
op ... with ...fixpoint definitions acceptsa non-well-founded operator, because it does not descend into all arguments of a recursive
call. This lets one define
f x = ¬ f x, which has no total-function solution and makes thetheory inconsistent.
Reproducer (accepted on current
main)easycrypt compileaccepts this. The outer callf k (…)decreases onk(a subterm ofS k), so the guard accepts it and never inspects the sibling argument! (f n b), whichrecurses on the whole input
n(non-decreasing). The defining equations givef (S Z) b = f Z (¬ f (S Z) b) = ¬ f (S Z) b, i.e.X = ¬X— inconsistent.(Note: extracting a literal
.ecproof offalseis blocked because EC's normalizer loops onthe self-referential reduct; but accepting the definition is already the soundness break — no
total function
fsatisfies these equations. A control with the bad call placed directly, nothidden in a valid call's argument, is correctly rejected.)
Root cause
trans_matchfix'scheck_body(ecHiInductive.ml), on finding a recursive call whosedecreasing-position argument is a valid structural subterm, returns
()without checkingthe call's other arguments — so a non-decreasing recursive call hidden in a sibling argument
escapes.
Fix
Descend into all arguments of an accepted recursive call
(
-> List.iter check_body args). Legitimate recursion — including nested decreasing siblingcalls like
f k (f k b)— is unaffected.Validation
tests/matchfix-termination.ec(fail op …).