Fix regression in cyclic constraint handling #16514
Merged
Conversation
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
smarter
force-pushed
the
fix-validBoundsFor
branch
from
16eefed to
bdf98c5
Compare
|
The CI fails because the test case fails -Ycheck:typer, but it was already broken like that in previous releases. I'll investigate and/or find a simpler test case. |
I agree, this would make it safer. |
This regressed in 50eb0e9 when `current.ensureNonCyclic` was incorrectly replaced by `validBoundsFor` which operates on `this`, not `current`. This isn't the first time we make this error (cf a8641c5), maybe we should refactor OrderingConstraint so that operations on `current` are done in the companion object where `this` isn't accessible. Fixes scala#16471. Note that the test case from this issue couldn't be added because it fails `-Ycheck:typer`, but this was also the case before the regression. This is now tracked by scala#16524.
smarter
force-pushed
the
fix-validBoundsFor
branch
from
bdf98c5 to
c1f77a5
Compare
|
I've replaced the test case by a unit test and opened #16524 to keep track of the Ycheck failure. |
odersky
approved these changes
Dec 14, 2022
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.
This regressed in 50eb0e9 when
current.ensureNonCyclicwas incorrectly replaced byvalidBoundsForwhich operates onthis, notcurrent.This isn't the first time we make this error (cf
a8641c5), maybe we should refactor OrderingConstraint so that operations on
currentare done in the companion object wherethisisn't accessible.Fixes #16471. Note that the test case from this issue couldn't be added because it fails
-Ycheck:typer, but this was also the case before the regression. This is now tracked by #16524.