-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
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
@@ -15,4 +15,5 @@ rustc-hash = "1.1.0" | |||
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:
OUTDIR
is obtained.ra_proc_macro
and 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.