Skip to content

internal: start porting VFS to Salsa #20102

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
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

davidbarsky
Copy link
Contributor

This is a small PR that starts porting the VFS over to Salsa (For context, #18948). In this PR, I'm starting by converting FileId to a salsa::input. The harder part—which is not done in this PR—is removing the vfs field on GlobalState and refactoring the read/write lock that is currently separate from the Salsa database.

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 25, 2025
@davidbarsky davidbarsky force-pushed the davidbarsky/port-file-id-to-salsa branch from ed0aedc to 36adc7b Compare June 25, 2025 17:13
@davidbarsky
Copy link
Contributor Author

I think that if we want to simplify the VFS, it will need to take a dependency on Salsa. This means that the proc-macro-server-api will also need a Salsa dependency.

@davidbarsky davidbarsky force-pushed the davidbarsky/port-file-id-to-salsa branch from 36adc7b to ce3239a Compare June 25, 2025 17:22
@ChayimFriedman2
Copy link
Contributor

Why does the proc macro server need the VFS?

@davidbarsky
Copy link
Contributor Author

The dependency graph is proc-macro-srv-cli -> proc-macro-api -> span -> vfs. Span declares EditionedFileId, which can be constructed from a vfs::FileId.

@ChayimFriedman2
Copy link
Contributor

Then we can continue as we did (do now? I don't remember), with span using a cfg and defining FileId as either a salsa::input or a wrapper around u32.

@davidbarsky
Copy link
Contributor Author

Then we can continue as we did (do now? I don't remember), with span using a cfg and defining FileId as either a salsa::input or a wrapper around u32.

Yeah, we do a #[cfg(feature = "salsa")]/#[cfg(not(feature = "salsa"))] dance with SyntaxContext in crates/span/src/hygiene.rs. I think that we should probably do something similar to vfs::FileId, but I'll hold off on implementing that until I move the vfs over use a Ruff-style Files, as that might design implications.

(The implementation for a ruff-style Vfs will be in a separate PR.)

@Veykril
Copy link
Member

Veykril commented Jun 27, 2025

We should probably reconsider exposing span to proc-macro-srv in general, once we have bidirectional messages there, the server knowing the format is merely a performance thing (as we could skip going back and forth for some semantic requests).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants