This is another PR towards the goal of getting Z3 to compile cleanly
when included via FetchContents into clang-tidy, which uses a pretty
strict set of warnings.
This is a second version of https://github.com/Z3Prover/z3/pull/9957. I
address @NikolajBjorner 's comments about not changing the semicolons
after macro invocations, because some editors work better with them
present. It now, to the best of my ability, only deletes semis:
* after the closing brace of namespace decl.
* after the closing brace of an extern "C" decl.
* after a function definition.
This PR is very large, but it consists entirely of deletions of
semicolons in these situations.
(If there was a way to update the previous PR, which had been closed,
and that is preferable, please let me know. I couldn't figure it out.)
## Summary
Follow-up to #10001 addressing @NikolajBjorner's review comment:
> isn't this nearly identical AI generated code to the other file? There
has to be some modular approach to deal with sorting vectors?
#10001 introduced two nearly-identical copies of a bounds-safe,
mutation-aware index-permutation merge sort:
- `algebraic_numbers.cpp::merge_sort_roots_perm`
- `nlsat/levelwise.cpp::merge_sort_perm`
Both exist because the comparator (`anum_manager::compare`/`lt`) is
**not pure**: it mutates the algebraic numbers it compares (refining
isolating intervals) and may throw on the resource limit, which makes
`std::sort` undefined behavior (the original SIGSEGV).
## Change
Extract the algorithm into a single shared helper
`util/index_sort_with_mutations.h` (`stable_index_merge_sort`). The long
rationale for why `std::sort` is unsafe and merge sort is safe now lives
in exactly one place. Both call sites become thin wrappers that build
the scratch buffer and forward their local comparator.
No behavioral change: same stable O(n log n) merge sort over an index
permutation.
## Verification
CMake/Ninja Release build:
- `test-z3 /seq algebraic_numbers` — PASS
- `test-z3 /seq algebraic` — PASS
- NRA/NIA smoke solves with `nlsat.lws=true` return expected sat/unsat.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace goto-based control flow in get_cube_delta_for_term with an
all_ok flag for structured early-exit. Use aggregate initialization for
flip_candidate, constructor-based vector sizing for occs, brace
initialization for pairs in add_edge_rows_for_term.
No functional changes - all lcube tests pass.
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
## Summary
Alternative to #9991. Instead of disabling `nlsat.lws` by default, this
**fixes the underlying bug** so levelwise single-cell projection stays
enabled.
## Root cause
The crash was reproduced on the QF_NIA benchmark from #9991
(`20170427-VeryMax/ITS/From_AProVE_2014__Round3.jar-obl-8__p11898_terminationG_0.smt2`,
~40% SIGSEGV at `-T:20`). A core-dump backtrace points at:
```
mpbq_manager::le (mpbq.cpp:362)
algebraic_numbers::manager:👿:compare (algebraic_numbers.cpp:1913) c = 0xea24052d29f2d500 <- wild pointer
algebraic_numbers::manager:👿:compare (algebraic_numbers.cpp:2128)
nlsat::levelwise::impl::root_function_lt (levelwise.cpp:949)
... std::__unguarded_linear_insert ... <- OOB read
std::sort
nlsat::levelwise::impl::sort_root_function_partitions
```
The comparator (`root_function_lt` → `anum_manager::compare`, and
`anum_manager::lt`) **refines the isolating intervals of the algebraic
numbers it compares** and may **hit the resource limit (throwing)**
mid-comparison. Both make the order it induces non-deterministic / not a
strict weak ordering across a single `std::sort` — undefined behavior.
libstdc++'s *unguarded* insertion pass then walks past `begin()` and
dereferences a wild anum cell → SIGSEGV. This only fires when a timeout
interrupts levelwise, explaining the non-determinism (`signal-11`).
## Fix
Replace the two affected `std::sort` calls
(`sort_root_function_partitions` and `add_adjacent_root_resultants`)
with a **bounds-checked insertion sort over an index permutation**. A
fully guarded insertion sort can never read out of bounds regardless of
comparator consistency, and unwinds cleanly if `compare` throws on
cancellation. The partitions sorted here are small, so the O(n²) cost is
negligible.
`nlsat.lws` stays `true`.
## Verification
On the Linux repro box (Ubuntu 24.04, g++ 13), RelWithDebInfo:
- **Before:** ~40% SIGSEGV (e.g. 5/16 runs at `-T:20`).
- **After:** **0/30** SIGSEGV; results are `unsat`/`timeout`.
- Sanity batch over 25 QF_NIA/VeryMax/ITS files: no crashes, expected
sat/unsat/timeout mix.
- `model_validate=true` full solve still returns `unsat`.
---------
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Capture row as a pointer as lambda strips the reference and the vector was copied by value in lar_solver!
---------
Signed-off-by: Lev Nachmanson <levnach@hotmail.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The `Ubuntu build - cmake - debugGcc` job was failing because the solver
could emit an unexpected `check-assignment` line before normal
satisfiability output. This change removes that stray output so debug
GCC runs no longer contaminate expected CLI/results streams.
- **Root cause**
- `src/math/lp/nra_solver.cpp` printed `check-assignment` from
`solver::check_assignment()` via `IF_VERBOSE(0, ...)`.
- Verbosity level `0` made this effectively unconditional in the failing
path, so debug builds could leak internal diagnostics into user-visible
output.
- **Change**
- Remove the `check-assignment` print from the exception path in
`lp::solver::check_assignment()`.
- Preserve all existing control flow and error handling; only the
unintended output side effect is removed.
- **Effect**
- Debug GCC CMake builds keep their normal `sat`/`unsat` output shape.
- Internal solver diagnostics no longer interfere with output-sensitive
CI checks.
```c++
catch (z3_exception &) {
statistics &st = m_imp->m_nla_core.lp_settings().stats().m_st;
m_imp->m_nlsat->collect_statistics(st);
if (m_imp->m_limit.is_canceled()) {
return l_undef;
}
else {
throw;
}
}
```
---------
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
## Summary
Improves the Diophantine (`dio`) integer-feasibility controller in
`int_solver`, and fixes a latent bug where the user's Gomory-cut
configuration could be silently overridden at runtime. Also includes the
earlier `lia_w` work: randomized hammer gates, the `int_hammer_period` /
`random_hammers` parameters, and the linear `dio_calls_period` recovery.
## Motivation
The controller used a **single field** both as the static
`lp.dio_cuts_enable_gomory` parameter and as the live "is Gomory
running" flag. It started running Gomory (and the gcd test) once
`dio_calls_period` crossed a hard-coded `16`. Because `dio_calls_period`
is also driven by the randomized hammer gate, on instances where `dio`
is only intermittently productive the period could be ratcheted past 16
*by chance*, turning on Gomory + gcd and thrashing — e.g. `dillig/20-14`
went from a 100s solve (deterministic) to a 600s timeout (randomized)
purely from this spurious activation.
## Changes
- **Separate config from runtime state.** Split the shared field into
`m_dio_cuts_enable_gomory` (static config, never mutated) and
`m_run_gomory_with_dio` (runtime flag). Toggling the runtime state can
no longer clobber the user's `dio_cuts_enable_gomory` parameter.
- **Trigger on genuine dio failures, not the period proxy.** Running
Gomory-with-dio now starts after a count of **consecutive `undef` dio
returns** (reset on a dio conflict) rather than when the
randomization-inflated period crosses a threshold — robust to
`random_hammers` gate variance.
- **Parameterize the threshold.** New `lp.dio_gomory_enable_period`
(default 16). Set it very large to never auto-start Gomory, so Gomory
follows `dio_cuts_enable_gomory` only.
- **Try `dio` before Gomory** in `check()` so a productive dio conflict
preempts Gomory on dio-dominated instances.
## Evaluation (QF_LIA, full set, 600s, seed 555 paired)
- Dio-before-Gomory: **+33** problems across the 6 `random_hammers x
int_hammer_period` cells (5/6 cells improve).
- New trigger (`dio_gomory_enable_period=32`, random): **6417** vs the
period-16 baseline **6409**; no short-cutoff regression.
- Linear `dio_calls_period` recovery: keeping it on is worth ~+20 vs
off; `decrease=1` slightly ahead of the default 2.
Default behavior (`dio_gomory_enable_period=16`) is byte-for-byte
equivalent to the previous threshold logic.
## Notes
Debug-only tracing used during analysis (the `dio_calls_period_trace`
parameter plus per-hammer / period-evolution verbose output) is **not**
included.
---------
Signed-off-by: Lev Nachmanson <levnach@hotmail.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This is another PR towards the goal of getting Z3 to compile cleanly
when included via FetchContents into clang-tidy, which uses a pretty
strict set of warnings.
The PR adds
```
"-Wsuggest-override"
"-Winconsistent-missing-override"
```
to the CLANG_ONLY_WARNINGS. This exposes a relatively small number of places where method overrides did not use the "override" keyword. The PR fixes those.
(In cmd_util.h, I also made the *_CMD macros be uniform in not ending the class they define with a semicolon; the invocation of the macro can add the semicolon.)
Implemented the largest cube heuristic from Bromberger and Weidenbach's
paper on cubes. Also fixes an overflow bug in mzp.
Use vswhere to find the visual studio version on windows in the build's ymls.
---------
Signed-off-by: Lev Nachmanson <levnach@hotmail.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
MSVC ASan reports showed a container-overflow in LP tableau pivoting,
reproducible from both examples and solver tests (issue #9781). The
failure came from reading a `column_cell` through a reference after
pivoting removed that entry from the backing column.
- **Root cause**
- `pivot_column_tableau` and the analogous Diophantine elimination loop
both held `auto& c = column.back()` across a call
(`pivot_row_to_row_given_cell`) that immediately removes that very cell
from the column via `remove_element`.
- After the mutation, the subsequent read `c.var()` used for bookkeeping
observed invalid memory.
- **Change**
- Record the affected row in the bookkeeping set (`m_touched_rows` /
`m_changed_rows`) by reading `c.var()` **before** the pivot call, while
the back cell is still valid.
- Make `static_matrix::pivot_row_to_row_given_cell` return `void`
instead of `bool`. Its result (`!rowii.empty()`) was always `true`: both
callers keep the matrix at full row rank (the tableau basis columns form
an identity submatrix; the Diophantine `m_l_matrix` stays invertible),
so an elementary row operation can never empty a row. The dead `if
(!...) return false;` early-exit in `pivot_column_tableau` is removed
and replaced with a `SASSERT(!rowii.empty())` documenting the invariant.
- **Affected code paths**
- `src/math/lp/static_matrix.h`, `src/math/lp/static_matrix.cpp`,
`src/math/lp/static_matrix_def.h`
- `src/math/lp/lp_core_solver_base_def.h`
- `src/math/lp/dioph_eq.cpp`
- **Behavioral impact**
- No algorithmic change to pivoting.
- Removes the stale-reference hazard in the loops that repeatedly
eliminate entries from a column.
```c++
while (column.size() > 1) {
auto& c = column.back();
SASSERT(c.var() != piv_row_index);
if (m_touched_rows != nullptr)
m_touched_rows->insert(c.var());
m_A.pivot_row_to_row_given_cell(piv_row_index, c, j);
}
```
- **Verification**
- Reproduced the exact issue #9781 failure on a local ASan build
(`container-overflow` in `pivot_column_tableau`) using the pre-fix code,
and confirmed it is gone with this change.
- The 4 reported tests pass clean under ASan: `c_example`,
`cpp_example`, `test-z3 get_implied_equalities`, `test-z3 quant_solve`.
- Full `test-z3 /a` suite: 89 passed, 0 failed, 0 ASan errors.
---------
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: Nikolaj Bjorner <nbjorner@microsoft.com>
Co-authored-by: Lev Nachmanson <levnach@hotmail.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
A `root-obj`-driven unsat case was exiting with a leaked `mpz_manager`
allocation even though solver output was correct. The leak came from
temporary rational bounds created during algebraic-number comparison and
not released before shutdown.
- **Root cause**
- `algebraic_numbers::compare_core()` materialized interval bounds as
raw `mpq` temporaries.
- Those temporaries could allocate backing `mpz` storage, but their
lifetime was not tied to the manager, so the allocator retained leaked
cells at process exit.
- **Change**
- Replace the raw `mpq` temporaries with `scoped_mpq` in
`/src/math/polynomial/algebraic_numbers.cpp`.
- This keeps the comparison logic unchanged while making temporary bound
conversion use RAII-managed cleanup.
- **Effect**
- `root-obj` comparisons no longer leave `mpz_manager` allocations
behind.
- Solver behavior is unchanged; the fix is limited to temporary numeral
lifetime management.
```c++
- mpq l_a, u_a, l_b, u_b;
+ scoped_mpq l_a(qm()), u_a(qm()), l_b(qm()), u_b(qm());
```
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
So far, `algebraic_numbers compare_core ` handles an edge case
incorrectly:
- If the two compared numbers (`a`, `b`) are different,
- the intervals still overlap after refinements, and
- both a and b are a root of the second polynomial (`cell_b->m_p`), e.g.
they are the first and second root
then the method would return `sign_zero` (i.e. "equal"). This behavior
can be replicated with the provided test case (before the fix). This
requires `algebraic.factor=false`, though i first encountered it during
solver runs on QF_NRA instances with the default
`algebraic.factor=true`, which apparently means that the polynomials for
anums are still not always factored.
The fix is to compare the interval bounds of b to a and vice versa. Then
the Sturm-Tarski check is only run if `a` and `b` both lie in the
intersection of the intervals, because only then is it guaranteed to be
correct.
Adds a new lemma pattern to nla_grobner::propagate_quotients that
derives a modular-residue constraint from polynomial divisibility,
filling a gap between quotient1-5 (model-value-driven case splits) and
the polynomials Grobner actually produces on Skolem-encoded mod
arithmetic.
Pattern
-------
For a polynomial p with all-integer free variables and a linear monomial
c_v * v (single integer var), the pattern computes M = gcd(|c_i/c_v|)
over the other monomials and K = c0/c_v for the constant term. When both
are integers, dividing p by c_v gives
v + M*Q + K = 0 with Q an integer
so v ≡ -K (mod M). The pattern emits the sound disjunctive lemma
(v < 0) ∨ (v ≥ M) ∨ (v = target)
where target = (-K) mod M ∈ [0, M-1]. This encodes "v ∈ target + M·Z" in
a form the LP / SAT layer can refute against current bounds.
Motivation
----------
QF_UFNIA verification benchmarks over fixed-prime modular arithmetic
(e.g. zk applications using the BabyBear prime 2013265921) regularly
produce basis polynomials of the form
-p*v_div + p*(v_a * v_b) - v_mod = 0
where v_mod is the result of (mod (* v_a v_b) p). The polynomial sits in
the Grobner basis but none of quotient1-5 fires: they all require
specific model-value alignments (r_value == 0, |v_value| > |r_value|,
etc.) that don't hold when all variables in scope are similarly sized
integers in [0, p). The proof spins on interval-tightening lemmas
without ever extracting the modular conclusion.
The author of propagate_quotients flagged this gap with the comment
\"other division lemmas are possible\" preceding the fall-through \"no
lemmas found\" CTRACE. This patch supplies one.
Soundness
---------
The lemma is sound regardless of v's LP bounds — the bound-negation
disjuncts (v < 0) and (v ≥ M) make the disjunction unconditionally true
under the polynomial identity, with v = target as the canonical residue
in [0, M-1]. M is derived from the polynomial's coefficient gcd, not
from any LP-side bound.
Validated under smt.arith.validate=true on the mod-factor-propagation
reproducers (PR #9235 follow-up), zk verifier benchmarks, and a broader
QF_UFNIA sample — 50+ files total, zero validate_conflict() assertion
violations.
Performance
-----------
A model-value gate (skip emission when v's current value already
satisfies one of the disjuncts) prevents the pattern from
short-circuiting the propagate_quotients || propagate_gcd_test ||
propagate_eqs || propagate_factorization || propagate_linear_equations
chain with redundant emissions. Without the gate, a single (v, M,
target) triple can re-emit each Grobner round and starve the downstream
propagators — observed in regression testing as thousands of identical
emissions on a small benchmark, turning a sub-second closure into a
timeout.
On six small mod-factor-propagation reproducers, the patch closes four
cases that previously timed out at 30 s (~1 s typical under the
Grobner-ramped config: smt.arith.nl.gr_q=50,
smt.arith.nl.grobner_eqs_growth=50,
smt.arith.nl.grobner_exp_delay=false, smt.arith.nl.grobner_frequency=1).
The two remaining timeouts in that set are attributable to different
gaps (Boolean-disjunction propagation, and the multi-bounded-mod-result
polynomial shape that needs Grobner over Z/pZ), not to mod_residue
itself.
Diagnostics
-----------
TRACE under the existing 'grobner' tag emits one line per lemma
emission, recording v, M, c_v, c0, and target.
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: Nikolaj Bjorner <nbjorner@microsoft.com>
Preserve the de-linearization of the linear constraints but fixing the
den bug. @ValentinPromies, that is what you had in mind.
---------
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Two TRACE blocks under the existing nla_solver tag:
1. theory_lra::false_case_of_check_nla emits a "varmap:" line for each
NLA lemma, listing j-var → SMT-name mappings for variables in the
lemma's collect_vars set. Lets lemur nla resolve the LP-internal
j-numbering back to the original SMT term names when displaying
lemmas. Without this, lemma-level analysis has to either guess at
variable identities (and j-numbers are reused across nlsat
invocations under backtracking — see j-vars-unstable note) or use a
different trace tag entirely (-tr:nra) for stable algebraic-number
IDs.
2. nla_grobner emits a "grobner-linear-eq:" line at each call to
add_term + update_column_type_and_bound that produces a Linear
Propagation row from completion. Lets us count Gröbner's effective
contribution to the LP tableau independently of the lemma stream.
Useful when investigating Gröbner-deficit hypotheses in NLA cascade
diagnosis.
Both are pure trace emission, behind TRACE(nla_solver, ...). Zero
runtime cost when tracing is off; no semantic change.
* Add adaptive growth knobs for Gröbner under arith.nl.grobner_adaptive
When enabled, the per-call growth budget (m_eqs_growth, m_expr_size_growth,
m_expr_degree_growth, m_max_simplified) is scaled by m_growth_boost:
- two consecutive productive runs bump the boost by 3/2 (cap 4x)
- a miss resets the streak and decays the boost toward 1.0x by 1/4 of excess
Default is off; the existing miss-frequency throttle (m_quota / m_delay_base)
is unchanged, so this only affects per-call power, not call frequency.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Update src/params/smt_params_helper.pyg
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: Arie Gurfinkel <arie.gurfinkel@gmail.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
* Add dual-row shared-factor sandwich for NLA bound propagation
When enabled via arith.nl.monomial_sandwich (default off), monomial_bounds
finds LP term columns whose term has shape a_m * m + a_v * v with exactly
two variables — both factors of a binary monomial m = u*v. The term column's
bound bounds (a_m * m + a_v * v); substituting m = u*v gives v * (a_m*u + a_v),
and sign-aware interval division by v plus an affine shift yields a numeric
bound on u. The derived interval is fed to the existing propagate_value path
so the lemma channel and integer rounding logic are shared with the rest of
NLA's forward/backward propagation; no new emit code.
Catches conflicts of the form
α_v1 * v + α_m * m ≥ k1
α_v2 * v + α_m * m ≤ k2
that today require nlsat (when no single row alone yields infeasibility but
their conjunction tightly bounds u after factoring v).
Scope: binary monomials only (m.size()==2, no squares); cap of 16 term-columns
scanned per call; one lemma per (u,v) attempt to keep the lemma channel quiet.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Add arith.nl.order.binomial_sign flag (default true)
Granular gate for order_lemma_on_binomial_sign — the only order family that
embeds a model-snapshot literal (x ≷ val(x)) in the lemma body. Disabling it
keeps the always-good structural mon-ol family running while removing the
SAT-splitter shape that cascades under model perturbations (e.g., from
arith.nl.monomial_sandwich tightening factor bounds).
Default true preserves master behaviour; the flag is intended as an
experimental knob to measure how much of an observed cascade is specifically
attributable to the binomial-sign splitter vs. the structural cancellation
lemmas in the same module.
See ord-binom-opportunities.md for the full gap analysis and the
deterministic-replacement directions (sandwich, McCormick) that would let
this flag eventually default to false without regressing leaves where
ord-binom currently carries the proof.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Add sign-pinned binomial bound for NLA (Opportunity 1 from ord-binom doc)
When enabled via arith.nl.monomial_binomial_sign (default off), monomial_bounds
adds a third pass alongside propagate_down (existing) and propagate_shared_factor
(sandwich). For a binary monomial m = u*v in m_to_refine whose model value mv
disagrees with val(u)*val(v), and where v has a determined sign:
1. synthesize a one-sided interval for m.var() at mv (no deps; the snapshot
enters as a literal in the lemma body, not as an antecedent)
2. divide by v's interval (sign-aware via dep.div<with_deps>) to get a
deterministic interval for u
3. emit a propagate_value-style lemma whose body is
m.var() < mv (or > mv) ∨ u-bound
conditioned on v's bound witness
Targets the case ord-binom currently handles: factors have determined signs,
m.var() may have no LP bound. The clause is sound modulo the monomial
definition (same condition propagate_down, propagate_shared_factor, and
ord-binom already rely on).
A new throttle kind MONOMIAL_BINOMIAL_SIGN keyed on (m.var, u, v, direction)
prevents cascading: without it, each new val(m.var()) snapshot would re-emit
across model changes the same way ord-binom does.
Validated via smt.arith.validate=true: 0 soundness errors across the
32-leaf test corpus.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Add McCormick box-corner tangent points (Opportunity 2 from ord-binom doc)
When enabled via arith.nl.tangents.box_corners (default off, sub-flag of
arith.nl.tangents), tangent_imp::get_points selects m_a, m_b at the corners
of the bound box [x_lo, x_hi] × [y_lo, y_hi] instead of the model-centered
points val(x) ± delta. The selection follows the classical McCormick
under/over envelope:
- m_below=true (under-approximation):
m_a = (x_lo, y_lo), m_b = (x_hi, y_hi)
- m_below=false (over-approximation):
m_a = (x_lo, y_hi), m_b = (x_hi, y_lo)
The existing generate_plane already produces the McCormick linear form
xy ≷ pl.y·x + pl.x·y − pl.x·pl.y at any chosen point pl. push_point is
skipped in box-corner mode: corners are extremes, so doubling the offset
moves out of the box and would invalidate the McCormick property.
Falls back to the existing model-driven point selection when either factor
has an unbounded side or the box is degenerate (single-point in a
dimension).
Soundness — non-strict inequality at corners. The classical model-driven
flow uses pl strictly in the interior of the box, so generate_plane emits
xy > T (strict). At the box corners the tangent meets the surface along
the box's edges (xy = T when x = pl.x or y = pl.y), so the strict
inequality is violated by any model with x at the box boundary. A new
m_pl_strict_interior member, set false on a successful set_box_corners(),
switches generate_plane's emission to ≥/≤ (non-strict). The model-driven
path keeps strict — its push_point + plane_is_correct_cut chain already
guarantees pl is interior.
Validated via smt.arith.validate=true: 0 validate_conflict() failures
across the 32-leaf test corpus with box_corners=true.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
updates to nlsat polynomial simplification introduced checkpoints.
These can throw exceptions (if setting a timeout).
The code that uses this was not properly protected from exceptions to distinguish timeout based tactics from genuine exceptions that should terminate solving altogether.
see updates such as: 117da362f0
Fixes a double-free (SIGSEGV in mpz_manager::del) in
algebraic_numbers::manager:👿:del_poly, reached through the
destruction of nlsat::evaluator's scoped_anum_vector members on a
subsequent call to nra::solver:👿:reset.
Root cause: sort_roots runs std::sort over a numeral_vector with a
comparator (lt_proc -> manager::lt -> compare_core) that legitimately
throws when the reslimit fires mid-comparison. libc++'s insertion sort
shifts elements via move-assignment inside its inner loop, and because
anum previously had only compiler-generated shallow copy/move (both
just copied m_cell without nulling the source), a throw between two
consecutive shifts could leave two vector slots pointing at the same
algebraic_cell. When the owning scoped_anum_vector was later destroyed
it del'd the same cell twice, reading through a freed chunk whose
first bytes had been overwritten by small_object_allocator's free-list
next pointer.
Fix: give anum proper move constructor and move assignment that
transfer the tagged m_cell pointer and null the source. Copy stays
a shallow handle copy (ownership is still tracked externally by the
manager / owning vector, as before). With the new move, every
intermediate state of sort's move-via-tmp sequence has at most one
slot referencing any given cell, so a throwing comparator can leak
the in-flight tmp cell but cannot produce aliased slots and therefore
cannot cause the downstream double-free.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Fix broken term_comparer in m_normalized_terms_to_columns lookup
The `m_normalized_terms_to_columns` map in `lar_solver` uses a
`term_comparer` that delegates to `lar_term::operator==`, which
intentionally returns `false` (with comment "take care not to create
identical terms"). This makes `fetch_normalized_term_column` unable to
find any term, rendering the Horner module's `interval_from_term`
bounds-recovery path dead code.
History: `lar_term::operator==` returning `false` has been present since
the original "merge LRA" commit (911b24784, 2018). The
`m_normalized_terms_to_columns` lookup was added later (dfe0e856,
c95f66e0, Aug 2019) as "toward fetching existing terms intervals from
lar_solver". The initial code had `lp_assert(find == end)` on
registration (always true with broken ==) and `lp_assert(find != end)`
on deregister (always false). The very next commit (207c1c50, one day
later) removed both asserts, replacing them with soft checks. The
`term_comparer` struct delegating to `operator==` was introduced during
a later PIMPL refactor (b375faa77).
Fix: Replace the `term_comparer` implementation with a structural
comparison that checks size and then verifies each coefficient-variable
pair via `coeffs().find_core()`. This is localized to the
`m_normalized_terms_to_columns` map and does not change
`lar_term::operator==`, preserving its intentional semantics elsewhere.
Validated: on a QF_UFNIA benchmark, `interval_from_term` lookups go
from 0/573 successful to 34/573 successful. Unit test added for the
`fetch_normalized_term_column` round-trip.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
* Disable operator== for lar_term
The operator== for lar_term was never intended to be used.
This changes physically disables it to identify what happens to depend
on the operator.
* Work around missing lar_term==
Previous commit disabled lar_term==. This is the only use of the
operator that seems meaningful. Changed it to compare by references
instead.
Compiles, but not sure this is the best solution.
* replace with e
Signed-off-by: Nikolaj Bjorner <nbjorner@microsoft.com>
* Delete unused ineq::operator==
The operator is unused, so there is no need to figure what is
the best fix for it.
* Remove lp tests that use ineq::operator==
---------
Signed-off-by: Nikolaj Bjorner <nbjorner@microsoft.com>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-authored-by: Nikolaj Bjorner <nbjorner@microsoft.com>
When a monic x*y has a factor x with mod(x, p) = 0 (fixed), propagate
mod(x*y, p) = 0. This enables Z3 to prove divisibility properties like
x mod p = 0 => (x*y) mod p = 0, which previously timed out even for
p = 2. The lemma fires in the NLA divisions check and allows Gröbner
basis and LIA to subsequently derive distributivity of div over addition.
Extends division tuples from (q, x, y) to (q, x, y, r) to track the
mod lpvar. Also registers bounded divisions from the mod internalization
path in theory_lra, not just the idiv path.
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Calling factor_sqf_pp recursively on Hensel-lifted factors corrupts
shared mutable state in the polynomial manager, m_m2pos, m_som_buffer,
m_cheap_som_buffer, m_tmp1, etc., causing assertion violations:
- polynomial.cpp:473 id < m_m2pos.size()
- upolynomial.cpp:2624 sign_a == -sign_b
Use factor_1_sqf_pp/factor_2_sqf_pp for small degrees, push directly
for larger degrees. These don't conflict with the outer call's buffers.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Fix memory leaks: use scoped_numeral instead of raw numeral for
evaluation points, ensuring cleanup on exceptions
- Precompute lc_inv before the Hensel lifting loop instead of
recomputing each iteration
- Use scoped_numeral_vector for eval_vals for consistency with codebase
- Move eval_values and candidate_primes to static constexpr class-level
- Document limitations: single-prime Hensel lifting, contiguous factor
splits only, pseudo-division lc-power caveat
- Condense Bezout derivation comment to 4-line summary
- Fix README to say Hensel lifting instead of GCD recovery
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace the stub factor_n_sqf_pp (TODO: invoke Dejan's procedure) with a
working implementation using bivariate Hensel lifting:
- Evaluate away extra variables to reduce to bivariate
- Factor the univariate specialization
- Lift univariate factors to bivariate via linear Hensel lifting in Zp[x]
- Verify lifted factors multiply to original over Z[x,y]
- For >2 variables, check bivariate factors divide the original polynomial
Tests: (x0+x1)(x0+2x1)(x0+3x1) now correctly factors into 3 linear factors.
All 89 unit tests pass in both release and debug builds.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
resultant vanishes during a nested isolate_roots call. The mathematical
invariant that the resultant cannot vanish again after recovery does not
hold in all cases, e.g. with certain nonlinear real arithmetic formulas.
The algebraic_exception propagates cleanly through the nlsat solver and
tactic layers which already catch z3_exception.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* outline of signature for assignment based conflict generation
Signed-off-by: Nikolaj Bjorner <nbjorner@microsoft.com>
* outline of interface contract
Signed-off-by: Nikolaj Bjorner <nbjorner@microsoft.com>
* remove confusing construction
Signed-off-by: Nikolaj Bjorner <nbjorner@microsoft.com>
* add material in nra-solver to interface
Signed-off-by: Nikolaj Bjorner <nbjorner@microsoft.com>
* add marshaling from nlsat lemmas into core solver
Signed-off-by: Nikolaj Bjorner <nbjorner@microsoft.com>
* tidy
Signed-off-by: Nikolaj Bjorner <nbjorner@microsoft.com>
* add call to check-assignment
Signed-off-by: Nikolaj Bjorner <nbjorner@microsoft.com>
* Nl2lin (#7795)
* add linearized projection in nlsat
* implement nlsat check for given assignment
* add some comments
* fixup loop
Signed-off-by: Nikolaj Bjorner <nbjorner@microsoft.com>
* updates
Signed-off-by: Nikolaj Bjorner <nbjorner@microsoft.com>
* fixes
Signed-off-by: Nikolaj Bjorner <nbjorner@microsoft.com>
* debug nl2lin
Signed-off-by: Lev Nachmanson <levnach@hotmail.com>
* Nl2lin (#7827)
* fix linear projection
* fix linear projection
* use an explicit cell description in check_assignment
* clean up (#7844)
* Simplify no effect checks in nla_core.cpp
Move up linear nlsat call to replace bounded nlsat.
* t
Signed-off-by: Lev Nachmanson <levnach@hotmail.com>
* t
Signed-off-by: Lev Nachmanson <levnach@hotmail.com>
* detangle mess
Signed-off-by: Nikolaj Bjorner <nbjorner@microsoft.com>
* remove the too early return
Signed-off-by: Lev Nachmanson <levnach@hotmail.com>
* do not set use_nra_model to true
Signed-off-by: Lev Nachmanson <levnach@hotmail.com>
* remove a comment
Signed-off-by: Lev Nachmanson <levnach@hotmail.com>
* add a hook to add new multiplication definitions in nla_core
* add internalization routine that uses macro-expanded polynomial representation
Signed-off-by: Nikolaj Bjorner <nbjorner@microsoft.com>
* add internalization routine that uses macro-expanded polynomial representation
Signed-off-by: Nikolaj Bjorner <nbjorner@microsoft.com>
* fixup backtranslation to not use roots
Signed-off-by: Nikolaj Bjorner <nbjorner@microsoft.com>
* call setup_assignment_solver instead of setup_solver
Signed-off-by: Nikolaj Bjorner <nbjorner@microsoft.com>
* debug the setup, still not working
Signed-off-by: Lev Nachmanson <levnach@hotmail.com>
* updated clang format
Signed-off-by: Nikolaj Bjorner <nbjorner@microsoft.com>
* simplify
Signed-off-by: Nikolaj Bjorner <nbjorner@microsoft.com>
* create polynomials with integer coefficients, use the hook to create new monomials
Signed-off-by: Lev Nachmanson <levnach@hotmail.com>
* integrating changes from master related to work with polynomials
Signed-off-by: Lev Nachmanson <levnach@hotmail.com>
* add forgotten files
Signed-off-by: Lev Nachmanson <levnach@hotmail.com>
* Update nlsat_explain.cpp
Remove a duplicate call
* fix
* move linear cell construction to levelwise
* fix
* fix
* Port throttle and soundness fixes from master
- Fix soundness: pop incomplete lemma from m_lemmas on add_lemma failure
- Gracefully handle root atoms in add_lemma
- Throttle check_assignment with failure counter (decrement on success)
- Add arith.nl.nra_check_assignment parameter
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Add arith.nl.nra_check_assignment_max_fail parameter
Replace hardcoded failure threshold with configurable parameter (default 10).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Add cha_abort_on_fail parameter to control failure counter decrement
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* abort nla check_assignment after a set number of allowed failures
Signed-off-by: Lev Nachmanson <levnach@hotmail.com>
* Add missing AST query methods to Java API (#8977)
* add Expr.isGround() to Java API
Expose Z3_is_ground as a public method on Expr. Returns true when the
expression contains no free variables.
* add Expr.isLambda() to Java API
Expose Z3_is_lambda as a public method on Expr. Returns true when the
expression is a lambda quantifier.
* add AST.getDepth() to Java API
Expose Z3_get_depth as a public method on AST. Returns the maximum
number of nodes on any path from root to leaf.
* add ArraySort.getArity() to Java API
Expose Z3_get_array_arity as a public method on ArraySort. Returns
the number of dimensions of a multi-dimensional array sort.
* add DatatypeSort.isRecursive() to Java API
Expose Z3_is_recursive_datatype_sort as a public method on
DatatypeSort. Returns true when the datatype refers to itself.
* add FPExpr.isNumeral() to Java API
Expose Z3_fpa_is_numeral as a public method on FPExpr. Returns true
when the expression is a concrete floating-point value.
* add isGroundExample test to JavaExample
Test Expr.isGround() on constants, variables, and compound
expressions.
* add astDepthExample test to JavaExample
Test AST.getDepth() on leaf nodes and nested expressions to verify
the depth computation.
* add arrayArityExample test to JavaExample
Test ArraySort.getArity() on single-domain and multi-domain array
sorts.
* add recursiveDatatypeExample test to JavaExample
Test DatatypeSort.isRecursive() on a recursive list datatype and a
non-recursive pair datatype.
* add fpNumeralExample test to JavaExample
Test FPExpr.isNumeral() on a floating point constant and a symbolic
variable.
* add isLambdaExample test to JavaExample
Test Expr.isLambda() on a lambda expression and a plain variable.
* change the default number of failures in check_assignment to 7
Signed-off-by: Lev Nachmanson <levnach@hotmail.com>
* Fix high and medium priority API coherence issues (Go, Java, C++, TypeScript) (#8983)
* Initial plan
* Add missing API functions to Go, Java, C++, and TypeScript bindings
Co-authored-by: NikolajBjorner <3085284+NikolajBjorner@users.noreply.github.com>
---------
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: NikolajBjorner <3085284+NikolajBjorner@users.noreply.github.com>
* qf-s-benchmark: debug build + seq tracing + seq-fast/nseq-slow trace analysis (#8988)
* Initial plan
* Update qf-s-benchmark: debug build, seq tracing, trace analysis
Co-authored-by: NikolajBjorner <3085284+NikolajBjorner@users.noreply.github.com>
---------
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: NikolajBjorner <3085284+NikolajBjorner@users.noreply.github.com>
* disable linear approximation by default to check the merge
Signed-off-by: Lev Nachmanson <levnach@hotmail.com>
* set check_assignment to true
Signed-off-by: Lev Nachmanson <levnach@hotmail.com>
* fix restore_x by recalulating new column values
Signed-off-by: Lev Nachmanson <levnach@hotmail.com>
* fix restore_x by recalulating new column values
Signed-off-by: Lev Nachmanson <levnach@hotmail.com>
* fix a memory leak
Signed-off-by: Lev Nachmanson <levnach@hotmail.com>
---------
Signed-off-by: Nikolaj Bjorner <nbjorner@microsoft.com>
Signed-off-by: Lev Nachmanson <levnach@hotmail.com>
Co-authored-by: Nikolaj Bjorner <nbjorner@microsoft.com>
Co-authored-by: ValentinPromies <44966217+ValentinPromies@users.noreply.github.com>
Co-authored-by: Valentin Promies <valentin.promies@rwth-aachen.de>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Angelica Moreira <48168649+angelica-moreira@users.noreply.github.com>
Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
Co-authored-by: NikolajBjorner <3085284+NikolajBjorner@users.noreply.github.com>
- Relax restore_x() to handle backup/current size mismatches: when
backup is shorter (new columns added), call
move_non_basic_columns_to_bounds() to find a feasible solution.
- Fix 100x performance regression in nonlinear optimization: save LP
optimum before check_nla and return it as bound regardless of NLA
result, so opt_solver::check_bound() can validate via full re-solve
with accumulated NLA lemmas.
- Refactor theory_lra::maximize() into three helpers: max_with_lp(),
max_with_nl(), and max_result().
- Add mk_gt(theory_var, impq const&) overload for building blockers
from saved LP optimum values.
- Add BNH multi-objective optimization test (7/7 sat in <1s vs 1/7
in 30s before fix).
- Add restore_x test for backup size mismatch handling.
Fixes#8890
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The NRA solver's check() uses cone-of-influence (COI) to select a subset
of constraints for nlsat. When nlsat returns l_true, the model is validated
against all constraints, but non-COI constraints can legitimately be
violated since nlsat only solved over the COI subset.
- Non-COI violations gracefully return l_undef (fallback to other strategies)
- COI violations still trigger an assertion (indicating a real nlsat bug)
Fixes#8883
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Three bugs in the def ref-counting infrastructure:
1. dec_ref() incremented (++) instead of decrementing (--) the ref count,
so objects were never freed.
2. def_ref lacked copy and move constructors, so the compiler-generated
default copy just copied the raw pointer without inc_ref. This caused
use-after-free when def_ref values were copied into vectors.
3. Compound def types (add_def, mul_def, div_def) lacked destructors to
dec_ref their children. Added virtual destructor to base def class
and child-releasing destructors to compound types.
Fixes the memory leak from #7027 (model_based_opt.cpp:81).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add checkpoint() call in gcd_prs() main loop so polynomial GCD
computation respects rlimit/timeout. Add cancellation callback to
HNF calculation so it can be interrupted when the solver is cancelled.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>