Repository navigation
Suggest result checks and argument conditions in state hints - #474
Conversation
|
[Codex review] The parameterized guard hint is not a usable Java edit: the new test expects |
Methods whose state refinements mention their parameters or result were left out of the state machine, so the hints could not suggest checking canRedo() before redo(), or calling setCompressionMode(int) with mode == 2. They are now kept as call variants: with any arguments, with arguments meeting each condition the refinement tests, and, for a boolean result, with the result checked. Boolean methods whose return refinement relates the result to the state are kept as checks too. The solver works out what a variant does only when a hint needs it. A check is suggested only if the verifier allows it in every state the receiver can be in, its result can have the value checked, and the failing call is then allowed; if several checks would do, none is suggested. An argument condition is suggested only if every argument meeting it leads to a state that allows the call, and the value shown for an earlier call's argument is now checked the same way. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…mpiles Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Fixed in 9f90bd5: a guard is now written as a call with the parameters' names, so |
be9b5ba to
9f90bd5
Compare
|
[Codex re-review] The type-name issue is fixed, but the guard is still not usable as written in the new |
…ditions mention A guard is written as if (l.tryLock(...)), with the declaration tryLock(long timeout) to fill in, since the callee's parameter names are not in scope where the call is made. Argument conditions and deciding arguments show the parameters' names too, so "with lo < hi" says which argument is which. The path search ranks paths that need no argument conditions first before capping the paths to each set of states. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Fixed in 0afdf6a. A guard no longer presents the callee's parameter names as expressions: it is written The same problem existed in argument conditions and deciding arguments ( From a review of the rest of the PR, also fixed: the path search capped the paths to each set of states before preferring ones without argument conditions, so a later unconditioned path could be dropped; it now ranks first, then caps. Plus small cleanups (a dead local, a duplicated returnsValue check). Full suite passes locally (681 tests, 0 failures). |
|
[Codex re-review] The latest update resolves my parameterized guard comment: the hint now asks the caller to supply the arguments and names the formal parameters separately. I also checked the path ranking change; it keeps unconditioned paths before applying the per-state cap. I found no further issue in this update, and CI is green. |
Follow-up to #474, stacked on it: **review and merge #474 first**. The diff here is only the last commit. ## What State hints asked Z3 about every state set, even the ones a refinement never names. This PR stops asking about those sets and changes no hint. The verifier already keeps the state of every set a refinement's `to` does not name (`AuxStateHandler.addOldStates` adds `stateK(this) == stateK(old(this))`). Different state sets are different ghost functions. So: - **Entry.** Whether a `from` holds depends only on the sets it names, so it is asked once for each state of those sets, not once for each of the 12 combined states. This is done both when building the state machine (`StateMachineBuilder.transitionsOf`) and for hint variants (`SolverEffects.entry`). - **States after.** The only candidates are those that keep the sets `to` does not name. Where the call leads in the sets it does name depends only on the state before in the sets it names for `old(this)`, so that answer is reused across states (`statesAfter`, `SolverEffects.computeAfter`). - **Guards.** If the receiver can be in any combination of states, a guard whose state or return refinements name no state of the sets the failing call checks cannot change them. It is skipped before any query. An alias is assumed to name every set, because its body is not expanded at this point. ## Numbers: the study's UndoManager (A1_56 fixture, 12 states in 3 sets) | | #474 | this PR | |---|---|---| | Solver queries (whole file) | 1419 | 258 | | Machine build (entry + states after) | 951 | 187 | | Hint queries | 432 | 35 | | Wall time, whole run (3 runs) | 10.0–10.2 s | 5.0–6.1 s | ## Checks - **Same output.** On every test-suite path and every fixture project under `liquidjava-dev/fixtures`, errors and hints are identical to #474. - **Sweep time.** The test suite went from 1m25s to 1m07s and the fixtures from 25 s to 17 s. - **Full suite.** Passes locally: 685 tests, 0 failures. - **New unit tests** in `StateMachineTest` for `namedSets`, `namedSetsBefore` (only `old(this)` mentions count), the alias case and `keeping`. - **A bug the sweep caught.** Query guards such as `hasNext()` with `_ == hasMore(this)` keep their state in the return refinement, so the guard filter now looks there too. Before this fix, it dropped the `hasNext()`, `canUndo()` and `!isDisplayable()` guards. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Closes #482. ## What With more than one `@StateSet`, the call-sequence hint named every state set, including the ones the suggested calls leave as they are. It now names only the sets whose states differ before and after the calls. Only the displayed hint changes; no solver query changes. Reproducer from the issue: ``` before: Call s.mark() before this call to move s from open and unmarked to open and marked after: Call s.mark() before this call to move s from unmarked to marked ``` Existing fixture `ErrorStateHintMultipleSets`: ``` before: Call d.unlock() before this call to move d from closed and locked to closed and unlocked after: Call d.unlock() before this call to move d from locked to unlocked ``` ## How - `StateMachine.stateSetsChanged(before, after)`: the sets whose states differ. If the sets it would leave out, taken together, differ too (how states of different sets are combined changed), every set is named, as before. - `StateMachine.describeOnly(subset, sets)`: names `subset` only in the given sets, reusing `StateCover` (the naming from #474/#475) on the states that agree with `subset` in those sets. - `StateHints.callSequence` names both sides with these. `from any state` is unchanged. ## Not changed, on purpose - **`In state open and unmarked, s can call: mark()`** still names the full state: which calls are allowed depends on every set, so dropping `open` would make the hint less accurate. - **Found/expected** (`found open(s) && unmarked(s) but expected marked(s) && open(s)`) is left as it is. These are the verifier's real predicates (the simplified premise chain and the `from` disjunction, with arguments, ghosts and aliases). The IDE reads them too. Dropping or reordering conjuncts that hold on both sides would need a semantic check on arbitrary predicates. Doing that only for display is riskier than this PR should be. It could be a follow-up if the study needs it. ## Tests - `TestStateHints.callSequenceNamesOnlyTheStateSetsItChanges` (the renamed multi-set test; fails on main). - `StateMachineTest`: `statesAreNamedOnlyInTheSetsShown`, `setsChangedAreThoseWhoseStatesDiffer` (including the cases where how the sets are combined changes). - `mvn test`: 692 tests, 0 failures. No other expectation changed. The dcm4che hint (`from startCompression to compressionExplicit`) already named a single set. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Fixes #454, fixes #455, fixes #473.
What changes
The state machine behind the state hints used to drop every method whose state refinements mention a parameter or the result. That meant
canRedo()could never be suggested beforeredo(), andsetCompressionMode(int)withmode == 2could never be suggested beforesetCompressionType. Those methods are now kept as call variants:mode == 2,lo < hi, its negationlimit <= 0)_/!_)tomentions the resultBoolean methods without a state refinement whose return refinement relates the result to the state, such as
@Refinement("_ == true --> hasMore(this)") boolean hasNext(), are kept as guards too.What a variant does is worked out by the solver only when a hint needs it, and then cached. Building the machine costs the same as on main.
When a hint is given
fromholds in all of them. The result must be able to have the value checked, and the failing call must then be allowed in every state that is left. It is shown when the state is unknown or was checked without being decided. In a known state, it is shown only if no call sequence exists or the state depends on the path taken. If several checks would do, none is shown, because the refinements can't tell them apart (e.g. a coarse UndoManager spec wherecanUndo,canRedoandcanUndoOrRedoall imply the same state).Hints
A1_56 (study spec, three state sets):
After guarding
redo(), theundo()call gets "Guard this call with if (undoManager.canUndo()): when it returns true, undoManager is undoable", and with both guards the program verifies.A1_01:
Negated guard: "Guard this call with if (!c.isClosed()): when it returns false, c is open".
#454 can close fully. Part (a) (name only the needed state, don't suggest
undo()) was done in #458, and part (b) (suggestcanRedo()) is done here.Tests
ErrorStateHintGuards(11 cases: negated guard, guard with a parameter, known state, and 8 cases that must get no guard, including several checks proving the same thing)ErrorStateHintArgumentConditions(7 cases, including the State hint "with mode == N" uses one example value without checking it always works #473 example)CorrectStateHintGuards(the hints applied: it verifies)state_hint_compression_mode_error(A1_01)state_hint_study_undo_manager_error/_correct(A1_56 with the study's spec, unchanged)TestStateHintsasserts every hint text.StateMachineTestcovers how refinements are chosen for a set of states.Performance
For the study's 12-state UndoManager (
state_hint_study_undo_manager_error):The extra cost is the guard check in an unknown state, paid once per class per run. To keep it down, the possible states are narrowed one state set at a time, and a check of the result reuses the states found for any result.
🤖 Generated with Claude Code