red-knot[salsa part 2]: Setup semantic DB and Jar #11837
Conversation
|
MichaReiser
force-pushed
the
salsa-2-semantic-jar
branch
2 times, most recently
from
2138b7b
to
b81e1f3
Compare
MichaReiser
force-pushed
the
salsa-2-semantic-jar
branch
from
b81e1f3
to
b299766
Compare
There was a problem hiding this comment.
I still have some concerns about having this code just mixed in directly with the rest of the modules in ruff_python_semantic and using only features to distinguish what is red-knot, but otherwise this PR looks fine, and I assume we will revisit that structure as follow-up, not on this PR, for rebase-conflict reasons.
| db.file_system_mut().write_file(path, "x = 20".to_string()); | ||
| file.set_revision(&mut db).to(FileTime::now().into()); |
There was a problem hiding this comment.
It's surprising to me that we have to manually update the revision here; should write_file do that itself?
There was a problem hiding this comment.
The FileSystem that I have in mind should be independent of the vfs and salsa db. It's just an abstraction over std::fs. What I have in mind to avoid the set_revision call here is that we have an apply_changes method where we pass all file changes (adds, remove, modified) that updates the vfs state. It's probably the FileSystem's or the host's responsibility to collect all made changes.
There was a problem hiding this comment.
That makes sense, and would address my concern: I don't want to expose/use APIs that make it easy to change a file's contents but forget to bump its revision.
I think this also gets back to what I find confusing about the Vfs and FileSystem structure, which is that there is no single entity which really encapsulates our virtual file system. Where does this apply_changes method live? I guess it is just a free function that takes a Db and some changes to apply?
Probably this is just something I have to get used to with Salsa, that the Db owns all the state, so most API that we use for managing state is just a function that takes a Db.
There was a problem hiding this comment.
Where does this apply_changes method live? I guess it is just a free function that takes a Db and some changes to apply?
It would probably be a free standing function that takes &mut Db. And yes, the fact that it is a free standing function takes some time to get used to. An alternative is to make it a method in Vfs, but I think that won't work because it would require holding a mutable reference to Db and a read only reference to FileSystem.
Probably this is just something I have to get used to with Salsa, that the Db owns all the state, so most API that we use for managing state is just a function that takes a Db.
yes, that takes some time to get used to. It's also something I'm still figuring out
Co-authored-by: Alex Waygood <[email protected]>
I think it's easiest if we discuss this in person in our planning meeting on Friday. |
Summary
This PR sets up an empty Salsa
JarandDb. This is mostly boring template code but I thought it's worth separating it form an actual interesting PR ;)Test Plan
cargo build