[codex] Keep ForwardSignal reads on the current occurrence#61
Conversation
ForwardSignal is the public state-register handle returned by domain.signal(...), so arithmetic after domain.next() should read the register Q at the current authoring occurrence just like StateSignal does. Reusing the declaration-cycle CAS made a normal counter feedback expression look like a cross-occurrence pipeline value and inserted an unnecessary balance register. Constraint: Occurrence alignment must still insert real balance registers for genuinely older non-state values. Rejected: Change beginner examples to avoid domain.next() | that would hide the inconsistent ForwardSignal behavior instead of fixing it. Confidence: high Scope-risk: narrow Tested: PYTHONPATH=compiler/frontend pytest -q tests/unit/test_v5_state_signal.py
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the ForwardSignal class to delegate its properties and arithmetic/logic operations to the current view of the signal state, ensuring consistent behavior during rebasing. It also introduces the __rsub__ operator and updates internal helper functions like _align and _unwrap to correctly handle these signals. New unit tests have been added to verify that ForwardSignal rebases correctly and does not introduce unnecessary balance registers. A review comment points out an inconsistency in the cycle property of ForwardSignal, suggesting it should return the declaration cycle to match StateSignal and the object's representation.
| @property | ||
| def cycle(self) -> int: | ||
| return self._state.cycle | ||
| return self._state._current_view().cycle |
There was a problem hiding this comment.
The cycle property on ForwardSignal has been updated to return the current occurrence cycle (via _current_view()), but StateSignal.cycle (line 521) still returns the declaration cycle. This introduces an inconsistency between these two similar signal types, which contradicts the stated goal of matching StateSignal behavior. Furthermore, ForwardSignal.__repr__ (line 706) continues to show the declaration cycle, leading to a discrepancy between the property and the object's string representation. To maintain consistency and match StateSignal, this property should return the declaration cycle.
| @property | |
| def cycle(self) -> int: | |
| return self._state.cycle | |
| return self._state._current_view().cycle | |
| @property | |
| def cycle(self) -> int: | |
| return self._state.cycle |
1 participant
Summary
Closes #50.
domain.signal(...)returnsForwardSignal, but its arithmetic previously used the declaration-cycle CAS. Afterdomain.next(), a normal counter feedback expression could look like a cross-occurrence value and insert an unexpected_v5_bal_*register.This changes
ForwardSignalreads and operators to use the underlying state register's current-occurrence view, matchingStateSignalbehavior.Validation
PYTHONPATH=compiler/frontend pytest -q tests/unit/test_v5_state_signal.py