Repository navigation
Drop true rows for variables that another row of Found already shows - #472
Merged
Merged
Conversation
A binder refined only by true states no fact. When another row of Found mentions its variable, the variable is already shown there and the true row only reads as if its value were true. Keep such a row only in a refinement error, for a variable whose value expected constrains and that no other row mentions: it then shows that nothing is known about a value the error depends on. This removes the linking to expected that kept getUndoManager, the parameters of a conditional state and enum constants, and the binder ordering pass that only moved those kept rows. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Collaborator
Author
|
[Codex review] I reviewed the latest binder filtering and removal of the ordering pass. The retained |
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.
Problem
After #459, Found still shows
truerows for variables that another row already shows (#375):EditAction, state error):∀getUndoManager true.PlayfieldWindow, state error):∀previousMode true,∀DisplayMode.BORDERLESS trueand∀newMode true, above the ternary that uses them.#459 kept a
truebinder when a fact linked to Expected constrains its value. That fact is always a row the user already sees, and it already names the variable. So the extratruerow adds no fact. It also reads as if the variable's value weretrue, which is what #375 is about. The A1_09 rows are not needed: the ternary row showspreviousMode,newModeandBORDERLESS, and that row explains whyhostmay still be displayable.DisplayMode.BORDERLESSis an enum constant, so a binder row for it says nothing.Rule
A binder refined only by
trueis shown only when all of these hold:When all three hold, the row is the only place the variable appears in Found. It tells the user that nothing is known about a value the error depends on (
∀t true, counterexamplet == 0). In every other case the row goes, because the variable is either shown in the rows that use it or not needed.No variable disappears from what the user sees:
RefinementError.filterCounterexampletakes its terms from Expected and from Found's facts, not from the binders, so for exampleBasket.served == -1still shows after its row goes.NeededVariablesno longer links facts to Expected. Two pieces of code had no other use, so this PR removes them:splitConjunctsandlinkedConjunctsinVCSimplificationUtils;VCBinderOrderingpass and its test. That pass only moved kepttruebinders before their first use. A kept binder now has no other use, so the pass could never fire.Before / after
A1_56 (
undoManager.redo()on a manager from a getter)A1_09 (
host.setUndecorated(true)after a conditionaldispose())Test suite fixture (
classes/found_binders_error)int sum = first + secondrefined_ > 0(new)∀first true,∀second true,∀sum sum == first + second∀sum sum == first + second, counterexamplesum == 0 && first == 0 && second == 0(unchanged)int count = served + 1(static field)∀Basket.served true,∀count count == Basket.served + 1∀count count == Basket.served + 1, counterexamplecount == 0 && Basket.served == -1(unchanged)positive(t)withtunconstrained (new, kept)∀t true, counterexamplet == 0In the same way,
ErrorStateHintPathDependentnow shows onlyready ? open(f) : created(f), without∀ready true.What the fixed
#freshpath rows still show (∀#fresh¹¹² !#fresh¹¹¹) is the separate part of #375 that ties in with #373. They are nottruerows, and this PR leaves them alone.Tests
TestFoundBinders:variablesAnotherRowShowsGetNoTrueRow(positive): newaddUpfixture, the A1_09 pattern as a refinement error.variableOnlyExpectedNamesKeepsItsTrueRow(negative): newforwardfixture. The informative∀t truerow and its counterexample stay.staticFieldIsWrittenWithItsClassupdated: theBasket.servedrow goes, and its counterexample value stays.VCBinderSimplificationTest:removesTrueBinderThatALaterRowShows,removesTrueBinderThatIsTheArgumentOfAGhost, andremovesTrueBindersThatAConditionalStateShows(the A1_09 shape).removesTrueBinderOfExpectedVariableThatAnotherRowShows.keepsTrueBinderOfExpectedVariableNoOtherRowMentions(renamed) still keeps the row.skipsInapplicableTrueBinderAndSimplifiesLaterBindernow uses a row that does not mentionx, soxis still the inapplicable binder.mainsources, new tests kept), 6 of the 22 tests in these two classes fail, including both positive tests. The negative test passes on both, as it should.mvn test: 667 tests, 0 failures.Part of #375. The
#freshpath rows are left for #373.🤖 Generated with Claude Code