-
Notifications
You must be signed in to change notification settings - Fork 985
Fix use <2024 identifier sorting transitivity
#6786
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
The head ref may contain hidden characters: "\u{1F3F3}\uFE0F\u200D\u26A7\uFE0F-sorting"
Conversation
|
There's also a chance this might not be a breaking change at all, but I think this would be an acceptable one if it were since there's no other way to resolve the panic that I can think of. Running our Diff Check shows that the current |
|
After the tests are updated we can get this merged. @rustbot author |
The sorting tries to create a lowercase>Capital>SCREAMING relation, but does this in a way that causes non-transitive behavior for identifiers starting with `_`. These identifiers are neither Capital nor SCREAMING so they are always compared via lexicographic ASCII matching. This causes a problem, because `A < _ < a` lexicographically in ASCII. But `A > a` with our special case rules! When doing these manual comparisons, hitting these issues is very easy. To fix this, we can simply create a sorting key that mirrors our desired order and sort that normally. This way, it's guaranteed to have all the necessary properties. By *always* sorting uppercase identifiers first, we ensure that `A > _`.
37baf61 to
68eb180
Compare
|
Added the tests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thank you for helping out with this fix!
The sorting tries to create a lowercase>Capital>SCREAMING relation, but does this in a way that causes non-transitive behavior for identifiers starting with
_. These identifiers are neither Capital nor SCREAMING so they are always compared via lexicographic ASCII matching.This causes a problem, because
A < _ < alexicographically in ASCII. ButA > awith our special case rules!When doing these manual comparisons, hitting these issues is very easy. To fix this, we can simply create a sorting key that mirrors our desired order and sort that normally. This way, it's guaranteed to have all the necessary properties.
By always sorting uppercase identifiers first, we ensure that
A > _.fixes #6668
This may cause formatting changes if there are identifiers starting with
_in use items, but that breakage is necessary as the previous formatting was completely arbitrary and subject to change by the standard library (and prone to crash rustfmt alltogether).