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. (2 more flags after this!)
The first of these was -Wctad-maybe-unsupported. That has to do with
"class template argument deduction" -- the flag requires template
deduction guides to be explicitly provided if templated types are used
in situations that requires argument deduction. This fired for various
uses of templated types in the utils directory.
I decided that this should be a case of if it ain't broke, don't fix it,
and explicitly disabled the warning in the Z3 build (which will override
the setting if the flag is enabled in a larger build including Z3, like
clang).
-----
The second flag has to do with the C++ "rule of 3". Here is Google's AI
summary (inf_s_integer is a class in Z3 that triggered the warning):
_This warning means your inf_s_integer class defines a custom copy
assignment operator but lacks a user-defined copy constructor, which the
C++ standard deprecates to encourage the "Rule of Three". To fix this,
explicitly declare and = default the copy constructor in your class
definition._
_...example of how to fix..._
_This updates your code to modern C++ standards, cleanly silencing the
warning._
This seemed like a good standard to follow, and didn't require too many
changes, so I propose them.
## Summary
Adds a global configuration parameter **`suppress_platform_verbose`**
(bool, default `false`).
When set to `true`, memory and time information is suppressed from
verbose output — the same behavior as the `-V` flag proposed in #10301,
but driven by a global parameter instead of a dedicated command-line
switch. This yields deterministic, platform-independent verbose output
that is useful for testing and diffing.
Usage:
```
z3 -v:2 suppress_platform_verbose=true problem.smt2
```
## Approach
Instead of introducing a `-V` command-line flag (#10301), this registers
`suppress_platform_verbose` as a global parameter via `env_params`,
wired to a lightweight `get_suppress_platform_verbose()` flag in `util`.
Verbose output sites guard only their **memory** (`mem_stat`) and
**time** (`m_watch` / `:time` / `:before-memory` / `:after-memory`)
fields on this flag. Platform-independent counters (e.g. `:cost`,
`:threshold`, `:elim-vars`) are always emitted.
Affected sites:
- SAT: asymm-branch, probing, scc, simplifier (subsumer /
blocked-clauses / resolution), solver stats (`sat.stats`, `:memory`,
`mk_stat`)
- SMT: context stats (`smt.stats`)
- Tactic report (`:time`, `:before-memory`, `:after-memory`)
## Validation
- Release build succeeds.
- `suppress_platform_verbose` appears in `z3 -pd` with the correct
type/default, and is accepted on the command line.
- All 93 `test-z3 /a` unit tests pass.
Closes the need for the `-V` switch in #10301.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 57b9b87e-950a-49ea-bbb3-ed585646a5a9
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 PR enable the "-Wimplicit-fallthrough" warning, then fix all the
warnings this gets
in the clang build, by:
* Augmenting UNREACHABLE to add __builtin_unreachable(), which
suppresses
warnings for fallthrough in that cse.
* Adding Z3_fallthrough in many cases, to make it clear that
fallthroughs are intentional.
* Adding [[noreturn]] to functions that throw, so the compiler knows
they don't fall through to the next case.
* In a couple of cases, there's a fall-through to a default case, which
does "break", or "return nullptr". In those cases, I duplicated the
action in the preceding case, to make it more self-contained, and robust
in the face of change.
In some cases, I am concerned about whether the warnings are identifying
real bugs. For example, the fall-throughs in these files seem at least a
little suspect:
nnf.cpp
seq_rewriter.cpp
lar_solver.cpp
while very probably correct, also seem at least a tiny bit suspect.
However, this PR does *not* attempt to change any behavior, only to
silence the warnings. It would be great if somebody with more knowledge
of the code could vet these cases. If vetted, the explicit presence of
the Z3_fallthrough would reassure future readers of the code that the
fall-through is intentional, not accidental.
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 PR completes the job started by
https://github.com/Z3Prover/z3/pull/10169. It adds `-Wextra-semi` to the
set of CLANG_ONLY_WARNINGS, and adds
```
START_DISABLE_EXTRA_SEMI_WARNING;
...macro invocations with trailing semis...
END_DISABLE_WARNING;
```
around all the blocks of macro invocations that provoked warnings.
(Additionally, in realclosure.h, there was one block of macro
invocations that did *not* follow the trailing-semi pattern; changed
that to look like all the others).
## Summary
Fixes a completeness regression where `elim_uncnstr` was silently
disabled for ordinary (non-polymorphic) goals, detected by the
`snapshot-regression` corpus.
- **Originating discussion:**
https://github.com/Z3Prover/bench/discussions/3054
- **Benchmark:** `iss-6260/small-2.smt2` (corpus `Z3Prover/bench`,
`inputs/issues/iss-6260/`)
- **Divergence:** recorded oracle `sat` → current z3 produces `unknown`
### Divergence diff
```diff
--- small-2.expected.out (expected)
+++ produced (current z3)
@@ -1,3 +1,3 @@
-sat
+unknown
(error "line 17 column 0: unexpected character")
(error "line 17 column 1: unexpected character")
```
(The `(error ...)` lines are expected: the benchmark contains a stray
```` ``` ```` fence on line 17. Only the `sat` → `unknown` change is the
regression.)
## Root cause
`git bisect` over the regression window pins the flip to commit
`208cc5686` ("fix build"), which added `|| m().has_type_vars()` to the
`elim_uncnstr_tactic` guard and an equivalent `if (m.has_type_vars())
return;` to the `elim_unconstrained` simplifier.
`ast_manager::has_type_vars()` is a **manager-wide, sticky** flag: it is
set to `true` as soon as *any* type variable is created, and is never
reset. In particular `finite_set_decl_plugin::init()` creates type
variables `A`/`B` to define its polymorphic signatures. Those type
variables never occur in the user's assertions, but once the finite_set
plugin is initialized — which happens while processing this benchmark —
the flag is globally `true` (confirmed by instrumenting `mk_type_var`:
the only type vars created for this benchmark are the finite_set
signature vars `A` and `B`).
As a result `elim_uncnstr` bails out for goals that contain **no**
polymorphic terms at all, i.e. it is effectively disabled. Unconstrained
subterms that used to be eliminated now reach the theory solvers.
For this benchmark the (single) assertion is `(not (xor (>= x 0) (>= x1
0) (>= x 0) x4 (str.contains ...)))`. The duplicated `(>= x 0)` cancels
(`a xor a = false`), and the free Boolean `x4` can fix the parity
regardless of the value of the `str.contains` term, so the goal is
trivially `sat`. That `str.contains`/`str.replace_re` subterm is
unconstrained and was previously removed by `elim_uncnstr`; without that
elimination it reaches `theory_seq`, which marks `str.replace_re` as
unhandled and gives up in `final_check` → `unknown` (`incomplete (theory
seq)`).
## Fix
Make the guard precise. Keep `has_type_vars()` as a cheap pre-filter
(matching existing usage in `ast_translation.cpp` and
`ast_manager::has_type_var`), but only bail out when the goal / asserted
formulas **actually** contain type-variable typed terms, using the
existing `polymorphism::util::has_type_vars(expr*)`.
This preserves the polymorphism crash-protection (goals with genuine
type-variable terms still skip `elim_uncnstr`) while restoring
`elim_uncnstr` for the vast majority of goals that merely triggered a
polymorphic-plugin initialization. Both twin guards (the `elim_uncnstr`
tactic and the `elim_unconstrained` simplifier) are fixed consistently.
## Validation
Built this checkout and re-ran the benchmark (step 5 of the fixer
workflow):
- `./configure && make -C build -j$(nproc)` — Z3 version 4.17.0.
- Unpatched master reproduced the divergence: `z3 -T:20
iss-6260/small-2.smt2` → `unknown`.
- After the fix, `z3 -T:20 inputs/issues/iss-6260/small-2.smt2` produces
**exactly** the recorded oracle:
```
sat
(error "line 17 column 0: unexpected character")
(error "line 17 column 1: unexpected character")
(error "line 17 column 2: unexpected character")
```
- Regression sanity checks: sibling `iss-6260/small.smt2` unchanged
(`sat`); plain arithmetic unconstrained goals unchanged; polymorphic
goals with genuine type-variable terms (including `declare-type-var` and
forcing `(check-sat-using (then elim-uncnstr smt))`) still solve without
crashing — the guard still fires for those.
Opened as a **draft** for human review.
> Generated by [Fix a Z3 snapshot-regression
divergence](https://github.com/Z3Prover/bench/actions/runs/28844840657)
· 861.2 AIC · ⌖ 45.8 AIC · ⊞ 8.9K ·
[◷](https://github.com/search?q=repo%3AZ3Prover%2Fz3+%22gh-aw-workflow-id%3A+snapshot-regression-fixer%22&type=pullrequests)
<!-- gh-aw-agentic-workflow: Fix a Z3 snapshot-regression divergence,
engine: copilot, version: 1.0.63, model: claude-opus-4.8, id:
28844840657, workflow_id: snapshot-regression-fixer, run:
https://github.com/Z3Prover/bench/actions/runs/28844840657 -->
<!-- gh-aw-workflow-id: snapshot-regression-fixer -->
<!-- gh-aw-workflow-call-id: Z3Prover/bench/snapshot-regression-fixer
-->
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.
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.)
Add new parallel algorithm as a tactic (parallel_tactical2.cpp)
Don't port over old experiments from smt_parallel that we aren't using
(sls, inprocessing, failed_literal_mode for bb detection)
Fix bugs: lease cancellation/reslimit race condition, involves changing
lease epoch to simple boolean flag
Also, now there is a single shared set of params for the tactic and
smt_parallel
**Test runs for the parallel_tactical2 vs old smt_parallel version:**
run-2747-Z3-threads-4-qflia-30s-stats.md
run-2746-Z3-threads-4-qflia-30s-parallel_tactic-stats.md
run-2745-Z3-threads-1-qfbv-30s-stats.md
run-3013-Z3-threads-4-qfbv-30s-parallel_tactic-stats.md --> note this is
indeed run-3013, I reran after a bugfix in inc_sat_solver
run-2743-Z3-threads-4-qfnia-30s-stats.md
run-2742-Z3-threads-4-qfnia-30s-parallel_tactic-stats.md
**Test runs for the new smt_parallel with bugfixes:**
run-2801-Z3-threads-4-qflia-30s-smtparallel-bugfixes-stats.md,
run-2800-Z3-threads-4-qflia-30s-smtparallel-bugfixes-stats.md
run-2797-Z3-threads-4-qfnia-30s-smtparallel-bugfixes-stats.md
compare to old smt_parallel:
run-2747-Z3-threads-4-qflia-30s-stats.md
run-2743-Z3-threads-4-qfnia-30s-stats.md
Note that there is a slight regression on lia in run-2800. The source of
this appears to be the new new LP largest-cube LIA heuristic param,
which is enabled by default. disabling this param in run-2801 restored
performance (I didn't change this in this PR though, just something to
note)
http://mtzguido.tplinkdns.com:8081/z3/compare_stats.html
---------
Signed-off-by: Nikolaj Bjorner <nbjorner@microsoft.com>
Co-authored-by: Ilana Shapiro <ilanashapiro@Ilanas-MacBook-Pro.local>
Co-authored-by: Ilana Shapiro <ilanashapiro@Ilanas-MBP.localdomain>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
mk_preamble_tactic in qflia_tactic.cpp constructed lia2card with a
throttled params_ref but did not wrap the call in using_params, so
when the preamble is invoked standalone the throttle is silently
clobbered by and_then's ambient param propagation: each child gets
updt_params(outer_p) called on it, re-reading lia2card.max_range
from the outer params (default 101) and discarding the constructor
override.
mk_qflia_tactic masks the bug because it wraps the whole chain in
using_params(..., main_p) where main_p also carries
lia2card.max_range=1. But QF_UFNIA goals reach mk_preamble_tactic
through the fall-through tail in mk_default_tactic
(default_tactic.cpp:52) without that outer wrap: is_qfnia_probe
rejects goals containing UF, so QF_UFNIA does not route through
mk_qfnia_tactic and instead lands on the unguarded preamble. Any
integer variable with concrete range hi-lo <= 101 then gets
hot-encoded into ~hi-lo indicator Booleans plus a sum-of-ITEs,
inflating SAT search and bloating each NLA refutation that touches
the partition.
Fix: wrap the call with using_params(mk_lia2card_tactic(m),
lia2card_p) so the throttle survives ambient propagation.
Verified on a Certora QF_UFNIA VC with a 0..98 integer: metrics now
match running with explicit tactic.lia2card.max_range=0 (mk-bool-var,
decisions, nlsat-restarts all within run-to-run noise of the
workaround), confirming the built-in throttle is finally effective.
This mirrors the pattern from commit 87e45accd ("Throttle lia2card
in QF_NIA preamble", #9362) which fixed the same bug in
mk_qfnia_preamble but did not propagate the fix to the QF_LIA
preamble. The original throttle parameters (max_range=1,
max_ite_nesting=1) were introduced by Nikolaj's commit 99cbfa715
("Add a sharp throttle to lia2card tactic to control overhead in
default tactic mode") in Feb 2025; that commit set the params at
construction time, which works under mk_qflia_tactic's outer
using_params wrap but not under standalone invocation.
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
mk_qfnia_preamble invoked lia2card with no params, so the default
max_range=101 was in effect. Any integer variable with a concrete
range hi-lo <= 101 was expanded into that many fresh Booleans plus
a sum-of-ITEs, bloating SAT search alongside the nonlinear structure.
On an observed QF_UFNIA benchmark this drove a 0.2s problem to a 30s
timeout.
Mirror the throttle already applied in mk_preamble_tactic
(qflia_tactic.cpp, commit 99cbfa715): limit lia2card to 0-1 integer
variables and nesting depth 1. Wrap with using_params so the
override survives and_then's downstream updt_params calls (passing
the params to mk_lia2card_tactic alone is overwritten when and_then
re-propagates the ambient params to each child).
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
