Merge #6745

6745: Some more proc macro cleanups r=jonas-schievink a=jonas-schievink

* Remove `ProcMacroClient::dummy` and just use `Option<ProcMacroClient>` instead
* Remember the type of proc macros (later allows us to reject using an incorrect macro type)
* Prepare a few internals for procedural attribute macros

bors r+

Co-authored-by: Jonas Schievink <jonasschievink@gmail.com>

Cargo.lock

@@ -1187,6 +1187,7 @@ dependencies = [
 name = "proc_macro_api"
 version = "0.0.0"
 dependencies = [
+ "base_db",
  "crossbeam-channel 0.5.0",
  "jod-thread",
  "log",

crates/base_db/src/input.rs

@@ -143,9 +143,17 @@ impl CrateDisplayName {
 #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
 pub struct ProcMacroId(pub u32);
 
+#[derive(Copy, Clone, Eq, PartialEq, Debug)]
+pub enum ProcMacroKind {
+    CustomDerive,
+    FuncLike,
+    Attr,
+}
+
 #[derive(Debug, Clone)]
 pub struct ProcMacro {
     pub name: SmolStr,
+    pub kind: ProcMacroKind,
     pub expander: Arc<dyn TokenExpander>,
 }
 
@@ -198,11 +206,8 @@ impl CrateGraph {
         display_name: Option<CrateDisplayName>,
         cfg_options: CfgOptions,
         env: Env,
-        proc_macro: Vec<(SmolStr, Arc<dyn tt::TokenExpander>)>,
+        proc_macro: Vec<ProcMacro>,
     ) -> CrateId {
-        let proc_macro =
-            proc_macro.into_iter().map(|(name, it)| ProcMacro { name, expander: it }).collect();
-
         let data = CrateData {
             root_file_id: file_id,
             edition,

crates/base_db/src/lib.rs

@@ -14,7 +14,7 @@ pub use crate::{
     change::Change,
     input::{
         CrateData, CrateDisplayName, CrateGraph, CrateId, CrateName, Dependency, Edition, Env,
-        ProcMacroId, SourceRoot, SourceRootId,
+        ProcMacro, ProcMacroId, ProcMacroKind, SourceRoot, SourceRootId,
     },
 };
 pub use salsa;

crates/hir_expand/src/proc_macro.rs

@@ -26,7 +26,6 @@ impl ProcMacroExpander {
 
     pub fn dummy(krate: CrateId) -> Self {
         // FIXME: Should store the name for better errors
-        // FIXME: I think this is the second layer of "dummy" expansion, we should reduce that
         Self { krate, proc_macro_id: None }
     }
 

crates/proc_macro_api/Cargo.toml

@@ -17,3 +17,4 @@ crossbeam-channel = "0.5.0"
 jod-thread = "0.1.1"
 
 tt = { path = "../tt", version = "0.0.0" }
+base_db = { path = "../base_db", version = "0.0.0" }

crates/proc_macro_api/src/lib.rs

@@ -16,6 +16,7 @@ use std::{
     sync::Arc,
 };
 
+use base_db::ProcMacro;
 use tt::{SmolStr, Subtree};
 
 use crate::process::{ProcMacroProcessSrv, ProcMacroProcessThread};
@@ -23,7 +24,7 @@ use crate::process::{ProcMacroProcessSrv, ProcMacroProcessThread};
 pub use rpc::{ExpansionResult, ExpansionTask, ListMacrosResult, ListMacrosTask, ProcMacroKind};
 
 #[derive(Debug, Clone)]
-pub struct ProcMacroProcessExpander {
+struct ProcMacroProcessExpander {
     process: Arc<ProcMacroProcessSrv>,
     dylib_path: PathBuf,
     name: SmolStr,
@@ -42,21 +43,24 @@ impl tt::TokenExpander for ProcMacroProcessExpander {
     fn expand(
         &self,
         subtree: &Subtree,
-        _attr: Option<&Subtree>,
+        attr: Option<&Subtree>,
     ) -> Result<Subtree, tt::ExpansionError> {
-        self.process.custom_derive(&self.dylib_path, subtree, &self.name)
+        let task = ExpansionTask {
+            macro_body: subtree.clone(),
+            macro_name: self.name.to_string(),
+            attributes: attr.cloned(),
+            lib: self.dylib_path.to_path_buf(),
+        };
+
+        let result: ExpansionResult = self.process.send_task(msg::Request::ExpansionMacro(task))?;
+        Ok(result.expansion)
     }
 }
 
-#[derive(Debug)]
-enum ProcMacroClientKind {
-    Process { process: Arc<ProcMacroProcessSrv>, thread: ProcMacroProcessThread },
-    Dummy,
-}
-
 #[derive(Debug)]
 pub struct ProcMacroClient {
-    kind: ProcMacroClientKind,
+    process: Arc<ProcMacroProcessSrv>,
+    thread: ProcMacroProcessThread,
 }
 
 impl ProcMacroClient {
@@ -65,47 +69,35 @@ impl ProcMacroClient {
         args: impl IntoIterator<Item = impl AsRef<OsStr>>,
     ) -> io::Result<ProcMacroClient> {
         let (thread, process) = ProcMacroProcessSrv::run(process_path, args)?;
-        Ok(ProcMacroClient {
+        Ok(ProcMacroClient { process: Arc::new(process), thread })
-            kind: ProcMacroClientKind::Process { process: Arc::new(process), thread },
-        })
     }
 
-    pub fn dummy() -> ProcMacroClient {
+    pub fn by_dylib_path(&self, dylib_path: &Path) -> Vec<ProcMacro> {
-        ProcMacroClient { kind: ProcMacroClientKind::Dummy }
+        let macros = match self.process.find_proc_macros(dylib_path) {
-    }
+            Err(err) => {
-
+                eprintln!("Failed to find proc macros. Error: {:#?}", err);
-    pub fn by_dylib_path(&self, dylib_path: &Path) -> Vec<(SmolStr, Arc<dyn tt::TokenExpander>)> {
+                return vec![];
-        match &self.kind {
-            ProcMacroClientKind::Dummy => vec![],
-            ProcMacroClientKind::Process { process, .. } => {
-                let macros = match process.find_proc_macros(dylib_path) {
-                    Err(err) => {
-                        eprintln!("Failed to find proc macros. Error: {:#?}", err);
-                        return vec![];
-                    }
-                    Ok(macros) => macros,
-                };
-
-                macros
-                    .into_iter()
-                    .filter_map(|(name, kind)| {
-                        match kind {
-                            ProcMacroKind::CustomDerive | ProcMacroKind::FuncLike => {
-                                let name = SmolStr::new(&name);
-                                let expander: Arc<dyn tt::TokenExpander> =
-                                    Arc::new(ProcMacroProcessExpander {
-                                        process: process.clone(),
-                                        name: name.clone(),
-                                        dylib_path: dylib_path.into(),
-                                    });
-                                Some((name, expander))
-                            }
-                            // FIXME: Attribute macro are currently unsupported.
-                            ProcMacroKind::Attr => None,
-                        }
-                    })
-                    .collect()
             }
-        }
+            Ok(macros) => macros,
+        };
+
+        macros
+            .into_iter()
+            .map(|(name, kind)| {
+                let name = SmolStr::new(&name);
+                let kind = match kind {
+                    ProcMacroKind::CustomDerive => base_db::ProcMacroKind::CustomDerive,
+                    ProcMacroKind::FuncLike => base_db::ProcMacroKind::FuncLike,
+                    ProcMacroKind::Attr => base_db::ProcMacroKind::Attr,
+                };
+                let expander: Arc<dyn tt::TokenExpander> = Arc::new(ProcMacroProcessExpander {
+                    process: self.process.clone(),
+                    name: name.clone(),
+                    dylib_path: dylib_path.into(),
+                });
+
+                ProcMacro { name, kind, expander }
+            })
+            .collect()
     }
 }

crates/proc_macro_api/src/process.rs

@@ -10,11 +10,10 @@ use std::{
 };
 
 use crossbeam_channel::{bounded, Receiver, Sender};
-use tt::Subtree;
 
 use crate::{
     msg::{ErrorCode, Message, Request, Response, ResponseError},
-    rpc::{ExpansionResult, ExpansionTask, ListMacrosResult, ListMacrosTask, ProcMacroKind},
+    rpc::{ListMacrosResult, ListMacrosTask, ProcMacroKind},
 };
 
 #[derive(Debug, Default)]
@@ -58,23 +57,6 @@ impl ProcMacroProcessSrv {
         Ok(result.macros)
     }
 
-    pub(crate) fn custom_derive(
-        &self,
-        dylib_path: &Path,
-        subtree: &Subtree,
-        derive_name: &str,
-    ) -> Result<Subtree, tt::ExpansionError> {
-        let task = ExpansionTask {
-            macro_body: subtree.clone(),
-            macro_name: derive_name.to_string(),
-            attributes: None,
-            lib: dylib_path.to_path_buf(),
-        };
-
-        let result: ExpansionResult = self.send_task(Request::ExpansionMacro(task))?;
-        Ok(result.expansion)
-    }
-
     pub(crate) fn send_task<R>(&self, req: Request) -> Result<R, tt::ExpansionError>
     where
         R: TryFrom<Response, Error = &'static str>,

crates/proc_macro_api/src/rpc.rs

@@ -19,7 +19,7 @@ pub struct ListMacrosTask {
     pub lib: PathBuf,
 }
 
-#[derive(Clone, Eq, PartialEq, Debug, Serialize, Deserialize)]
+#[derive(Copy, Clone, Eq, PartialEq, Debug, Serialize, Deserialize)]
 pub enum ProcMacroKind {
     CustomDerive,
     FuncLike,

crates/project_model/src/workspace.rs

@@ -2,10 +2,14 @@
 //! metadata` or `rust-project.json`) into representation stored in the salsa
 //! database -- `CrateGraph`.
 
-use std::{fmt, fs, path::Component, process::Command};
+use std::{
+    fmt, fs,
+    path::{Component, Path},
+    process::Command,
+};
 
 use anyhow::{Context, Result};
-use base_db::{CrateDisplayName, CrateGraph, CrateId, CrateName, Edition, Env, FileId};
+use base_db::{CrateDisplayName, CrateGraph, CrateId, CrateName, Edition, Env, FileId, ProcMacro};
 use cfg::CfgOptions;
 use paths::{AbsPath, AbsPathBuf};
 use proc_macro_api::ProcMacroClient;
@@ -194,15 +198,20 @@ impl ProjectWorkspace {
     pub fn to_crate_graph(
         &self,
         target: Option<&str>,
-        proc_macro_client: &ProcMacroClient,
+        proc_macro_client: Option<&ProcMacroClient>,
         load: &mut dyn FnMut(&AbsPath) -> Option<FileId>,
     ) -> CrateGraph {
+        let proc_macro_loader = |path: &Path| match proc_macro_client {
+            Some(client) => client.by_dylib_path(path),
+            None => Vec::new(),
+        };
+
         let mut crate_graph = match self {
             ProjectWorkspace::Json { project, sysroot } => {
-                project_json_to_crate_graph(target, proc_macro_client, load, project, sysroot)
+                project_json_to_crate_graph(target, &proc_macro_loader, load, project, sysroot)
             }
             ProjectWorkspace::Cargo { cargo, sysroot, rustc } => {
-                cargo_to_crate_graph(target, proc_macro_client, load, cargo, sysroot, rustc)
+                cargo_to_crate_graph(target, &proc_macro_loader, load, cargo, sysroot, rustc)
             }
         };
         if crate_graph.patch_cfg_if() {
@@ -216,7 +225,7 @@ impl ProjectWorkspace {
 
 fn project_json_to_crate_graph(
     target: Option<&str>,
-    proc_macro_client: &ProcMacroClient,
+    proc_macro_loader: &dyn Fn(&Path) -> Vec<ProcMacro>,
     load: &mut dyn FnMut(&AbsPath) -> Option<FileId>,
     project: &ProjectJson,
     sysroot: &Option<Sysroot>,
@@ -236,8 +245,7 @@ fn project_json_to_crate_graph(
         })
         .map(|(crate_id, krate, file_id)| {
             let env = krate.env.clone().into_iter().collect();
-            let proc_macro =
+            let proc_macro = krate.proc_macro_dylib_path.clone().map(|it| proc_macro_loader(&it));
-                krate.proc_macro_dylib_path.clone().map(|it| proc_macro_client.by_dylib_path(&it));
 
             let target = krate.target.as_deref().or(target);
             let target_cfgs =
@@ -279,7 +287,7 @@ fn project_json_to_crate_graph(
 
 fn cargo_to_crate_graph(
     target: Option<&str>,
-    proc_macro_client: &ProcMacroClient,
+    proc_macro_loader: &dyn Fn(&Path) -> Vec<ProcMacro>,
     load: &mut dyn FnMut(&AbsPath) -> Option<FileId>,
     cargo: &CargoWorkspace,
     sysroot: &Sysroot,
@@ -309,7 +317,7 @@ fn cargo_to_crate_graph(
                     &mut crate_graph,
                     &cargo[pkg],
                     &cfg_options,
-                    proc_macro_client,
+                    proc_macro_loader,
                     file_id,
                 );
                 if cargo[tgt].kind == TargetKind::Lib {
@@ -385,7 +393,7 @@ fn cargo_to_crate_graph(
                         &mut crate_graph,
                         &rustc_workspace[pkg],
                         &cfg_options,
-                        proc_macro_client,
+                        proc_macro_loader,
                         file_id,
                     );
                     pkg_to_lib_crate.insert(pkg, crate_id);
@@ -433,7 +441,7 @@ fn add_target_crate_root(
     crate_graph: &mut CrateGraph,
     pkg: &cargo_workspace::PackageData,
     cfg_options: &CfgOptions,
-    proc_macro_client: &ProcMacroClient,
+    proc_macro_loader: &dyn Fn(&Path) -> Vec<ProcMacro>,
     file_id: FileId,
 ) -> CrateId {
     let edition = pkg.edition;
@@ -452,11 +460,8 @@ fn add_target_crate_root(
             env.set("OUT_DIR", out_dir);
         }
     }
-    let proc_macro = pkg
+    let proc_macro =
-        .proc_macro_dylib_path
+        pkg.proc_macro_dylib_path.as_ref().map(|it| proc_macro_loader(&it)).unwrap_or_default();
-        .as_ref()
-        .map(|it| proc_macro_client.by_dylib_path(&it))
-        .unwrap_or_default();
 
     let display_name = CrateDisplayName::from_canonical_name(pkg.name.clone());
     let crate_id = crate_graph.add_crate_root(

crates/rust-analyzer/src/cli/load_cargo.rs

@@ -33,12 +33,12 @@ pub fn load_cargo(
 
     let proc_macro_client = if with_proc_macro {
         let path = std::env::current_exe()?;
-        ProcMacroClient::extern_process(path, &["proc-macro"]).unwrap()
+        Some(ProcMacroClient::extern_process(path, &["proc-macro"]).unwrap())
     } else {
-        ProcMacroClient::dummy()
+        None
     };
 
-    let crate_graph = ws.to_crate_graph(None, &proc_macro_client, &mut |path: &AbsPath| {
+    let crate_graph = ws.to_crate_graph(None, proc_macro_client.as_ref(), &mut |path: &AbsPath| {
         let contents = loader.load_sync(path);
         let path = vfs::VfsPath::from(path.to_path_buf());
         vfs.set_file_contents(path.clone(), contents);

crates/rust-analyzer/src/global_state.rs

@@ -75,7 +75,7 @@ pub(crate) struct GlobalState {
     pub(crate) shutdown_requested: bool,
     pub(crate) status: Status,
     pub(crate) source_root_config: SourceRootConfig,
-    pub(crate) proc_macro_client: ProcMacroClient,
+    pub(crate) proc_macro_client: Option<ProcMacroClient>,
     pub(crate) workspaces: Arc<Vec<ProjectWorkspace>>,
     latest_requests: Arc<RwLock<LatestRequests>>,
 }
@@ -127,7 +127,7 @@ impl GlobalState {
             shutdown_requested: false,
             status: Status::default(),
             source_root_config: SourceRootConfig::default(),
-            proc_macro_client: ProcMacroClient::dummy(),
+            proc_macro_client: None,
             workspaces: Arc::new(Vec::new()),
             latest_requests: Default::default(),
         }

crates/rust-analyzer/src/reload.rs

@@ -171,16 +171,16 @@ impl GlobalState {
         let project_folders = ProjectFolders::new(&workspaces);
 
         self.proc_macro_client = match &self.config.proc_macro_srv {
-            None => ProcMacroClient::dummy(),
+            None => None,
             Some((path, args)) => match ProcMacroClient::extern_process(path.into(), args) {
-                Ok(it) => it,
+                Ok(it) => Some(it),
                 Err(err) => {
                     log::error!(
                         "Failed to run proc_macro_srv from path {}, error: {:?}",
                         path.display(),
                         err
                     );
-                    ProcMacroClient::dummy()
+                    None
                 }
             },
         };
@@ -212,7 +212,7 @@ impl GlobalState {
             for ws in workspaces.iter() {
                 crate_graph.extend(ws.to_crate_graph(
                     self.config.cargo.target.as_deref(),
-                    &self.proc_macro_client,
+                    self.proc_macro_client.as_ref(),
                     &mut load,
                 ));
             }