Skip to content

clang: Disable source order sorting for now. #2558

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

Merged
merged 6 commits into from
Jun 15, 2023

Conversation

emilio
Copy link
Contributor

@emilio emilio commented Jun 15, 2023

This disables (hopefully temporarily, if this is eventually going to be needed)
source order sorting, for causing correctness regressions like #2556, for which
I added a test.

emilio added 2 commits June 15, 2023 13:34
This doesn't change behavior but is easier to debug and reason about
(because you have the relevant cursors there).
This disables (hopefully temporarily) source order sorting, for causing
correctness regressions like rust-lang#2556.

Fixes rust-lang#2556.
@emilio emilio force-pushed the disable-sorting branch from eebb3eb to 9a6a078 Compare June 15, 2023 13:25
@reitermarkus
Copy link
Contributor

Thanks, I will have a look at the new test.

@bors-servo
Copy link

☔ The latest upstream changes (presumably 2034a0f) made this pull request unmergeable. Please resolve the merge conflicts.

@pvdrz pvdrz merged commit 6020f89 into rust-lang:master Jun 15, 2023
@pvdrz
Copy link
Contributor

pvdrz commented Jun 15, 2023

@emilio now that this is merged, is there any way you can check if there's any other regressions from your side before releasing a new version?

pvdrz added a commit that referenced this pull request Jun 15, 2023
* clang: Clean up source order sorting.

This doesn't change behavior but is easier to debug and reason about
(because you have the relevant cursors there).

* clang: Disable source order sorting for now.

This disables (hopefully temporarily) source order sorting, for causing
correctness regressions like #2556.

Fixes #2556.

* tests: Add a test for #2556

* Remove merge artifact

* Update clang.rs

---------

Co-authored-by: Christian Poveda Ruiz <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants