-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Move retokenize hack to save_analysis #76057
Conversation
r? @oli-obk (rust_highfive has picked a reviewer for you, use r? to override) |
cc @Xanewok Note that if I change |
|
||
let src = if let Some(src) = &source_file.src { | ||
Lrc::clone(&src) | ||
} else if let Some(src) = source_file.external_src.borrow().get_source() { |
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.
Why was the external_src
logic removed?
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 assumed from the comment that it was only needed for retokenize
. But let me check blame to see if the two were actually added simultaneously.
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.
yup, both ensure_source_file_present and this branch were added in #70172.
I think it should be fine to remove thouse now, as we no longer have retokenize. It might be the case that some other code-path now relies on this path, but this seems unlikely (StringReader is not really used outside of librustc_parse), and, in any case, lexing external files which might or might not be there feels just weird.
And the RLS test for this is ignored: https://github.com/rust-lang/rls/blob/48ef96dd00b90d950a122ca180923aba77efaf74/tests/client.rs#L1134-L1136 |
@bors r+ |
📌 Commit 6621895 has been approved by |
Rollup of 14 pull requests Successful merges: - rust-lang#75832 (Move to intra-doc links for wasi/ext/fs.rs, os_str_bytes.rs…) - rust-lang#75852 (Switch to intra-doc links in `core::hash`) - rust-lang#75874 (Shorten liballoc doc intra link while readable) - rust-lang#75881 (Expand rustdoc theme chooser x padding) - rust-lang#75885 (Fix another clashing_extern_declarations false positive.) - rust-lang#75892 (Fix typo in TLS Model in Unstable Book) - rust-lang#75910 (Add test for issue rust-lang#27130) - rust-lang#75917 (Move to intra doc links for core::ptr::non_null) - rust-lang#75975 (Allow --bess ing expect-tests in tools) - rust-lang#75990 (Add __fastfail for Windows on arm/aarch64) - rust-lang#76015 (Fix loading pretty-printers in rust-lldb script) - rust-lang#76022 (Clean up rustdoc front-end source code) - rust-lang#76029 (Move to intra-doc links for library/core/src/sync/atomic.rs) - rust-lang#76057 (Move retokenize hack to save_analysis) Failed merges: r? @ghost
closes #76046