Skip to content

Commit c5ac281

Browse files
Reuse solver results across verification runs (#430)
In VS Code, the language server verifies the whole source tree again on every save, in the same JVM, and asks Z3 mostly the same queries each time. On the study workspace (`proposal-C-stars`, 15 files) a save makes about 1,100 queries. About 96% of them depend only on the library specs: the typestate state machines built for state hints, and the satisfiability check of each `@StateRefinement`. This PR keeps solver results across verification runs, in memory, so a save after the first one only asks Z3 the queries that changed. ## How a query is identified The cache sits in `SMTEvaluator`, around the only place the verifier calls Z3. The key (`smt/QueryCache.java`) is everything `TranslatorToZ3` reads to build the query: - the expression, **node by node** (an `ExpressionVisitor`, so a new AST node does not compile until the key handles it), with `static final` constants including their resolved values; - for each variable: its type (the enum constants for an enum) and its supertypes; - every enum, ghost and ghost state in the context. Variables are renamed in order of first appearance. A query whose fresh variables were numbered differently because of an edit earlier in the file is still found. Names that denote enum constants (`Mode.Photo`) are values, not variables, so they are never renamed. The translator and the cache share the helper that names them. Results only live in memory, for as long as the process runs, so the verifier and the solver cannot change under them. **What is never reused:** - **Unknown results** are never kept. - **Translation errors** (e.g. a name that is not found) are never kept. - **Counterexamples** are only reused when the variables have exactly the same names. Otherwise a caller that needs one asks Z3 again. Callers that only use the yes/no answer (`checkStateSMT`, which builds the state machines and checks state refinements, and the constraint simplifier) say so and get the cached answer. ## Keeping it correct - **Switch:** `-Dliquidjava.cache=on|off|check` (default `on`). `check` recomputes every result it finds in the cache and throws an `AssertionError` on any difference. An `AssertionError` is not caught by the code that recovers from exceptions, so a mismatch cannot be hidden. - **The whole test suite runs in `check` mode** (surefire `systemPropertyVariables`). Every cache hit during `mvn test` is verified against a fresh solver call. - **`TestQueryCache`** verifies a program, edits one thing the queries depend on without changing their text, and verifies it again in the same JVM, as the language server does after a save. The cached result must equal an uncached run, and the edit must change the result. The edits: - a typestate transition; - a state refinement that can never hold; - a parameter's type (`int` → `long`: `v > 0` implies `v >= 1` only for integers); - the constants of an enum; - the type of a ghost (`int` → `double`). Each test also checks that both versions of the program reach the solver, so a test cannot pass because the edited program fails to parse. - **`enumConstantsAreValuesNotVariables`:** `photo(Mode.Photo)` and `photo(Mode.Video)` differ only in the enum constant, and the second call must be reported. A first version of this PR renamed enum constants like variables, so both calls shared one key and the second could have reused the first's "ok". The check mode caught this while I was reviewing the code, and this test fails on that version. - **`QueryCacheTest`** checks the renaming, and that variable types and supertypes are part of the key. - **Mutation check:** I removed each part of the key in turn. Removing any of these makes a test fail: - variable types; - ghost declarations; - literal values; - supertypes; - the exclusion of enum constants from renaming. ## Same results I compared every diagnostic, with its full rendered message, hints and counterexamples included, against `main`. These results are for the final version. All cases ran in a single JVM, so specs are shared between cases and the cache gets many hits: | Corpus | Cases | `check` mode: mismatches / crashes | `on` mode: verdict / message diffs vs `main` | |---|---|---|---| | `testSuite` | 298 | 0 / 0 | 0 / 0 | | Study exercises (`exercises`, `exercises-oss`) | 98 | 0 / 0 | 0 / 1* | | Barista conformance tests | 3,262 | 0 / 0 | 0 / 0 | \* The variable number in one message (`transactionCommitted²⁷` vs `²⁸`). The messages are otherwise identical. This is the existing determinism issue described below. `mvn test`: 205 tests, 0 failures. ## Timings Study workspace, the same 15 files verified 5 times in one JVM, like 5 saves. The machine was shared, so main-thread CPU time is shown too: | | cache off | cache on | |---|---|---| | First save, CPU | 14.2 s | 9.1 s (queries repeat within a run) | | Later saves, CPU | 9.1–10.4 s | **2.2–4.6 s** | | Z3 solves, all 5 saves | 5,965 | 716 | Corpus totals (each corpus in one JVM, on a shared machine): Barista 9,667 s → 349 s, exercises 207 s → 77 s. The CLI verifies once and exits. It still benefits within a run, from repeated queries, and otherwise behaves as before. ## Not changed / known - **Dead code:** `SMTEvaluator` lost a `catch (SyntaxException)` that nothing could throw. It only compiled because `TranslatorToZ3.close()` declares `throws Exception`, and with the solving moved into its own method it no longer compiles. - **Key maintenance:** if `TranslatorToZ3` starts reading something new from the context, the key must include it. The class comment says so, and `check` mode in the tests is the safety net. - **An existing determinism issue on `main`:** the numbering of fresh variables in one message (`transactionCommitted²⁷` vs `²⁸` in `E_OSS_A1_50`) depends on what was verified earlier in the same JVM. It shows with the cache off as well, and does not affect verdicts. I'll look into it separately. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 1a506fe commit c5ac281

10 files changed

Lines changed: 634 additions & 56 deletions

File tree

‎liquidjava-verifier/pom.xml‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,10 @@
8989
<version>3.2.1</version>
9090
<configuration>
9191
<trimStackTrace>true</trimStackTrace>
92+
<systemPropertyVariables>
93+
<!-- recompute every result found in the solver cache and fail if it differs -->
94+
<liquidjava.cache>check</liquidjava.cache>
95+
</systemPropertyVariables>
9296
</configuration>
9397
</plugin>
9498

‎liquidjava-verifier/src/main/java/liquidjava/processor/refinement_checker/TypeChecker.java‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -415,13 +415,19 @@ public boolean checkStateSMT(Predicate prevState, Predicate expectedState, Sourc
415415

416416
public boolean checkStateSMT(Predicate prevState, Predicate expectedState, SourcePosition p, boolean silent)
417417
throws LJError {
418-
return verifyStateSMT(prevState, expectedState, p, silent).isOk();
418+
return verifyStateSMT(prevState, expectedState, p, silent, false).isOk();
419419
}
420420

421421
/** Checks that {@code prevState} implies {@code expectedState}, with a counterexample when it does not */
422422
public SMTResult verifyStateSMT(Predicate prevState, Predicate expectedState, SourcePosition p, boolean silent)
423423
throws LJError {
424-
return vcChecker.verifySMTSubtypeStates(prevState, expectedState, context.getGhostStates(), p, factory, silent);
424+
return verifyStateSMT(prevState, expectedState, p, silent, true);
425+
}
426+
427+
private SMTResult verifyStateSMT(Predicate prevState, Predicate expectedState, SourcePosition p, boolean silent,
428+
boolean needsCounterexample) throws LJError {
429+
return vcChecker.verifySMTSubtypeStates(prevState, expectedState, context.getGhostStates(), p, factory, silent,
430+
needsCounterexample);
425431
}
426432

427433
public void throwRefinementError(SourcePosition position, SourcePosition declarationPosition,

‎liquidjava-verifier/src/main/java/liquidjava/processor/refinement_checker/VCChecker.java‎

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ public void processSubtyping(Predicate expectedType, List<GhostState> list, CtEl
7171
DebugLog.smtStart(impl, expected);
7272
SMTResult result;
7373
try {
74-
result = dischargeToSMT(expected, premises, annotationValuePos, true);
74+
result = dischargeToSMT(expected, premises, annotationValuePos, true, true);
7575
} catch (SMTUnknownError error) {
7676
error.setDeclarationPosition(declarationPosition);
7777
DebugLog.smtError(error.getMessage());
@@ -123,13 +123,13 @@ public void processSubtyping(Predicate type, Predicate expectedType, List<GhostS
123123
*/
124124
public SMTResult verifySMTSubtype(Predicate expected, Predicate found, SourcePosition position) throws LJError {
125125
DebugLog.smtVerifying(position);
126-
return dischargeToSMT(expected, found, position, false);
126+
return dischargeToSMT(expected, found, position, false, true);
127127
}
128128

129-
private SMTResult dischargeToSMT(Predicate expected, Predicate found, SourcePosition position, boolean silent)
130-
throws LJError {
129+
private SMTResult dischargeToSMT(Predicate expected, Predicate found, SourcePosition position, boolean silent,
130+
boolean needsCounterexample) throws LJError {
131131
try {
132-
return new SMTEvaluator().verifySubtype(found, expected, context, silent);
132+
return new SMTEvaluator().verifySubtype(found, expected, context, silent, needsCounterexample);
133133
} catch (LJError e) {
134134
if (e.getPosition() == null) {
135135
e.setPosition(position);
@@ -158,6 +158,15 @@ public SMTResult verifySMTSubtypeStates(Predicate type, Predicate expectedType,
158158

159159
public SMTResult verifySMTSubtypeStates(Predicate type, Predicate expectedType, List<GhostState> states,
160160
SourcePosition position, Factory factory, boolean silent) throws LJError {
161+
return verifySMTSubtypeStates(type, expectedType, states, position, factory, silent, true);
162+
}
163+
164+
/**
165+
* @param needsCounterexample
166+
* whether the caller uses the counterexample when the verification fails
167+
*/
168+
public SMTResult verifySMTSubtypeStates(Predicate type, Predicate expectedType, List<GhostState> states,
169+
SourcePosition position, Factory factory, boolean silent, boolean needsCounterexample) throws LJError {
161170
if (!silent) {
162171
DebugLog.smtVerifying(position);
163172
}
@@ -185,7 +194,7 @@ public SMTResult verifySMTSubtypeStates(Predicate type, Predicate expectedType,
185194

186195
SMTResult result;
187196
try {
188-
result = dischargeToSMT(expected, premises, position, true);
197+
result = dischargeToSMT(expected, premises, position, true, needsCounterexample);
189198
} catch (RuntimeException ex) {
190199
if (!silent) {
191200
DebugLog.smtError(ex.getMessage());

‎liquidjava-verifier/src/main/java/liquidjava/rj_language/opt/VCConstraintSimplification.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ private VCImplication remove(VCImplication implication, VCImplication removed) {
7171
*/
7272
private boolean implies(Predicate stronger, Predicate weaker) {
7373
try {
74-
SMTResult result = new SMTEvaluator().verifySubtype(stronger, weaker, Context.getInstance(), true);
74+
SMTResult result = new SMTEvaluator().verifySubtype(stronger, weaker, Context.getInstance(), true, false);
7575
return result.isOk();
7676
} catch (Exception e) {
7777
return false;
Lines changed: 245 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,245 @@
1+
package liquidjava.smt;
2+
3+
import java.util.ArrayList;
4+
import java.util.Collection;
5+
import java.util.Collections;
6+
import java.util.HashSet;
7+
import java.util.LinkedHashMap;
8+
import java.util.List;
9+
import java.util.Map;
10+
import java.util.Optional;
11+
import java.util.Set;
12+
import java.util.stream.Collectors;
13+
14+
import liquidjava.processor.context.Context;
15+
import liquidjava.processor.context.GhostFunction;
16+
import liquidjava.processor.context.RefinedVariable;
17+
import liquidjava.rj_language.ast.AliasInvocation;
18+
import liquidjava.rj_language.ast.BinaryExpression;
19+
import liquidjava.rj_language.ast.Enum;
20+
import liquidjava.rj_language.ast.Expression;
21+
import liquidjava.rj_language.ast.FunctionInvocation;
22+
import liquidjava.rj_language.ast.Ite;
23+
import liquidjava.rj_language.ast.LiteralBoolean;
24+
import liquidjava.rj_language.ast.LiteralChar;
25+
import liquidjava.rj_language.ast.LiteralInt;
26+
import liquidjava.rj_language.ast.LiteralLong;
27+
import liquidjava.rj_language.ast.LiteralReal;
28+
import liquidjava.rj_language.ast.LiteralString;
29+
import liquidjava.rj_language.ast.UnaryExpression;
30+
import liquidjava.rj_language.ast.Var;
31+
import liquidjava.rj_language.visitors.ExpressionVisitor;
32+
import spoon.reflect.declaration.CtEnum;
33+
import spoon.reflect.reference.CtTypeReference;
34+
35+
/**
36+
* Results of solver queries, kept across verification runs. A long-lived process such as the language server verifies
37+
* the whole program again after every save, and asks the solver mostly the same queries each time.
38+
* <p>
39+
* A query is identified by everything {@link TranslatorToZ3} reads to build it: the expression, node by node, the type
40+
* and supertypes of each of its variables, and the enums, ghosts and ghost states of the context. Variables are renamed
41+
* in order of appearance, so that a query whose variables were numbered differently by an earlier edit is still found.
42+
* Names that denote enum constants are values, not variables, and keep their name. A counterexample names the
43+
* variables, so a cached one is only reused when the names are the same; otherwise a caller that needs one asks the
44+
* solver again. Unknown results and errors are never kept. Results only live as long as the process, so the verifier
45+
* and the solver cannot change under them.
46+
*/
47+
final class QueryCache {
48+
49+
private static final int MAX_RESULTS = 50_000;
50+
51+
private static final Map<String, Entry> RESULTS = Collections.synchronizedMap(new LinkedHashMap<>(16, 0.75f, true) {
52+
@Override
53+
protected boolean removeEldestEntry(Map.Entry<String, Entry> eldest) {
54+
return size() > MAX_RESULTS;
55+
}
56+
});
57+
58+
/**
59+
* @param text
60+
* the query with its variables renamed
61+
* @param names
62+
* the variables of the query, in the order they were renamed
63+
*/
64+
record Key(String text, List<String> names) {
65+
}
66+
67+
/** A result, whose counterexample names the variables in {@code names} */
68+
private record Entry(SMTResult result, List<String> names) {
69+
}
70+
71+
private QueryCache() {
72+
}
73+
74+
static Key key(Expression expression, Context context) {
75+
Map<String, CtTypeReference<?>> types = context.getContext();
76+
List<String> occurrences = new ArrayList<>();
77+
expression.getVariableNames(occurrences);
78+
Set<String> constants = enumConstants(types.values());
79+
List<String> names = occurrences.stream().distinct().filter(n -> types.containsKey(n) && !constants.contains(n))
80+
.toList();
81+
82+
StringBuilder text = new StringBuilder(expression.accept(new Canonical(names))).append('\n');
83+
Map<String, List<String>> supertypes = new LinkedHashMap<>();
84+
for (RefinedVariable variable : context.getAllVariablesWithSupertypes())
85+
if (names.contains(variable.getName()))
86+
supertypes.computeIfAbsent(variable.getName(), k -> new ArrayList<>()).add(
87+
variable.getSuperTypes().stream().map(QueryCache::describe).collect(Collectors.joining(", ")));
88+
for (int i = 0; i < names.size(); i++) {
89+
String name = names.get(i);
90+
text.append('@').append(i).append(": ").append(describe(types.get(name)));
91+
supertypes.getOrDefault(name, List.of()).forEach(s -> text.append(" <: ").append(s));
92+
text.append('\n');
93+
}
94+
// every enum of the context, as their constants can be named in the query
95+
types.values().stream().filter(CtTypeReference::isEnum).map(QueryCache::describe).distinct()
96+
.forEach(e -> text.append("enum ").append(e).append('\n'));
97+
for (GhostFunction ghost : context.getGhosts())
98+
text.append("ghost ").append(describe(ghost)).append('\n');
99+
for (GhostFunction state : context.getGhostStates())
100+
text.append("state ").append(describe(state)).append('\n');
101+
return new Key(text.toString(), names);
102+
}
103+
104+
/**
105+
* The result kept for {@code key}, or nothing if there is none or it cannot be used: a counterexample is only
106+
* reused when the variables have the same names
107+
*/
108+
static Optional<SMTResult> get(Key key, boolean needsCounterexample) {
109+
Entry entry = RESULTS.get(key.text());
110+
if (entry == null)
111+
return Optional.empty();
112+
if (entry.result().isOk() || entry.names().equals(key.names()))
113+
return Optional.of(entry.result());
114+
return needsCounterexample ? Optional.empty() : Optional.of(SMTResult.error(new Counterexample(List.of())));
115+
}
116+
117+
static void put(Key key, SMTResult result) {
118+
RESULTS.put(key.text(), new Entry(result, key.names()));
119+
}
120+
121+
/**
122+
* Fails if {@code recomputed} differs from {@code cached}, with an {@link AssertionError} so that code that
123+
* recovers from exceptions cannot hide it
124+
*/
125+
static void checkSame(Key key, SMTResult cached, SMTResult recomputed) {
126+
boolean same = cached.isOk() == recomputed.isOk() && (cached.isOk() || cached.getCounterexample().isEmpty()
127+
|| cached.getCounterexample().equals(recomputed.getCounterexample()));
128+
if (!same)
129+
throw new AssertionError(
130+
String.format("The solver query cache returned %s but recomputing gives %s for%n%s",
131+
describe(cached), describe(recomputed), key.text()));
132+
}
133+
134+
/** The names the translator binds to the constants of the enums, which denote values rather than variables */
135+
private static Set<String> enumConstants(Collection<CtTypeReference<?>> types) {
136+
Set<String> constants = new HashSet<>();
137+
for (CtTypeReference<?> type : types)
138+
if (type.isEnum() && type.getDeclaration()instanceof CtEnum<?> declaration)
139+
declaration.getEnumValues().forEach(
140+
v -> constants.add(TranslatorContextToZ3.enumConstantName(declaration, v.getSimpleName())));
141+
return constants;
142+
}
143+
144+
private static String describe(SMTResult result) {
145+
return result.isOk() ? "ok" : "error with " + result.getCounterexample();
146+
}
147+
148+
/** What the translator reads from a type: its names, and the constants of an enum */
149+
private static String describe(CtTypeReference<?> type) {
150+
if (type == null)
151+
return "?";
152+
if (type.isEnum() && type.getDeclaration()instanceof CtEnum<?> declaration)
153+
return type.getQualifiedName() + declaration.getEnumValues().stream().map(v -> v.getSimpleName()).toList();
154+
return type.getQualifiedName() + " " + type;
155+
}
156+
157+
private static String describe(GhostFunction ghost) {
158+
return ghost.getQualifiedName() + ghost.getParametersTypes().stream().map(Object::toString).toList() + " "
159+
+ ghost.getReturnType();
160+
}
161+
162+
/** The expression node by node, with its variables renamed in order of appearance */
163+
private static final class Canonical implements ExpressionVisitor<String> {
164+
165+
private final List<String> names;
166+
167+
Canonical(List<String> names) {
168+
this.names = names;
169+
}
170+
171+
private String all(List<Expression> expressions) {
172+
return expressions.stream().map(e -> e.accept(this)).collect(Collectors.joining(" "));
173+
}
174+
175+
@Override
176+
public String visitAliasInvocation(AliasInvocation alias) {
177+
return "(alias " + alias.getName() + " " + all(alias.getArgs()) + ")";
178+
}
179+
180+
@Override
181+
public String visitBinaryExpression(BinaryExpression exp) {
182+
return "(" + exp.getOperator() + " " + exp.getFirstOperand().accept(this) + " "
183+
+ exp.getSecondOperand().accept(this) + ")";
184+
}
185+
186+
@Override
187+
public String visitFunctionInvocation(FunctionInvocation fun) {
188+
return "(call " + fun.getName() + " " + all(fun.getArgs()) + ")";
189+
}
190+
191+
@Override
192+
public String visitIte(Ite ite) {
193+
return "(ite " + ite.getCondition().accept(this) + " " + ite.getThen().accept(this) + " "
194+
+ ite.getElse().accept(this) + ")";
195+
}
196+
197+
@Override
198+
public String visitLiteralInt(LiteralInt lit) {
199+
return "(int " + lit.getValue() + ")";
200+
}
201+
202+
@Override
203+
public String visitLiteralLong(LiteralLong lit) {
204+
return "(long " + lit.getValue() + ")";
205+
}
206+
207+
@Override
208+
public String visitLiteralBoolean(LiteralBoolean lit) {
209+
return "(bool " + lit.isBooleanTrue() + ")";
210+
}
211+
212+
@Override
213+
public String visitLiteralChar(LiteralChar lit) {
214+
return "(char " + (int) lit.getValue() + ")";
215+
}
216+
217+
@Override
218+
public String visitLiteralReal(LiteralReal lit) {
219+
return "(real " + Double.doubleToLongBits(lit.getValue()) + ")";
220+
}
221+
222+
@Override
223+
public String visitLiteralString(LiteralString lit) {
224+
return "(string " + lit.toString().length() + ":" + lit + ")";
225+
}
226+
227+
@Override
228+
public String visitUnaryExpression(UnaryExpression exp) {
229+
return "(" + exp.getOp() + " " + exp.getExpression().accept(this) + ")";
230+
}
231+
232+
@Override
233+
public String visitEnum(Enum en) {
234+
Expression literal = en.getResolvedLiteral();
235+
return "(const " + en.getTypeName() + " " + en.getConstName() + " "
236+
+ (literal == null ? "-" : literal.accept(this)) + ")";
237+
}
238+
239+
@Override
240+
public String visitVar(Var var) {
241+
int index = names.indexOf(var.getName());
242+
return index < 0 ? "(free " + var.getName().length() + ":" + var.getName() + ")" : "@" + index;
243+
}
244+
}
245+
}

0 commit comments

Comments
 (0)