Translate ternaries, and stop crashing on bitwise and real modulo - #99
Draft
HowardvanRooijen wants to merge 1 commit into
Draft
Translate ternaries, and stop crashing on bitwise and real modulo#99HowardvanRooijen wants to merge 1 commit into
HowardvanRooijen wants to merge 1 commit into
Conversation
The expression-tree layer had three rough edges. A ternary (Conditional node) had no case and threw "Unsupported expression node type"; it now maps onto Z3's if-then-else. Integer bitwise & | ^ and real % were translated by an unconditional cast that assumed the operand sort from the node - so bool & bool worked but int & int, and real %, crashed with an InvalidCastException from inside Z3, naming neither the operator nor the reason. The operators now choose by the operands' sort, the way the Convert case was taught to in #63/#76. Boolean operands still give the logical operators; an integer bitwise op and a real modulo are refused with a message that says a bit-vector representation would be needed, and that Z3 has no real remainder. Genuine bitwise support needs bit-vector symbols, which is a separate feature. Also cached the three reflected MethodInfos the Distinct translation looked up on every visit (Z3Methods.Distinct and two Enumerable methods), and removed the commented-out VisitParameter and a dead Debugger.Break branch. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This was referenced Sep 1, 2026
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.
Improves the expression-tree translation layer (
ExpressionVisitor) — the part of the librarythat turns a constraint lambda into Z3 terms. Behavioural, perf, and cleanup; no public-API
change (the instance-visitor refactor and
internalnarrowing are a separate PR).What was wrong, measured
Three rough edges, all confirmed by probe before the change:
(t.X1 > 0 ? t.X2 : 0) == 5— a ternaryNotSupportedException: Conditional— no case at all(t.X1 & 6) == 4— integer bitwise&(also|,^)InvalidCastException: IntExpr → BoolExpr, from inside Z3t.X1 % 2.5 == 0.5— real moduloInvalidCastException: RealExpr → IntExprThe bitwise and modulo crashes share one cause: the operator dispatch keyed on the expression
node and cast the operands to an assumed sort. But C# spells two different operations the same
way —
&/|/^are Boolean logic onbooland bitwise arithmetic on integers, and%isinteger remainder with no real counterpart. The node can't tell them apart, so the wrong operand
got a cast that succeeded for one meaning and threw an opaque Z3 cast error for the other. This is
the same lesson the
Convertcase learned in #63/#76 — decide from the operand's translatedsort, not the node — that the rest of the dispatch never got.
The change
Ternary → if-then-else.
ExpressionType.Conditionalnow translates toMkITE, sot.X1 > 0 ? a : bworks, including nested inside arithmetic and over real-sorted branches.Sort-aware
&|^. The operator is chosen from the operands: Boolean operands give thelogical operator exactly as before; anything else is a bitwise operation, which Z3's integer sort
has no counterpart for, so it is refused with
Sort-aware
%. Integer operands give the remainder as before; a real modulo is refused with amessage saying Z3 has no remainder on reals, rather than crashing in the cast.
Genuine integer bitwise and shift support needs bit-vector symbols — a separate feature. This PR
turns two opaque crashes into clear, honest diagnostics; it does not pretend to add the feature.
Perf. The
Distincttranslation looked up threeMethodInfos by reflection on every visit—
typeof(Z3Methods).GetMethod("Distinct")and, worse,typeof(Enumerable).GetMethods().First(…)twice, materialising all of
Enumerable. They are now cached instatic readonlyfields.Cleanup. Removed the commented-out
VisitParameterand a deadelse { //Debugger.Break(); };fixed
MkString(val.ToString())to(string)valwhere the value is already a string; correctedan "unsuported" message.
Tests
280 → 290. A new
ExpressionTranslationTestsfor the operators this touches, and two pins inUnsupportedExpressionTestsupdated because their subject changed.Solve_TernaryWhoseTestHolds_TakesTheTrueBranch/..TestFails_TakesTheFalseBranchSolve_TernaryOverRealBranches_TranslatesSolve_TernaryNestedInArithmetic_TranslatesSolve_BitwiseOperatorOnIntegerSymbols_ThrowsNotSupportedNamingIt(&,|,^)bit-vectorSolve_ModuloOnRealSymbols_ThrowsNotSupportedNamingItSolve_BooleanBitwiseOperators_StillTranslate&/|/^unchangedSolve_ModuloOnIntegerSymbols_StillTranslatesSolve_ConditionalExpression_ThrowsNotSupportedException(removed)Solve_SupportedExpressionsAlongsideRejectedOnes_StillRejects(updated)The
DistinctMethodInfo caching is behaviour-preserving and stays covered by the existingDistinctSelectorTheoremTests/DistinctInlineArrayTheoremTests.Mutation results
Conditionalcase&/|/^cast toBoolExprblindly (the old code)InvalidCastException%casts toIntExprblindly (the old code)Verification
dotnet build solutions/Z3.Linq.slnx -c Release— clean,TreatWarningsAsErrorsanddocumentation generation on
./build.ps1 -Configuration Release— 46 tasks, 0 errors, 0 warningsnew refusal branches that fire only in the negative tests plus the ternary
MkITEpaths — theoperators' own branches are covered in both directions
Follow-ups (not in this PR)
internalnarrowing —ExpressionVisitorthreads(Context, Environment, ParameterExpression)through every method and ispubliconly "forhistorical reasons." Turning it into an
internalinstance class with those as fields is themechanical modernisation the .NET 10 / major-version work unblocks; kept separate so a large
no-behaviour diff doesn't bury these fixes. Next PR on the stack.
be diagnosed. A feature in its own right.
Release note
Releases remain on hold under #60 until Microsoft.Z3 5.x reaches nuget.org, so this reaches
mainbut not consumers. Nothing about the hold changes.