Skip to content

rust-analyzer --stage=1 mixes incompatible proc_macro server and client #139810

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

Open
m-ou-se opened this issue Apr 14, 2025 · 3 comments
Open

rust-analyzer --stage=1 mixes incompatible proc_macro server and client #139810

m-ou-se opened this issue Apr 14, 2025 · 3 comments
Labels
A-bootstrap-stages Area: issues with the meaning of stage numbers C-bug Category: This is a bug. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-rust-analyzer Relevant to the rust-analyzer team, which will review and decide on the PR/issue.

Comments

@m-ou-se
Copy link
Member

m-ou-se commented Apr 14, 2025

Looks like rust-analyzer built with --stage=1 will use the proc_macro server from the checkout, but the proc_macro client from beta (stage 0). This "works" "fine" as long as we don't make any changes to the proc macro bridge. But it breaks terribly when we make any changes to the proc_macro interface.

To reproduce:

Make some backwards-incomplatible change to the proc macro bridge communication protocol. For example, in library/proc_macro/src/bridge/rpc.rs:

@@ -115,13 +115,14 @@ fn decode(_: &mut Reader<'_>, _: &mut S) -> Self {}
 impl<S> Encode<S> for u8 {
     fn encode(self, w: &mut Writer, _: &mut S) {
         w.push(self);
+        w.push(0);
     }
 }

 impl<S> DecodeMut<'_, '_, S> for u8 {
     fn decode(r: &mut Reader<'_>, _: &mut S) -> Self {
         let x = r[0];
-        *r = &r[1..];
+        *r = &r[2..];
         x
     }
 }

And then run

./x.py test src/tools/rust-analyzer --stage=1

Everything panics.

(With --stage=2, it works fine.)

This is a problem, because we run the r-a tests at --stage=1 as part of CI. This means we currently cannot make any changes to the proc macro bridge (without purposely breaking and disabling tests).

Discovered while trying to merge #139671

@m-ou-se m-ou-se added A-bootstrap-stages Area: issues with the meaning of stage numbers C-bug Category: This is a bug. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-rust-analyzer Relevant to the rust-analyzer team, which will review and decide on the PR/issue. labels Apr 14, 2025
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Apr 14, 2025
@m-ou-se
Copy link
Member Author

m-ou-se commented Apr 14, 2025

cc @rust-lang/rust-analyzer

@m-ou-se
Copy link
Member Author

m-ou-se commented Apr 14, 2025

Perhaps we should include the compiler version or something similar in the proc macro bridge communication protocol. Then we would have detected this much earlier.

@jieyouxu jieyouxu removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Apr 15, 2025
@Veykril
Copy link
Member

Veykril commented Apr 16, 2025

Stage 1 and stage 2 have the same rustc version string I assume right? Which means our version check won't catch it when we load a proc-macro dylib.

Sounds like --stage 1 r-a generally is not usable with proc-macros, so it sounds reasonable to me to not have --stage 1 tested / or specifically not have proc-macro server tests in r-a run in stage 1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-bootstrap-stages Area: issues with the meaning of stage numbers C-bug Category: This is a bug. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-rust-analyzer Relevant to the rust-analyzer team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants