Skip to content

Tuples: support them on the classic clausification pathway, and fix the existing tuple machinery - #901

Open
quickbeam123 wants to merge 10 commits into
masterfrom
martin-tuples-newcnf-and-old
Open

Tuples: support them on the classic clausification pathway, and fix the existing tuple machinery#901
quickbeam123 wants to merge 10 commits into
masterfrom
martin-tuples-newcnf-and-old

Conversation

@quickbeam123

Copy link
Copy Markdown
Collaborator

Why

TFX tuples ([$int,$int] sorts, [a,b] terms, $let([a,b] := t, ...) bindings) were reachable
only with -newcnf on: the parser rejected every tuple construct otherwise, and FOOLElimination
answered a tuple binding with NOT_IMPLEMENTED. Since newcnf is off by default, a TFX problem
using tuples died at parse time in the default strategy and in every portfolio slice that does not
turn newcnf on.

The newcnf pathway, which was supposed to be the one that works, turned out to be broken on
essentially every tuple shape that the two existing test problems do not cover. Of the 46 TPTP
problems that use tuple bindings, 20 crashed a debug build under -newcnf on --mode clausify.
Three-line inputs were enough to hit assertion violations, out-of-bounds reads and undefined
behaviour, in the kernel tuple machinery, in the parser, and in NewCNF itself.

What this PR does

Two things, in eight self-contained commits (each commit message explains its own bug and carries
the exact pre-fix reproducer):

1. Tuple $let is implemented in FOOLElimination, and the parser gate is removed, so tuples
now work on both clausification pathways. The implementation does not duplicate any let machinery:
it de-tuplifies a tuple binding into a nest of ordinary, single-symbol bindings and re-enters
process() on the result,

$let([c1,...,cn] := [s1,...,sn], t)
  ~~> $let(c1 := s1, ... $let(cn := sn, t) ... )

$let([c1,...,cn] := s, t)
  ~~> $let(g := s, $let(c1 := proj_1(g), ... $let(cn := proj_n(g), t) ... ))

where g is a fresh constant of the tuple sort and proj_i are the destructors of the tuple term
algebra (whose axioms TheoryAxioms already emits, since the algebra is registered at parse time).
Everything else — free variables, the symbol-renaming shortcut, definition introduction, Boolean
bindings — is the existing, well-exercised code path.

Two properties make this work out cleanly:

  • sequentialising the simultaneous binding is sound because c1,...,cn cannot occur in s: the
    parser only publishes them once the whole definition group has been read, so a use of their name
    in s refers to a symbol of an enclosing scope. The code says so, with a pointer to the place
    in Parse/TPTP.cpp that guarantees it;
  • a Boolean component needs no special case. It is a nullary predicate whose right hand side is the
    $o-sorted term proj_i(g), and process() already turns such a term into proj_i(g) = $true.
    This is precisely where NewCNF went wrong (it built a literal out of a function functor), so the
    new code avoids the bug by construction rather than by copying and patching.

g is declared over exactly the type variables of the tuple sort (none, in the monomorphic case),
so its arguments are always variables and the generic path never has to match a concrete sort.

2. The tuple machinery that both pathways share is fixed. The bugs were: a projection lookup in
Kernel/Theory that never matched a real projection while matching unrelated symbols, and then read
past the end of the destructor array; a "is this a tuple constructor?" query that extended the
signature as a side effect; a parser that decided "this is a tuple binding" from the result sort, so
an ordinary binding of a tuple-sorted symbol dereferenced a null list; a parser that could not read a
tuple binding inside a simultaneous definition group at all; and two NewCNF defects around Boolean
components and the arity of the constant naming a de-tuplified binding.

Effect

  • tuple problems now parse and clausify without -newcnf on: all 45 first-order TPTP problems with
    tuple bindings clausify cleanly on the FOOLElimination pathway, none of which could previously be
    parsed at all;
  • on the newcnf pathway the same corpus goes from 20 crashes to none;
  • on non-tuple problems nothing changes: clausifier output is byte-identical to master on samples
    from problemsTX0.txt and problemsTX1.txt, with and without newcnf.

The two pathways produce different (though each correct) clause sets for the same tuple problem, as
they do for other FOOL constructs; no attempt is made to make them agree.

Testing

  • UnitTests/tTuple.cpp — new unit test for the kernel-level tuple invariants; its cases fail
    before the corresponding commits and pass after;
  • thirteen new problems in checks/theory/, covering monomorphic and polymorphic tuple bindings,
    tuple and non-tuple right hand sides, Boolean components, variable right hand sides, nesting,
    simultaneous groups, shadowing, and tuple terms outside any $let — wired into checks/sanity
    for both pathways and, where applicable, with -ile off;
  • full ctest passes (100/100).

Notes for the reviewer

  • checks/sanity has a pre-existing failure earlier in the script,
    Problems/DAT/DAT023_1.p with -alasca on -alascai on -thf on (assertion at
    Kernel/Inference.cpp:461), which aborts the run before it reaches the tuple section. It fails on
    master too and is untouched here; I verified the tuple and FOOL sections separately.
  • Unrelated, unfixed, noticed in passing: after a TPTP USER_ERROR the input-syntax auto-detection
    falls back to SMTLIB2 and asserts in Shell/LispParser.hpp:109.
  • The now-live projection lookup would have switched on $proj(i, t) printing, which TPTP cannot
    read back; that printing is removed rather than resurrected. Projections print as the ordinary
    function symbols they are.

@quickbeam123
quickbeam123 requested a review from mezpusz August 13, 2026 08:17
Comment thread Kernel/Term.cpp Outdated
Comment thread Kernel/Theory.cpp Outdated
return false;
}

// the arity of the tuple is the arity of its sort, not the number of type

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.

I think this might be false. A polymorphic n-tuple has n type arguments and n term arguments, e.g. tuple_ctor2 : !>[X0 : $tType, X1 : $tType] : (X0 * X1) > tuple2(X0, X1), where tuple2 is the type constructor for 2 tuples. An example term is tuple_ctor2($int, $i, 2, f(a)) with 4 arguments.

@quickbeam123 quickbeam123 Aug 14, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

"The worry is dispelled — the code is right, but the reviewer was reacting to a genuinely misleading comment."

(More in a new commit, we new tests coming.)

Comment thread Parse/TPTP.cpp
quickbeam123 and others added 10 commits August 14, 2026 11:54
…unds

findTupleProjection had three defects:

* it tested projType->arity() != 1, but OperatorType::arity() counts the
  type arguments as well, so a genuine projection (n type args + 1 term
  arg) never passed the test, while any *monomorphic* unary symbol over a
  tuple sort did;
* it then read projType->arg(0), which for a polymorphic projection is a
  type argument rather than the tuple argument;
* it iterated up to c->arity(), which for the tuple constructor is 2n (n
  type args + n term args), while destructorFunctor asserts an index
  below n -- so the loop read past the end of the destructor array.

Together this meant that printing the type declaration of, say,
h: [$int,$int,$int] > $int aborted in a debug build (and picked up a
garbage functor in a release build).

Fix by rejecting predicates outright (destructors are always functions,
also for Boolean components), gating on termAlgebraDest(), discounting
the type arguments in both the arity test and the argument lookup, and
bounding the destructor loop by the constructor's number of term
arguments.  The same wrong bound in getTupleProjectionFunctor's
assertion is fixed too.

With the function actually returning true, the "$proj(i, t)" printing in
Term::headToString and Literal::toString becomes live -- but TPTP has no
surface syntax for projections, so that output could not be read back
in.  Drop it; projections now print as the ordinary function symbols
they are.

Reproduce before the fix (assertion at Shell/TermAlgebra.hpp:47, note
that the message is lost unless run under a pty):

  script -q /dev/null ./build-debug/vampire -newcnf on --mode clausify \
    -t 5 checks/theory/tuple-proj-print.p
  script -q /dev/null ./build-debug/vampire -newcnf on --mode clausify \
    -t 5 Problems/SWW/SWW998_1.p

Tests: UnitTests/tTuple.cpp (both new test cases fail before this
commit) and a checks/sanity exact-output check on the new problem, which
produced no output at all before the fix.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
isTupleConstructor asked getTupleConstructor(t->numTypeArguments()) for
any term of a tuple sort, and getTupleConstructor *registers* the tuple
term algebra of that arity in the signature.  So merely asking whether a
term is a tuple constructor could extend the signature -- for a plain
constant of a tuple sort (0 type arguments) it registered a bogus arity-0
tuple algebra, with the corresponding term algebra axioms.

Answer cheaply and without side effects instead: reject anything that is
not a term algebra constructor first, then take the tuple arity from the
*sort* rather than from the term's type argument count.

This also makes the query safe to ask about terms whose type argument
count differs from the tuple arity, which the upcoming de-tuplification
in FOOLElimination relies on.

Reproduce before the fix with

  tff(g_type,type,g: [$int,$int]).
  tff(p_type,type,p: [$int,$int] > $o).
  tff(c,conjecture, $let(x: [$int,$int], x := g, p(x))).

run as

  ./build-debug/vampire -newcnf on -t 5 <file>

which registers 'Tuple'/0 and saturates on its exhaustiveness axiom
(tuple = X0) although the problem has no arity-0 tuple.

Test: UnitTests/tTuple.cpp, isTupleConstructor_does_not_touch_the_signature
(fails before this commit).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
TPTP::endLet decided whether a definition was a tuple binding
([c1,...,cn] := t) by looking at the *result sort* of the bound symbol.
But an ordinary single-symbol binding may have a tuple sort as well, as
in $let(x: [$int,$int], x := g, p(x)).  Such a binding then took the
tuple branch, where it iterated the (null) list of bound constants.

Record instead, in LetSymbolReference, whether the reference came from
TPTP::tupleDefinition, and use that in endLet.

Reproduce before the fix (undefined behaviour; the parse then happens to
continue with the right result, so only a sanitizer build complains):

  ./build-debug/vampire -newcnf on -t 5 checks/theory/let-tuple-sorted-const.p

  Parse/TPTP.cpp:2712:27: runtime error: member call on null pointer of
  type 'Lib::List<unsigned int>'

Test: checks/theory/let-tuple-sorted-const.p, added to checks/sanity for
both clausification pathways (checks/sanity fails on the UBSan message,
which run_vampire picks up from stderr).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
When de-tuplifying $let([c1,...,cn] := s, body) for a non-tuple s, NewCNF
binds each ci to a projection out of a fresh tuple constant.  For a
Boolean ci it built that projection with Literal::create -- but tuple
projections are *functions*, also when the projected component has sort
$o, so the projection functor was used to index the predicate signature.

Just build the function term proj_i(...,tuple) in all cases:
SymbolDefinitionInlining already wraps a $o-sorted term into a formula
(via BoolTermFormula::create) where a formula is expected, so the special
case was not only wrong but unnecessary.

Reproduce before the fix:

  script -q /dev/null ./build-debug/vampire -newcnf on -t 10 \
    checks/theory/let-tuple-bool-nontuple.p

  Condition env.signature->predicateArity(predicate) == arity at
  location Kernel/Term.cpp:1566 was violated

(checks/theory/let-tuple-bool.p does not catch this: its right hand side
is a tuple literal, which takes the other branch of the de-tuplification.)

Test: checks/theory/let-tuple-bool-nontuple.p, added to checks/sanity.
The -ile off variant of the same problem still fails, for an unrelated
reason fixed in the next commit.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
De-tuplifying $let([c1,...,cn] := s, body) for a non-tuple s introduces a
fresh constant standing for s.  It was declared with n type arguments (as
many as the tuple has components) and applied to the tuple's actual type
arguments, so in the monomorphic case it was a spuriously polymorphic
symbol applied to ground sorts.  nameLetBinding, which handles the
resulting binding when the let is not inlined, then re-applied that
symbol to the bound and free variables of the binding -- none, for a
ground right hand side -- and Term::create hit an arity mismatch.

Fix both ends:

* declare the fresh constant over exactly the type variables occurring in
  the tuple sort (none, in the monomorphic case);
* in nameLetBinding, when the bound symbol is kept rather than renamed,
  apply it to its own arguments instead of to the bound and free
  variables, which need not coincide.

Reproduce before the fix:

  script -q /dev/null ./build-debug/vampire -newcnf on -ile off -t 10 \
    checks/theory/let-tuple-nontuple.p

  Condition env.signature->functionArity(function) == arity at location
  Kernel/Term.cpp:1061 was violated, as:
  env.signature->functionArity(function) == 2
  arity == 0

checks/theory/let-tuple.p did not catch this because there the two free
type variables of the binding happen to match the two type arguments.

Over the 46 TPTP problems using tuple bindings, "-newcnf on --mode
clausify -t 5" now crashes on none of them (20 crashed before this
series; the one remaining exit-4 is a THF parse error in SYN000^2.p,
unrelated to tuples).

Tests: checks/theory/let-tuple-nontuple.p and the -ile off variant of
checks/theory/let-tuple-bool-nontuple.p, added to checks/sanity.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
When replacing occurrences of the symbol being replaced, the arguments of
the old application were matched against the arguments of the occurrence
after running them through process(Term*) -- which starts with
ASS(!term->isSort()).  The generic argument loops already skipped sorts;
the two matching loops did not.

This is currently unreachable, because the $let binders the parser builds
apply the bound symbol to type *variables*.  It becomes reachable as soon
as a binder carries a concrete type argument, which is exactly what
de-tuplifying a monomorphic tuple binding would produce if the fresh
constant were declared polymorphically.

Factor the "leave variables and sorts alone" logic into processArgument
and use it in all four places.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…arser

Tuples were handled by NewCNF only: FOOLElimination hit NOT_IMPLEMENTED
on a tuple binding, and the TPTP parser refused tuple terms and tuple
bindings altogether unless -newcnf on was given.  As newcnf is off by
default, any TFX problem using tuples died at parse time in the default
strategy.

FOOLElimination now de-tuplifies a tuple binding into a nest of ordinary,
single-symbol bindings and re-enters process() on the result, so all the
existing machinery (free variables, symbol renaming, definition
introduction, Boolean bindings) is reused as is:

  $let([c1,...,cn] := [s1,...,sn], t)
    ~~> $let(c1 := s1, ... $let(cn := sn, t) ... )

  $let([c1,...,cn] := s, t)
    ~~> $let(g := s, $let(c1 := proj_1(g), ... $let(cn := proj_n(g), t) ... ))

where g is a fresh constant of the tuple sort and proj_i are the
destructors of the tuple term algebra (whose axioms are already added by
TheoryAxioms, as the algebra is registered at parse time).

Turning the simultaneous binding into a nest is sound because c1,...,cn
cannot occur in s: TPTP::endDefinition only publishes them once the whole
definition group has been read, so a use of their name in s refers to a
symbol of an enclosing scope.  A Boolean component needs no special case:
it is a nullary predicate whose right hand side is the $o-sorted term
proj_i(g), which process() turns into proj_i(g) = $true.

g is declared over exactly the type variables of the tuple sort (none, in
the monomorphic case), so that its arguments are always variables and the
generic path never has to match a concrete sort.

Reproduce before this commit:

  ./build-debug/vampire -t 10 checks/theory/let-tuple.p
  % User error: Set --newcnf on if using tuples

and, with the two parser guards removed but without the FOOLElimination
part, a NOT_IMPLEMENTED from Shell/FOOLElimination.cpp.

Over the 45 first-order TPTP problems that use tuple bindings,
"--mode clausify -t 5" without newcnf now succeeds on all of them (none
of them could be parsed at all before).

Tests: checks/theory/let-tuple-{mono,var,nested,arg,shadow,poly-nontuple}.p,
plus the pre-existing and newly added tuple problems, all added to
checks/sanity for both clausification pathways.

Known limitation, not addressed here: a tuple binding inside a
simultaneous definition group, $let([a: $int,b: $int,f: $int > $int],
[[a,b] := [1,1], f(X) := X], ...), is rejected by the parser on both
pathways.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
TPTP::tupleDefinition expects the first constant of the tuple on the
_strings stack, with the comma after it already consumed -- which is what
TPTP::definition does when a tuple binding is the only definition.  For a
tuple binding inside a group of simultaneous definitions, neither
TPTP::definition nor TPTP::midDefinition consumed that name, so
tupleDefinition popped an unrelated string and complained about it.

Reproduce before the fix with

  tff(p_type,type,p: ($int * $int) > $o).
  tff(c,conjecture,
      $let([a: $int, b: $int, f: $int > $int],
           [[a,b] := [1,1], f(X) := X],
           p(f(a),b) ) ).

run as ./build-debug/vampire -newcnf on -t 10 <file>:

  % User error: Constant $let is used in a tuple let definition without
    a declared sort

(and the same for a tuple binding in a later position of the group).

Tests: checks/theory/let-tuple-simultaneous.p (tuple binding first) and
checks/theory/let-tuple-simultaneous2.p (tuple binding second), added to
checks/sanity for both clausification pathways.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The comment on the tuple arity read as if it claimed a tuple constructor
term had no type arguments.  It does: an n-tuple constructor has n type
arguments and n term arguments, hence signature arity 2n, and the arity
we need, n, is the arity of the tuple sort.  The "0 type arguments" case
was about the terms the query gets *rejected* on -- a plain constant of a
tuple sort -- where asking getTupleConstructor(0) would register a bogus
arity-0 tuple algebra (the bug fixed in 61302ca).

No behavioural change; the code was already correct.  Two tests in
UnitTests/tTuple.cpp pin it down: a heterogeneous pair
tuple($int, s, i, f(a)), spelling out arity 4 / 2 type arguments / sort
arity 2, and a nested pair Tuple(Tuple(s,s,s), s), where only the outer
arity is the right one.  Both fail if the arity is taken from t->arity().

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@quickbeam123
quickbeam123 force-pushed the martin-tuples-newcnf-and-old branch from da8bf96 to 1fd7163 Compare August 14, 2026 11:54
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