Skip to content

Commit 070fb75

Browse files
CatarinaGamboaclaude
CatarinaGamboa
and
claude
authored
Origin note: say where the wrong state of a receiver comes from (#528)
Closes #522, part 2: **origin notes F1 and F2**, where the wrong state comes from. Design: #522 (comment) **Base: `main`.** #526 is merged. #530 is stacked on this PR, so retarget it to `main` before merging this one with `--delete-branch`. ## The Origin note The PM's review renamed the note from **"From:"** to **"Origin:"**, because "From" read like `@StateRefinement(from = …)` in declared-here. Every origin note now has one shape, `Origin: <where>, <call or variable> <verb> <state or refinement>`, and only the verb changes. The first commit renames the note #526 merged (`FromNotes` → `OriginNotes`, `TestFromNotes` → `TestOriginNotes`). The soundness conditions are unchanged. | Case | Text | |---|---| | F1, the call that last set the sets | `Origin: line 134, new BufferedInputStream(stream) makes markState(buffered) == unmarked` | | F2, a returned value | `Origin: line 130, writer.getDefaultWriteParam() returns compression(wParam) == compressionNotExplicit` | | F2, nothing known | `Origin: line 69, frame.getUndoManager() says nothing about redoState(undoManager)` | | F1 fallback, creation | `Origin: line L, new T(…) creates x with <state>` | | F3 (#526) | `Origin: line 94, timeout is a parameter of checkCimReachable(String, int) with no refinement` | "Makes" is used only for the call the walk proves last set the shown sets. The fallback says "creates … with", because a later call may have set the state again. That is a fact about the constructor, not about the current state. ## When there is no note at all No origin note is given when the receiver may change state out of the verifier's sight before the call. That covers three cases: - **Another name may hold it.** A local, a parameter or `this` is used, before the call, other than as the target of a call or field access or in `==`/`!=`. That means copied, passed as an argument (including to the failing call itself), stored in an array, returned, bound in a method reference (`x::start`), or used in a lambda or a local class. "Before" means earlier in the source, in the failing call's arguments, or in a loop around the call, unless the use is inside a `return`/`throw`. - **A method of the analyzed code with no state refinement is called on it before.** The verifier assumes such a call keeps the state. - **It is a field.** Declared state contracts are trusted, as the verifier trusts them. A body that breaks its own declared contract is a verifier issue, outside the notes (see the round 3 reply). ## Which sets The note covers the sets that the failing call's `from` checks. When the state machine models the call, it narrows them to the sets where some state the receiver may be in is the state of no allowed state. This only selects what is shown (S1). ## F1, a state set by a call Walk back from the receiver through `StateChange`/`previous()` to the first change whose `to` names a shown set. The note is printed only if: - every instance on the way has a `StateChange`, so the walk crosses no join, loop head or unknown origin; - every change crossed names none of the shown sets. Each `to` keeps the sets it does not name, through `set(this) == set(old(this))`; - `StateSolver.implies(<instance after that change>, <Found's facts on the shown sets>)` holds. Text: `Origin: line L, <call> makes <Found on the sets that change names>`. With two wrong sets, a change to one is not given the other. **Fallback, when the walk breaks.** The note gives the creation fact only if all hold: - the receiver is a local whose only write is its initializer, a constructor call with a single state refinement; - no other call on it is to a method that changes state, and no loop may repeat the failing call; - Found's facts on the shown sets imply what that constructor gives. Text: `Origin: line L, new T(…) creates x with <state>`. It is a fact about the constructor. The last condition stops it from speaking when the state may have changed since (A1_09). ## F2, a state from a returned value Printed only if the walk reaches, possibly after crossing changes to other sets, an instance with no `StateChange` that holds the call result it was assigned. Instances now record a `CallResult` when a local is declared or assigned from a call to a refined method. A state-preserving call keeps it, as it keeps the `StateChange`. Joins, loop heads and transitions make new instances without one. - `Origin: line L, <call> returns <facts>`: the conjuncts of the instantiated return refinement on the shown sets. Printed only if they imply Found's facts on those sets. - `Origin: line L, <call> says nothing about <set>(x)`: printed only if all hold: - Found has no fact on the sets; - the method has no state refinement with a `to` (as `to = "running(_)"`); - the return refinement names no state of them, mentions no variable but `_`, and uses no alias. This rules out `_ == this`-style returns; - no shown set has a generated name (`state1`), which the message would otherwise never show. ## DebugLog check for A1_38 (asked before coding F1) With `-d`, the VC at `buffered.reset()` (line 157) has a single `buffered` instance, `#buffered_259 : (stream == open) ∧ (markState == unmarked)`. That is the constructor's state, with no ITE and no loop-head instance. The note also took the F1 path, which needs a `StateChange` on that instance. So neither loop (lines 136 and 150) creates a join for `buffered`. A broken walk now logs `[WARN] Origin note: no known call gives #x_n its state, e.g. a join of branches`. A1_09 logs it for `#host_95`. ## Study tasks, before (main) and after ```diff State Refinement Error: found stream(buffered) == open && markState(buffered) == unmarked but expected markState(buffered) == marked && stream(buffered) == open + Origin: line 134, new BufferedInputStream(stream) makes markState(buffered) == unmarked ``` ```diff State Refinement Error: found compression(wParam) == compressionNotExplicit but expected compression(wParam) == compressionExplicit || compression(wParam) == compressionTypeSet + Origin: line 130, writer.getDefaultWriteParam() returns compression(wParam) == compressionNotExplicit ``` ```diff State Refinement Error: expected redoState(undoManager) == redoable but could not prove it + Origin: line 69, frame.getUndoManager() says nothing about redoState(undoManager) ``` ```diff Refinement Error: expected timeout >= 0 but could not prove it - From: timeout is a parameter of checkCimReachable(String, int) (line 94) with no refinement + Origin: line 94, timeout is a parameter of checkCimReachable(String, int) with no refinement ``` - A1_09: no origin note. Its receiver is a join, and Found does not say the state `new JFrame(…)` gives. - A2_10, practice and practice2: unchanged. Each task still gives exactly one error, on its `reportedLine`. All three notes are the review's mock-ups, word for word. I checked every state-error fixture of the suite. Each note names the creating or the last call. After the Codex rounds, the fallback fires in none, and fixtures where the receiver is captured, a field, `this` in a loop, or given an unspecified call get no note. ## Tests `TestOriginNotes`, three tests that fail without the change: - `ErrorStateHintMultipleSets` (existing): `Origin: line 27, d.lock() makes lockState(d) == locked`. Only the wrong set is named, and the walk stops at the change to it; - `ErrorStateFromReturnedValue` (new, small; no existing fixture assigns a state from a return refinement): `Origin: line 27, FromReturnedTask.create() returns phase(task) == idle`. - `ErrorDeferredCallbackState` (existing): the receiver captured by a lambda gets no note. `mvn test`: 736 run, 0 failures. ## Conflicts - With #520/#521 (another agent: `AuxStateHandler` declared-here): the one `throwStateRefinementError` call in `changeState`, which gets a `messageNotes` argument. The rest is additive. - With #518/#519 (`StateHints`): none. - With #527 (state counterexample, mine): both add a parameter to `throwStateRefinementError`, a one-line rebase. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 920bf15 commit 070fb75

13 files changed

Lines changed: 533 additions & 49 deletions

File tree

‎liquidjava-example/src/main/java/testSuite/ErrorStateFromReturnedValue.java‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
package testSuite;
2+
3+
import liquidjava.specification.Refinement;
4+
import liquidjava.specification.StateRefinement;
5+
import liquidjava.specification.StateSet;
6+
7+
// The state of an object assigned the value of a call comes from the call's return refinement
8+
@StateSet(name = "phase", value = {"idle", "running"})
9+
class FromReturnedTask {
10+
@StateRefinement(to = "idle(this)")
11+
FromReturnedTask() {}
12+
13+
@StateRefinement(from = "idle(this)", to = "running(this)")
14+
void start() {}
15+
16+
@StateRefinement(from = "running(this)")
17+
void await() {}
18+
19+
@Refinement("idle(_)")
20+
static FromReturnedTask create() {
21+
return new FromReturnedTask();
22+
}
23+
}
24+
25+
public class ErrorStateFromReturnedValue {
26+
static void use() {
27+
FromReturnedTask task = FromReturnedTask.create();
28+
task.await(); // Expect: State Refinement Error
29+
}
30+
}

‎liquidjava-verifier/src/main/java/liquidjava/processor/context/CallResult.java‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
package liquidjava.processor.context;
2+
3+
import liquidjava.rj_language.Predicate;
4+
import spoon.reflect.cu.SourcePosition;
5+
6+
/**
7+
* The value a call to a refined method returned
8+
*
9+
* @param function
10+
* the method called
11+
* @param returned
12+
* the method's return refinement at this call, with its arguments and receiver substituted and {@code _} for
13+
* the value
14+
* @param position
15+
* where the call is
16+
*/
17+
public record CallResult(RefinedFunction function, Predicate returned, SourcePosition position) {
18+
}

‎liquidjava-verifier/src/main/java/liquidjava/processor/context/VariableInstance.java‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ public class VariableInstance extends RefinedVariable {
88

99
private Variable parent;
1010
private StateChange stateChange;
11+
private CallResult returnedBy;
1112

1213
public VariableInstance(String name, CtTypeReference<?> type, Predicate c) {
1314
super(name, type, c);
@@ -43,6 +44,15 @@ public void setStateChange(StateChange stateChange) {
4344
this.stateChange = stateChange;
4445
}
4546

47+
/** The call whose value this instance was assigned, if it was assigned one and its state has not changed since */
48+
public Optional<CallResult> getReturnedBy() {
49+
return Optional.ofNullable(returnedBy);
50+
}
51+
52+
public void setReturnedBy(CallResult returnedBy) {
53+
this.returnedBy = returnedBy;
54+
}
55+
4656
public Optional<Variable> getParent() {
4757
return parent == null ? Optional.empty() : Optional.of(parent);
4858
}

liquidjava-verifier/src/main/java/liquidjava/processor/refinement_checker/FromNotes.java renamed to liquidjava-verifier/src/main/java/liquidjava/processor/refinement_checker/OriginNotes.java

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,12 @@
1818
import liquidjava.utils.Utils;
1919

2020
/**
21-
* The notes of a refinement error that say where the values its expected refinement is about come from. Each is given
22-
* only when it restates what the verifier used, and is left out otherwise
21+
* The origin notes of a refinement error, which say where the values its expected refinement is about come from. Each
22+
* is given only when it restates what the verifier used, and is left out otherwise
2323
*/
24-
final class FromNotes {
24+
final class OriginNotes {
2525

26-
private FromNotes() {
26+
private OriginNotes() {
2727
}
2828

2929
/**
@@ -46,9 +46,9 @@ static List<String> unrefinedParameters(RefinementError error, CtElement checked
4646
|| !(context.getVariableByName(variable)instanceof Variable declared)
4747
|| !declared.getMainRefinement().isBooleanTrue() || isRefined(parameter.get(), context))
4848
return List.of();
49-
notes.add(String.format("From: %s is a parameter of %s (line %d) with no refinement",
50-
error.getDisplayNames().get(variable), signature(method.get()),
51-
parameter.get().getPosition().getLine()));
49+
notes.add(String.format("Origin: line %d, %s is a parameter of %s with no refinement",
50+
parameter.get().getPosition().getLine(), error.getDisplayNames().get(variable),
51+
signature(method.get())));
5252
}
5353
return notes;
5454
}

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

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ public class RefinementTypeChecker extends TypeChecker {
9999
ContextHistory contextHistory = ContextHistory.getInstance();
100100
// the metadata that checking an element puts on it
101101
private static final List<String> CHECK_METADATA = List.of(Keys.REFINEMENT, Keys.TARGET, Keys.RECEIVER,
102-
Keys.RETURN_VAR_NAME, Keys.STATE_CHANGE);
102+
Keys.RETURN_VAR_NAME, Keys.STATE_CHANGE, Keys.CALL_RESULT);
103103
// the loops being checked, innermost first
104104
private final Deque<LoopTypestates> loops = new ArrayDeque<>();
105105

@@ -195,9 +195,16 @@ public <T> void visitCtLocalVariable(CtLocalVariable<T> localVariable) {
195195
context.addVarToContext(varName, localVariable.getType(), new Predicate(), e);
196196
checkVariableRefinements(refinementFound, varName, localVariable.getType(), localVariable, localVariable);
197197
AuxStateHandler.addStateRefinements(this, varName, e);
198+
recordReturnedValue(varName, e);
198199
}
199200
}
200201

202+
/** Records on the new instance of a local variable the call whose value it was assigned, if any */
203+
private void recordReturnedValue(String name, CtExpression<?> value) {
204+
if (value != null && value.getMetadata(Keys.CALL_RESULT)instanceof CallResult result)
205+
context.getLastVariableInstance(name).ifPresent(instance -> instance.setReturnedBy(result));
206+
}
207+
201208
@Override
202209
public <T> void visitCtCatchVariable(CtCatchVariable<T> catchVariable) {
203210
super.visitCtCatchVariable(catchVariable);
@@ -271,6 +278,8 @@ private <T, A extends T> void visitAssignment(CtAssignment<T, A> assignment) thr
271278
CtVariable<T> varDecl = (CtVariable<T>) var.getDeclaration();
272279
String name = var.getSimpleName();
273280
checkAssignment(name, varDecl.getType(), ex, assignment.getAssignment(), assignment, varDecl);
281+
if (varDecl instanceof CtLocalVariable<?> && !(assignment instanceof CtOperatorAssignment<?, ?>))
282+
recordReturnedValue(name, assignment.getAssignment());
274283

275284
} else if (ex instanceof CtFieldWrite<?> fw) {
276285
CtFieldReference<?> cr = fw.getVariable();

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -614,14 +614,14 @@ public void throwRefinementError(SourcePosition position, SourcePosition declara
614614
public void throwStateRefinementError(SourcePosition position, SourcePosition declarationPosition, Predicate found,
615615
Predicate expected, String customMessage) throws LJError {
616616
throwStateRefinementError(position, List.of(declarationPosition), found, expected, customMessage, List.of(),
617-
null, null, List.of());
617+
null, null, List.of(), List.of());
618618
}
619619

620620
public void throwStateRefinementError(SourcePosition position, List<SourcePosition> declarationPositions,
621621
Predicate found, Predicate expected, String customMessage, List<Hint> hints, ErrorCategory category,
622-
StateErrorContext context, List<String> declarationNotes) throws LJError {
622+
StateErrorContext context, List<String> declarationNotes, List<String> messageNotes) throws LJError {
623623
vcChecker.throwStateRefinementError(position, declarationPositions, found, expected, customMessage, hints,
624-
category, context, declarationNotes);
624+
category, context, declarationNotes, messageNotes);
625625
}
626626

627627
public void throwStateConflictError(SourcePosition position, Predicate expectedType) throws LJError {

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

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ public void processSubtyping(Predicate expectedType, List<GhostState> list, CtEl
9090
RefinementError error = new RefinementError(element.getPosition(), declarationPosition, expectedType,
9191
implBeforeChange.simplify(NeededVariables.forRefinementError(expectedType)), map,
9292
result.getCounterexample(), customMessage, context.getAliases());
93-
FromNotes.unrefinedParameters(error, element, context).forEach(error::addMessageNote);
93+
OriginNotes.unrefinedParameters(error, element, context).forEach(error::addMessageNote);
9494
throw error;
9595
}
9696
}
@@ -459,16 +459,19 @@ protected Predicate shownFound(Predicate found, Predicate expected) {
459459

460460
protected void throwStateRefinementError(SourcePosition position, List<SourcePosition> declarationPositions,
461461
Predicate found, Predicate expected, String customMessage, List<Hint> hints, ErrorCategory category,
462-
StateErrorContext context, List<String> declarationNotes) throws StateRefinementError {
462+
StateErrorContext context, List<String> declarationNotes, List<String> messageNotes)
463+
throws StateRefinementError {
463464
TranslationTable map = new TranslationTable();
464465
VCImplication foundState = buildPremiseChain(map, expected, found);
465466
VCSimplificationResult simplified = foundState.simplify(NeededVariables.forStateError(expected));
466467
VCSimplificationResult shown = VCSimplification.simplifyOnce(simplified,
467468
new VCRelevantFacts(expected.getExpression(), this::ghostFunctionsOf));
468-
throw new StateRefinementError(position, declarationPositions, expected, shown, map, customMessage, hints,
469-
category, context,
469+
StateRefinementError error = new StateRefinementError(position, declarationPositions, expected, shown, map,
470+
customMessage, hints, category, context,
470471
e -> StateValues.toSetForm(e, this.context.getGhostStates(), this.context.getGhosts()),
471472
declarationNotes);
473+
messageNotes.forEach(error::addMessageNote);
474+
throw error;
472475
}
473476

474477
/** The ghost functions an invocation name stands for: a state's state set function, or the ghost itself */

‎liquidjava-verifier/src/main/java/liquidjava/processor/refinement_checker/general_checkers/MethodsFunctionsChecker.java‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
import java.util.Optional;
88

99
import liquidjava.diagnostics.errors.LJError;
10+
import liquidjava.processor.context.CallResult;
1011
import liquidjava.processor.context.Refined;
1112
import liquidjava.processor.context.RefinedFunction;
1213
import liquidjava.processor.context.RefinedVariable;
@@ -358,6 +359,7 @@ private Map<String, String> checkInvocationRefinements(CtExpression<?> invocatio
358359
? Predicate.createEquals(Predicate.createVar(Keys.WILDCARD), Predicate.createVar(returnViName))
359360
: new Predicate();
360361
invocation.putMetadata(Keys.REFINEMENT, returnRef);
362+
recordCallResult(invocation, f, new Predicate());
361363
return map;
362364
}
363365

@@ -371,6 +373,12 @@ private Map<String, String> checkInvocationRefinements(CtExpression<?> invocatio
371373
return map;
372374
}
373375

376+
/** Records on a call written in the source what it returns, for the variable it is assigned to */
377+
private static void recordCallResult(CtExpression<?> invocation, RefinedFunction f, Predicate returned) {
378+
if (invocation instanceof CtInvocation<?> && invocation.getPosition().isValidPosition())
379+
invocation.putMetadata(Keys.CALL_RESULT, new CallResult(f, returned, invocation.getPosition()));
380+
}
381+
374382
/**
375383
* Computes a stable return-value instance name so {@code _}/{@code return} in a {@code @StateRefinement to=} clause
376384
* matches the post-call VariableInstance. Returns {@code null} for void methods. When the function has no
@@ -412,6 +420,7 @@ private void applyReturnRefinement(CtExpression<?> invocation, RefinedFunction f
412420
varName = v.getName();
413421
}
414422

423+
recordCallResult(invocation, f, methodRef);
415424
String viName = returnViName != null ? returnViName
416425
: GeneratedNames.valueOf(invocation, rtc.getContext().getCounter());
417426
VariableInstance vi = (VariableInstance) rtc.getContext().addInstanceToContext(viName, f.getType(),

‎liquidjava-verifier/src/main/java/liquidjava/processor/refinement_checker/object_checkers/AuxStateHandler.java‎

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -827,8 +827,25 @@ private static void changeState(TypeChecker tc, VariableInstance vi, RefinedFunc
827827
StateErrorContext context = states
828828
.map(r -> StateErrorContext.of(r.machine(), function, r.current(), writtenReceiver(invocation)))
829829
.orElse(null);
830+
List<String> messageNotes = receiverOf(invocation)
831+
.flatMap(r -> fromNote(tc, vi, r, function, ghostStates, states, prevState, expected, invocation))
832+
.stream().toList();
830833
tc.throwStateRefinementError(invocation.getPosition(), declarationPositions, prevState,
831-
expectedStatesDisjunction, message, hints, category, context, declarationNotes);
834+
expectedStatesDisjunction, message, hints, category, context, declarationNotes, messageNotes);
835+
}
836+
}
837+
838+
/** Best effort, like {@link #receiverStates}: see {@link StateOrigin} */
839+
private static Optional<String> fromNote(TypeChecker tc, VariableInstance receiver, String written,
840+
RefinedFunction function, List<GhostState> ghostStates, Optional<ReceiverStates> states, Predicate found,
841+
Predicate expected, CtElement invocation) {
842+
try {
843+
return StateOrigin.note(tc, receiver, written, function, ghostStates, states.map(ReceiverStates::machine),
844+
states.map(ReceiverStates::current).orElse(Set.of()), found, expected, invocation);
845+
} catch (RuntimeException e) {
846+
// also an LJError, such as an unknown solver result: the note is left out, the error is still reported
847+
DebugLog.warn("Could not tell where the receiver's state comes from: " + e.getMessage());
848+
return Optional.empty();
832849
}
833850
}
834851

@@ -1042,8 +1059,9 @@ private static void sameState(TypeChecker tc, VariableInstance variableInstance,
10421059
Predicate c = variableInstance.getRefinement().substituteVariable(Keys.WILDCARD, newInstanceName)
10431060
.substituteVariable(variableInstance.getName(), newInstanceName);
10441061

1045-
addInstanceWithState(tc, name, newInstanceName, variableInstance, c, invocation)
1046-
.setStateChange(variableInstance.getStateChange().orElse(null));
1062+
VariableInstance same = addInstanceWithState(tc, name, newInstanceName, variableInstance, c, invocation);
1063+
same.setStateChange(variableInstance.getStateChange().orElse(null));
1064+
same.setReturnedBy(variableInstance.getReturnedBy().orElse(null));
10471065
}
10481066
}
10491067

0 commit comments

Comments
 (0)