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

chore: fix completions in #[salsa::tracked] functions #653

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

davidbarsky
Copy link
Contributor

This change allows rust-analyzer (and, I assume RustRover) to provide accurate completions inside of a #[salsa::tracked] function. Previously, I'd comment out the #[salsa::tracked] annotation when writing a tracked function, but this PR removes the need for that workaround.

As @Veykril put it, the core issue is that rust-analyzer's mbe crate is not very error-resilient: we no longer pick the correct match arm of the macro_rules macro because the input doesn't match—mbe should pick the closest match instead of giving up, but until mbe supports that, this PR papers over a rust-analyzer weakness 🌟 it makes the macro more IDE friendly 🌟 !

(Credit goes to Lukas for the tip, explanation, and joke!)

Copy link

netlify bot commented Jan 7, 2025

Deploy Preview for salsa-rs canceled.

Name Link
🔨 Latest commit 5f14bbd
🔍 Latest deploy log https://app.netlify.com/sites/salsa-rs/deploys/677d640387f3260008081d10

Copy link

codspeed-hq bot commented Jan 7, 2025

CodSpeed Performance Report

Merging #653 will not alter performance

Comparing davidbarsky:davidbarsky/fix-completions-in-salsa-tracked-fns (5f14bbd) with master (639fd29)

Summary

✅ 9 untouched benchmarks

Copy link
Member

@Veykril Veykril left a comment

Choose a reason for hiding this comment

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

Clearly I should not be the one to approve this but if we do this we ought to give the same treatment to setup_method_body

@davidbarsky
Copy link
Contributor Author

Getting methods to work is a bit trickier: the "wrap the inner fn in a block" trick doesn't seem to be sufficient for methods, and I'm not enough of an expert on macros to figure that out. Can we make that a followup?

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