Proc Macro Expansion with a `rust-project.json` Regressed with #14405

[davidbarsky]

rust-analyzer version: rust-analyzer version: 0.0.0 (f85fc28 2023-03-26) (I'm building from source.)

rustc version: (eg. output of rustc -V): rustc 1.68.1 (8460ca823 2023-03-20)

relevant settings: I'm using this with a rust-project.json sourced from Buck. I can't provide a minimal reproduction at the moment.

Hi! This is not particularly urgent issue, but I've noticed that procedural macro expansion has regressed when used with a rust-project.json since #14405 (this issue does not exist with #14404). This manifests with the error:

> `proc macro `Serialize` not expanded: No proc-macros present for crate`

However, rust-analyzer still correctly identifies the source crate, _serde_derive_1.0.152.

I haven't fully tracked down the issue, but I've noticed that in hir-def/src/nameres/collector.rs the CrateId for serde_derive in db.crate_graph() is 27, but in db.proc_macros(), serde_derive has a CrateId of 16. I'll dig into this more tomorrow.

[Veykril]

Ah, I know why I think. We only set the proc-macros if build scripts have run, otherwise we overwrite it for a better diagnostic which obviously causes problems in case of rust-project.json which usually skips the build script step. The id mismatch sounds weird though ...

[Veykril]

Okay there seem to be two bugs here, the id mismatch (which conveniently has a difference of 11 which is exactly the number of sysroot crates r-a knows about :) ) and us not loading proc-macros when build-scripts don't run.

[davidbarsky]

which conveniently has a difference of 11 which is exactly the number of sysroot crates r-a knows about :)

Yup! I just saw the 11 crate difference when debugging in GlobalWorkspaces. The crate_graph has 11 more crates at the beginning, all of which sysroot-related crates!

[Veykril]

Okay I found the id mismatch bug I think, will push out a PR for that

[Veykril]

#14419 would be great if you could check that commit. ( I still think the other issue will pop up now though, making it so we don't load proc-macros)

[davidbarsky]

#14419 would be great if you could check that commit.

It works!

I still think the other issue will pop up now though, making it so we don't load proc-macros.

Oh, I think that explains why when I add a new workspace via the temporary command, the newly added workspace doesn't have proc macros. It's always subsequent workspaces that lack proc macros.

[Veykril]

The relevant part there is here

rust-analyzer/crates/rust-analyzer/src/reload.rs

Lines 450 to 463 in b99d5eb

	if same_workspaces {
	if self.config.expand_proc_macros() {
	self.fetch_proc_macros_queue.request_op(cause, proc_macro_paths);
	}
	} else {
	// Set up errors for proc-macros upfront that we haven't run build scripts yet
	let mut proc_macros = FxHashMap::default();
	for paths in proc_macro_paths {
	proc_macros.extend(paths.into_iter().map(move |(crate_id, _)| {
	(crate_id, Err("crate has not yet been build".to_owned()))
	}));
	}
	change.set_proc_macros(proc_macros);
	}

Note that we only call self.fetch_proc_macros_queue.request_op(cause, proc_macro_paths); in the same_workspaces branch which only happens when switch_workspaces is called without changing workspaces, which only occurs from build scripts being finished building. The reason for that is better diagnostics when proc-macros arent loaded yet (but will be). There is probably a better way to handle that though.

[davidbarsky]

Resolved by #14419.

[davidbarsky]

The relevant part there is here

rust-analyzer/crates/rust-analyzer/src/reload.rs

Lines 450 to 463 in b99d5eb

	if same_workspaces {
	if self.config.expand_proc_macros() {
	self.fetch_proc_macros_queue.request_op(cause, proc_macro_paths);
	}
	} else {
	// Set up errors for proc-macros upfront that we haven't run build scripts yet
	let mut proc_macros = FxHashMap::default();
	for paths in proc_macro_paths {
	proc_macros.extend(paths.into_iter().map(move |(crate_id, _)| {
	(crate_id, Err("crate has not yet been build".to_owned()))
	}));
	}
	change.set_proc_macros(proc_macros);
	}

Note that we only call self.fetch_proc_macros_queue.request_op(cause, proc_macro_paths); in the same_workspaces branch which only happens when switch_workspaces is called without changing workspaces, which only occurs from build scripts being finished building. The reason for that is better diagnostics when proc-macros arent loaded yet (but will be). There is probably a better way to handle that though.

Thanks for pointing this out—I'll play around with this bit and see if I can make this a little better in the process.