Skip to content

KBO: don't mistake a sort and a term with the same functor number for the same symbol - #1018

Merged
quickbeam123 merged 1 commit into
masterfrom
martin-fix-SWV645_5
Oct 12, 2026
Merged

quickbeam123 merged 1 commit into
masterfrom
martin-fix-SWV645_5

Conversation

@quickbeam123

Copy link
Copy Markdown
Collaborator

Type constructors and function symbols are numbered independently, so a sort and a term can share a functor number. KBO::compare (and the precedence fallback in State::result) only compared the numbers, so e.g. compare(complex, one_one(complex)) took the lexicographic path over a 0-ary sort and a 1-ary term and returned EQUAL. Cross-kind comparisons are legitimately made by TermPartialOrdering when ground joinability / term ordering diagrams relate type and term variables; the bogus EQUAL made the partial ordering inconsistent and crashed the prover.

Now both places also require the two terms to be of the same kind. This agrees with PrecedenceOrdering::comparePrecedences (type constructors below function symbols), which the term ordering diagrams already used, and makes KBO a consistent order on sorts and terms together.

Reproducer (release build, SIGSEGV after ~5 s before the fix):
vampire_z3_rel -sa otter Problems/SWV/SWV645_5.p
Unit test KBO/kbo_test24 fails before the fix on ASS(t1->arity()) in traverseLexBidir.

… the same symbol

Type constructors and function symbols are numbered independently, so a
sort and a term can share a functor number. KBO::compare (and the
precedence fallback in State::result) only compared the numbers, so e.g.
compare(complex, one_one(complex)) took the lexicographic path over a
0-ary sort and a 1-ary term and returned EQUAL. Cross-kind comparisons
are legitimately made by TermPartialOrdering when ground joinability /
term ordering diagrams relate type and term variables; the bogus EQUAL
made the partial ordering inconsistent and crashed the prover.

Now both places also require the two terms to be of the same kind. This
agrees with PrecedenceOrdering::comparePrecedences (type constructors
below function symbols), which the term ordering diagrams already used,
and makes KBO a consistent order on sorts and terms together.

Reproducer (release build, SIGSEGV after ~5 s before the fix):
  vampire_z3_rel -sa otter Problems/SWV/SWV645_5.p
Unit test KBO/kbo_test24 fails before the fix on ASS(t1->arity()) in
traverseLexBidir.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@quickbeam123
quickbeam123 requested a review from mezpusz October 12, 2026 09:02

@mezpusz mezpusz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

From the PR I cannot tell if these are all the places where we should do this, but it's definitely a good thing to start doing it in these two places!

@quickbeam123
quickbeam123 merged commit 12e12e8 into master Oct 12, 2026
1 check passed
@quickbeam123
quickbeam123 deleted the martin-fix-SWV645_5 branch October 12, 2026 09: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.

2 participants