Skip to content

Commit 3b6aeeb

Browse files
VeykrilVeykril
committed
internal: Better type proc macro dylib build data state
1 parent 971c393 commit 3b6aeeb

File tree

6 files changed

+186
-114
lines changed

6 files changed

+186
-114
lines changed

‎crates/base-db/src/input.rs

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,8 @@ pub type ProcMacroPaths =
3030
pub enum ProcMacroLoadingError {
3131
Disabled,
3232
FailedToBuild,
33-
MissingDylibPath,
33+
ExpectedProcMacroArtifact(Box<[String]>),
34+
MissingDylibPath(Box<[String]>),
3435
NotYetBuilt,
3536
NoProcMacros,
3637
ProcMacroSrvError(Box<str>),
@@ -39,8 +40,9 @@ impl ProcMacroLoadingError {
3940
pub fn is_hard_error(&self) -> bool {
4041
match self {
4142
ProcMacroLoadingError::Disabled | ProcMacroLoadingError::NotYetBuilt => false,
42-
ProcMacroLoadingError::FailedToBuild
43-
| ProcMacroLoadingError::MissingDylibPath
43+
ProcMacroLoadingError::ExpectedProcMacroArtifact(_)
44+
| ProcMacroLoadingError::FailedToBuild
45+
| ProcMacroLoadingError::MissingDylibPath(_)
4446
| ProcMacroLoadingError::NoProcMacros
4547
| ProcMacroLoadingError::ProcMacroSrvError(_) => true,
4648
}
@@ -51,10 +53,23 @@ impl Error for ProcMacroLoadingError {}
5153
impl fmt::Display for ProcMacroLoadingError {
5254
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
5355
match self {
56+
ProcMacroLoadingError::ExpectedProcMacroArtifact(targets) => {
57+
write!(f, "proc-macro artifact expected, but got target: {}", targets.join(", "))
58+
}
5459
ProcMacroLoadingError::Disabled => write!(f, "proc-macro expansion is disabled"),
5560
ProcMacroLoadingError::FailedToBuild => write!(f, "proc-macro failed to build"),
56-
ProcMacroLoadingError::MissingDylibPath => {
57-
write!(f, "proc-macro crate build data is missing a dylib path")
61+
ProcMacroLoadingError::MissingDylibPath(candidates) if candidates.is_empty() => {
62+
write!(
63+
f,
64+
"proc-macro crate built but the dylib path is missing, this indicates a problem with your build system."
65+
)
66+
}
67+
ProcMacroLoadingError::MissingDylibPath(candidates) => {
68+
write!(
69+
f,
70+
"proc-macro crate built but the dylib path is missing, this indicates a problem with your build system. Candidates not considered due to not having a dynamic library extension: {}",
71+
candidates.join(", ")
72+
)
5873
}
5974
ProcMacroLoadingError::NotYetBuilt => write!(f, "proc-macro not yet built"),
6075
ProcMacroLoadingError::NoProcMacros => {

‎crates/project-model/src/build_dependencies.rs

Lines changed: 84 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ use la_arena::ArenaMap;
1616
use paths::{AbsPath, AbsPathBuf, Utf8PathBuf};
1717
use rustc_hash::{FxHashMap, FxHashSet};
1818
use serde::Deserialize as _;
19+
use stdx::{always, never};
1920
use toolchain::Tool;
2021

2122
use crate::{
@@ -30,6 +31,15 @@ pub struct WorkspaceBuildScripts {
3031
error: Option<String>,
3132
}
3233

34+
#[derive(Debug, Clone, Default, PartialEq, Eq)]
35+
pub enum ProcMacroDylibPath {
36+
Path(AbsPathBuf),
37+
DylibNotFound(Box<[Utf8PathBuf]>),
38+
NotProcMacro(Box<[TargetKind]>),
39+
#[default]
40+
NotBuilt,
41+
}
42+
3343
/// Output of the build script and proc-macro building step for a concrete package.
3444
#[derive(Debug, Clone, Default, PartialEq, Eq)]
3545
pub(crate) struct BuildScriptOutput {
@@ -43,15 +53,15 @@ pub(crate) struct BuildScriptOutput {
4353
/// Directory where a build script might place its output.
4454
pub(crate) out_dir: Option<AbsPathBuf>,
4555
/// Path to the proc-macro library file if this package exposes proc-macros.
46-
pub(crate) proc_macro_dylib_path: Option<AbsPathBuf>,
56+
pub(crate) proc_macro_dylib_path: ProcMacroDylibPath,
4757
}
4858

4959
impl BuildScriptOutput {
5060
fn is_empty(&self) -> bool {
5161
self.cfgs.is_empty()
5262
&& self.envs.is_empty()
5363
&& self.out_dir.is_none()
54-
&& self.proc_macro_dylib_path.is_none()
64+
&& self.proc_macro_dylib_path == ProcMacroDylibPath::NotBuilt
5565
}
5666
}
5767

@@ -126,6 +136,8 @@ impl WorkspaceBuildScripts {
126136
|package, cb| {
127137
if let Some(&(package, workspace)) = by_id.get(package) {
128138
cb(&workspaces[workspace][package].name, &mut res[workspace].outputs[package]);
139+
} else {
140+
never!("Received compiler message for unknown package: {}", package);
129141
}
130142
},
131143
progress,
@@ -140,12 +152,9 @@ impl WorkspaceBuildScripts {
140152
if tracing::enabled!(tracing::Level::INFO) {
141153
for (idx, workspace) in workspaces.iter().enumerate() {
142154
for package in workspace.packages() {
143-
let package_build_data = &mut res[idx].outputs[package];
155+
let package_build_data: &mut BuildScriptOutput = &mut res[idx].outputs[package];
144156
if !package_build_data.is_empty() {
145-
tracing::info!(
146-
"{}: {package_build_data:?}",
147-
workspace[package].manifest.parent(),
148-
);
157+
tracing::info!("{}: {package_build_data:?}", workspace[package].manifest,);
149158
}
150159
}
151160
}
@@ -198,39 +207,60 @@ impl WorkspaceBuildScripts {
198207
let path = dir_entry.path();
199208
let extension = path.extension()?;
200209
if extension == std::env::consts::DLL_EXTENSION {
201-
let name = path.file_stem()?.to_str()?.split_once('-')?.0.to_owned();
202-
let path = AbsPathBuf::try_from(Utf8PathBuf::from_path_buf(path).ok()?)
203-
.ok()?;
204-
return Some((name, path));
210+
let name = path
211+
.file_stem()?
212+
.to_str()?
213+
.split_once('-')?
214+
.0
215+
.trim_start_matches("lib")
216+
.to_owned();
217+
let path = match Utf8PathBuf::from_path_buf(path) {
218+
Ok(path) => path,
219+
Err(path) => {
220+
tracing::warn!(
221+
"Proc-macro dylib path contains non-UTF8 characters: {:?}",
222+
path.display()
223+
);
224+
return None;
225+
}
226+
};
227+
return match AbsPathBuf::try_from(path) {
228+
Ok(path) => Some((name, path)),
229+
Err(path) => {
230+
tracing::error!(
231+
"proc-macro dylib path is not absolute: {:?}",
232+
path
233+
);
234+
None
235+
}
236+
};
205237
}
206238
}
207239
None
208240
})
209241
.collect();
210242
for p in rustc.packages() {
211243
let package = &rustc[p];
212-
if package
213-
.targets
214-
.iter()
215-
.any(|&it| matches!(rustc[it].kind, TargetKind::Lib { is_proc_macro: true }))
216-
{
217-
if let Some((_, path)) = proc_macro_dylibs
218-
.iter()
219-
.find(|(name, _)| *name.trim_start_matches("lib") == package.name)
220-
{
221-
bs.outputs[p].proc_macro_dylib_path = Some(path.clone());
244+
bs.outputs[p].proc_macro_dylib_path =
245+
if package.targets.iter().any(|&it| {
246+
matches!(rustc[it].kind, TargetKind::Lib { is_proc_macro: true })
247+
}) {
248+
match proc_macro_dylibs.iter().find(|(name, _)| *name == package.name) {
249+
Some((_, path)) => ProcMacroDylibPath::Path(path.clone()),
250+
_ => ProcMacroDylibPath::DylibNotFound(Box::default()),
251+
}
252+
} else {
253+
ProcMacroDylibPath::NotProcMacro(
254+
package.targets.iter().map(|&target| rustc[target].kind).collect(),
255+
)
222256
}
223-
}
224257
}
225258

226259
if tracing::enabled!(tracing::Level::INFO) {
227260
for package in rustc.packages() {
228261
let package_build_data = &bs.outputs[package];
229262
if !package_build_data.is_empty() {
230-
tracing::info!(
231-
"{}: {package_build_data:?}",
232-
rustc[package].manifest.parent(),
233-
);
263+
tracing::info!("{}: {package_build_data:?}", rustc[package].manifest,);
234264
}
235265
}
236266
}
@@ -263,6 +293,8 @@ impl WorkspaceBuildScripts {
263293
|package, cb| {
264294
if let Some(&package) = by_id.get(package) {
265295
cb(&workspace[package].name, &mut outputs[package]);
296+
} else {
297+
never!("Received compiler message for unknown package: {}", package);
266298
}
267299
},
268300
progress,
@@ -272,10 +304,7 @@ impl WorkspaceBuildScripts {
272304
for package in workspace.packages() {
273305
let package_build_data = &outputs[package];
274306
if !package_build_data.is_empty() {
275-
tracing::info!(
276-
"{}: {package_build_data:?}",
277-
workspace[package].manifest.parent(),
278-
);
307+
tracing::info!("{}: {package_build_data:?}", workspace[package].manifest,);
279308
}
280309
}
281310
}
@@ -348,15 +377,33 @@ impl WorkspaceBuildScripts {
348377
progress(format!(
349378
"building compile-time-deps: proc-macro {name} built"
350379
));
351-
if message.target.kind.contains(&cargo_metadata::TargetKind::ProcMacro)
380+
always!(
381+
data.proc_macro_dylib_path == ProcMacroDylibPath::NotBuilt,
382+
"received multiple compiler artifacts for the same package: {message:?}"
383+
);
384+
data.proc_macro_dylib_path = if message
385+
.target
386+
.kind
387+
.contains(&cargo_metadata::TargetKind::ProcMacro)
352388
{
353-
// Skip rmeta file
354-
if let Some(filename) =
355-
message.filenames.iter().find(|file| is_dylib(file))
356-
{
357-
let filename = AbsPath::assert(filename);
358-
data.proc_macro_dylib_path = Some(filename.to_owned());
389+
match message.filenames.iter().find(|file| is_dylib(file)) {
390+
Some(filename) => {
391+
let filename = AbsPath::assert(filename);
392+
ProcMacroDylibPath::Path(filename.to_owned())
393+
}
394+
None => ProcMacroDylibPath::DylibNotFound(
395+
message.filenames.clone().into_boxed_slice(),
396+
),
359397
}
398+
} else {
399+
ProcMacroDylibPath::NotProcMacro(
400+
message
401+
.target
402+
.kind
403+
.iter()
404+
.map(|target| TargetKind::new(std::slice::from_ref(target)))
405+
.collect(),
406+
)
360407
}
361408
});
362409
}

‎crates/project-model/src/cargo_workspace.rs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -245,7 +245,7 @@ pub enum TargetKind {
245245
}
246246

247247
impl TargetKind {
248-
fn new(kinds: &[cargo_metadata::TargetKind]) -> TargetKind {
248+
pub fn new(kinds: &[cargo_metadata::TargetKind]) -> TargetKind {
249249
for kind in kinds {
250250
return match kind {
251251
cargo_metadata::TargetKind::Bin => TargetKind::Bin,
@@ -604,12 +604,12 @@ impl FetchMetadata {
604604
// but nothing else
605605
let mut extra_args = config.extra_args.iter();
606606
while let Some(arg) = extra_args.next() {
607-
if arg == "-Z" {
608-
if let Some(arg) = extra_args.next() {
609-
needs_nightly = true;
610-
other_options.push("-Z".to_owned());
611-
other_options.push(arg.to_owned());
612-
}
607+
if arg == "-Z"
608+
&& let Some(arg) = extra_args.next()
609+
{
610+
needs_nightly = true;
611+
other_options.push("-Z".to_owned());
612+
other_options.push(arg.to_owned());
613613
}
614614
}
615615

‎crates/project-model/src/lib.rs

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ use paths::{AbsPath, AbsPathBuf, Utf8PathBuf};
5959
use rustc_hash::FxHashSet;
6060

6161
pub use crate::{
62-
build_dependencies::WorkspaceBuildScripts,
62+
build_dependencies::{ProcMacroDylibPath, WorkspaceBuildScripts},
6363
cargo_workspace::{
6464
CargoConfig, CargoFeatures, CargoMetadataConfig, CargoWorkspace, Package, PackageData,
6565
PackageDependency, RustLibSource, Target, TargetData, TargetKind,
@@ -139,21 +139,22 @@ impl ProjectManifest {
139139
}
140140

141141
fn find_in_parent_dirs(path: &AbsPath, target_file_name: &str) -> Option<ManifestPath> {
142-
if path.file_name().unwrap_or_default() == target_file_name {
143-
if let Ok(manifest) = ManifestPath::try_from(path.to_path_buf()) {
144-
return Some(manifest);
145-
}
142+
if path.file_name().unwrap_or_default() == target_file_name
143+
&& let Ok(manifest) = ManifestPath::try_from(path.to_path_buf())
144+
{
145+
return Some(manifest);
146146
}
147147

148148
let mut curr = Some(path);
149149

150150
while let Some(path) = curr {
151151
let candidate = path.join(target_file_name);
152-
if fs::metadata(&candidate).is_ok() {
153-
if let Ok(manifest) = ManifestPath::try_from(candidate) {
154-
return Some(manifest);
155-
}
152+
if fs::metadata(&candidate).is_ok()
153+
&& let Ok(manifest) = ManifestPath::try_from(candidate)
154+
{
155+
return Some(manifest);
156156
}
157+
157158
curr = path.parent();
158159
}
159160

‎crates/project-model/src/sysroot.rs

Lines changed: 23 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -143,12 +143,11 @@ impl Sysroot {
143143
Some(root) => {
144144
// special case rustc, we can look that up directly in the sysroot's bin folder
145145
// as it should never invoke another cargo binary
146-
if let Tool::Rustc = tool {
147-
if let Some(path) =
146+
if let Tool::Rustc = tool
147+
&& let Some(path) =
148148
probe_for_binary(root.join("bin").join(Tool::Rustc.name()).into())
149-
{
150-
return toolchain::command(path, current_dir, envs);
151-
}
149+
{
150+
return toolchain::command(path, current_dir, envs);
152151
}
153152

154153
let mut cmd = toolchain::command(tool.prefer_proxy(), current_dir, envs);
@@ -291,29 +290,26 @@ impl Sysroot {
291290

292291
pub fn set_workspace(&mut self, workspace: RustLibSrcWorkspace) {
293292
self.workspace = workspace;
294-
if self.error.is_none() {
295-
if let Some(src_root) = &self.rust_lib_src_root {
296-
let has_core = match &self.workspace {
297-
RustLibSrcWorkspace::Workspace(ws) => {
298-
ws.packages().any(|p| ws[p].name == "core")
299-
}
300-
RustLibSrcWorkspace::Json(project_json) => project_json
301-
.crates()
302-
.filter_map(|(_, krate)| krate.display_name.clone())
303-
.any(|name| name.canonical_name().as_str() == "core"),
304-
RustLibSrcWorkspace::Stitched(stitched) => stitched.by_name("core").is_some(),
305-
RustLibSrcWorkspace::Empty => true,
293+
if self.error.is_none()
294+
&& let Some(src_root) = &self.rust_lib_src_root
295+
{
296+
let has_core = match &self.workspace {
297+
RustLibSrcWorkspace::Workspace(ws) => ws.packages().any(|p| ws[p].name == "core"),
298+
RustLibSrcWorkspace::Json(project_json) => project_json
299+
.crates()
300+
.filter_map(|(_, krate)| krate.display_name.clone())
301+
.any(|name| name.canonical_name().as_str() == "core"),
302+
RustLibSrcWorkspace::Stitched(stitched) => stitched.by_name("core").is_some(),
303+
RustLibSrcWorkspace::Empty => true,
304+
};
305+
if !has_core {
306+
let var_note = if env::var_os("RUST_SRC_PATH").is_some() {
307+
" (env var `RUST_SRC_PATH` is set and may be incorrect, try unsetting it)"
308+
} else {
309+
", try running `rustup component add rust-src` to possibly fix this"
306310
};
307-
if !has_core {
308-
let var_note = if env::var_os("RUST_SRC_PATH").is_some() {
309-
" (env var `RUST_SRC_PATH` is set and may be incorrect, try unsetting it)"
310-
} else {
311-
", try running `rustup component add rust-src` to possibly fix this"
312-
};
313-
self.error = Some(format!(
314-
"sysroot at `{src_root}` is missing a `core` library{var_note}",
315-
));
316-
}
311+
self.error =
312+
Some(format!("sysroot at `{src_root}` is missing a `core` library{var_note}",));
317313
}
318314
}
319315
}

0 commit comments

Comments
 (0)