-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Introduce ra_proc_macro #3727
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
Introduce ra_proc_macro #3727
Conversation
|
cc @kiljacken for the dylib path reading part :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like to pay some attention to minimizing the crate deps here.
Ideally, the ra_db should not depend only any proc-macro specfic, and just export a trait. It should operate on tt::TokenStream, so a dep on ra_tt is required.
Additionally, Ideally proc_macros and mbe do not depend on each other, and use solely tt (which does not depen on ra_syntax). I think we should also make use that proc_macros dont (transitivellly) depend on ra_syntax.
Not sure if the above plan is feasible, but i'd love to try
crates/ra_db/Cargo.toml
Outdated
| ra_syntax = { path = "../ra_syntax" } | ||
| ra_cfg = { path = "../ra_cfg" } | ||
| ra_prof = { path = "../ra_prof" } | ||
| ra_proc_macro = { path = "../ra_proc_macro" } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, I wonder if we can minimize the interface here...
Can we use something like this instead?
struct CrateData {
proc_macros: FxHashMap<SmolStr, Arc<dyn Fn(tt::TokenTree) -> tt::TokenTree>>
}
Ie, define a really minimal proc macro interface directly in the ra_db crate, where the minimal interface is ideally an dyn Fn(), or maybe a named trait.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed to :
#[derive(Debug, Clone)]
pub struct ProcMacro {
pub name: SmolStr,
pub expander: Arc<dyn TokenExpander>,
}
impl Eq for ProcMacro {}
impl PartialEq for ProcMacro { ... }
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct CrateData {
...
pub proc_macro: Vec<ProcMacro>,
}|
Also, thanks @edwin0cheng for splitting changes which are small diff-wise, but are large architecturally, into separate PRs! This is massively helpful! |
In other words, if we export a trait in |
|
Good question! Yeah, I think it would also be good if I think bridging them in project-model might work, but, if the expansion trait is really minimal and only depends on tt, we can just put it in the |
|
I think it is plausible, I could imagine it looks like following: enum ExpandError {
...
}
trait TokenExpander: Debug + Send + Sync + RefUnwindSafe {
fn exander(&self, subtree: &Subtree, attrs: Option<&Subtree>) -> Result<Subtree, ExpandError>;
}The trait bound here is needed in |
|
Cargo interacting code looks reasonable to me 👍 Good job, this is some really cool work! |
e9ca593 to
db162df
Compare
|
@matklad, I removed the dependency for mbe and refactoring a bit for salsa trait constraints. You could review it again. 👍 |
|
bors r+ |
Build succeeded |
Rustup To unblock rust-lang/miri#3688
This PR implemented:
OUTDIRis obtained.ra_proc_macroand implement the foot-work for reading result from external proc-macro expander.ProcMacroClient, which will be responsible to the client side communication to the External process.