Skip to content
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

Update salsa #1015

Merged
merged 14 commits into from
Jul 4, 2024
Merged

Update salsa #1015

merged 14 commits into from
Jul 4, 2024

Conversation

Y-Nak
Copy link
Member

@Y-Nak Y-Nak commented Jun 26, 2024

This PR updates salsa to salsa 3.0. The major changes are

  1. Tracked structs and interned structs are required to have 'db lifetime.
  2. The actual Id(or field) of tracked/interned structs is a pointer to a data location now. This means we can't prefill the keyword in salsa 3.0(I simply removed the 'define_keywordmacros. I just removed thefe-macro` crate.
  3. We need to implement the salsa::update::Update trait to store a type in another tracked type. In normal cases, we can use salsa::Update derive macro. The only types that I implemented the trait manually are Partial and IndexSet/IndexMap.

Please refer to salsa-rs/salsa#490 for more details.

@micahscopes I decided to leave the lsp crate as is, so it can't be compiled right now. Also, I temporarily excluded the language server crate from the workspace in dce1e09 to build/test the compiler itself. If you have any trouble migrating to salsa 3.0, please let me know.

@g-r-a-n-t This change would affect your driver implementation as well. So please take a quick look at the new driver implementation in this PR.

@Y-Nak Y-Nak force-pushed the update-salsa branch 2 times, most recently from 58e5a22 to 59b3a79 Compare June 27, 2024 12:32
Copy link
Collaborator

@sbillig sbillig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍
That's a whole lotta lifetime annotations.

@sbillig sbillig merged commit bd7343c into ethereum:fe-v2 Jul 4, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants