-
Notifications
You must be signed in to change notification settings - Fork 13.9k
fix panic when rustc tries to reduce intermediate filenames len with utf8 #148018
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
Conversation
|
not sure of the process for urgent beta fixes. I uploaded two versions of this patch, this one against master and #148019 against beta |
Thanks @tardyp! Patches for regressions can either be fixed on the master branch (like this one) or directly on the beta branch. In practice I think there is no big difference; fixing stuff on master obviously needs a backport patch to beta (which you have already submitted). In both case they are nominated for backport approval (see docs) and discussed during the weekly T-compiler triage meeting (in ~4 hours from now 🙂 ) |
|
r? compiler (wont get to this soon) |
|
I've nominates this one for backport and will close the other PR to follow our standard process of nominating PRs against master and then letting the release team cherry pick approved backports. Thank you for the quick fix! |
compiler/rustc_session/src/config.rs
Outdated
| .char_indices() | ||
| .find(|(i, _)| *i >= stripped_bytes) | ||
| .map(|(i, _)| i) | ||
| .unwrap_or(filename.len()); |
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.
I believe that the 5 lines above are equivalent to:
let split_at = filename.ceil_char_boundary(stripped_bytes);|
r=me after the comment above is resolved |
|
Reminder, once the PR becomes ready for a review, use |
96868ac to
a968930
Compare
|
@rustbot ready |
|
@thanks @JonathanBrouwer for the thoughtful suggestion on this recently out of nightly api. Note that I took the opportunity of this rework to replace the boring "28_找出字符串中第一个匹配项的下标" by more entertaining "ⴻⵎⵎⴻⵎ_ⴷⵉⵎⴰ_ⵖⴻⴼ_ⵢⵉⵙⴻⴽⴽⵉⵍⴻⵏ_ⵏ_ⵡⴰⵟⴰⵙ_ⵏ_ⵢⵉⴱⵢⵜⴻⵏ" which more adequately watches for multi byte character issues. |
|
Thank you so much for the quick fix! |
|
@bors try cancel |
|
Try build cancelled. Cancelled workflows: |
|
@bors try jobs=x86_64-mingw-,i686-msvc- |
This comment has been minimized.
This comment has been minimized.
fix panic when rustc tries to reduce intermediate filenames len with utf8 try-job: x86_64-mingw-* try-job: i686-msvc-*
006611b to
d73e0b8
Compare
|
@JonathanBrouwer I forgot the import in my commit from 5min ago. Forced pushed a fix after verifying it actually passes on my system. Will Need another try command.. 🙏 |
|
@bors try cancel |
|
Try build cancelled. Cancelled workflows: |
…th multi byte chars The issue cannot be reproduced with the former testcase of creating external crates because rust refuses to use "external crate 28_找出字符串中第一个匹配项的下标" because it is not a valid indentifier (starts with number, and contain non ascii chars) But still using 28_找出字符串中第一个匹配项的下标.rs as a filename is accepted by previous rustc releases So we consider it valid, and add an integration test for it to catch any regression on other code related to non ascii filenames.
d73e0b8 to
c6acffe
Compare
This comment has been minimized.
This comment has been minimized.
fix panic when rustc tries to reduce intermediate filenames len with utf8 try-job: x86_64-mingw-* try-job: i686-msvc-*
|
I'll be in vacation tomorrow for a week, mostly afk, so I wouldn't be able to be so reactive. hopefully we can finish this today! |
|
Let's boost this so we can get to the beta backport too... @bors r=JonathanBrouwer p=5 |
|
☀️ Test successful - checks-actions |
What is this?This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.Comparing ab92564 (parent) -> 2aaa62b (this PR) Test differencesShow 6 test diffsStage 1
Stage 2
Additionally, 2 doctest diffs were found. These are ignored, as they are noisy. Job group index
Test dashboardRun cargo run --manifest-path src/ci/citool/Cargo.toml -- \
test-dashboard 2aaa62b89d22b570e560731b03e3d2d6f5c3bbce --output-dir test-dashboardAnd then open Job duration changes
How to interpret the job duration changes?Job durations can vary a lot, based on the actual runner instance |
The issue cannot be reproduced with the former testcase of creating external crates because rust refuses to use "external crate 28_找出字符串中第一个匹配项的下标" because it is not a valid indentifier (starts with number, and contain non ascii chars)
But still using 28_找出字符串中第一个匹配项的下标.rs as a filename is accepted by previous rustc releases So we consider it valid, and add an integration test for it to catch any regression on other code related to non ascii filenames.
Fix #147975