Skip to content

Commit 9e57a8d

Browse files
committed
Auto merge of #13849 - epage:no-infer, r=weihanglo
perf(toml): Avoid inferring when targets are known ### What does this PR try to resolve? We read the file system to infer two different data points - Implicit targets - Implicit `path` values for targets I took a shortcut for this case and recognize the scenario where we can bypass both and do so. I went with a bypass, rather than this being integrating into the inferring code because the inferring code is complex and I didn't want to add to it further in isolating the inferring to only when its needed. The validation gets duplicated because having it in the middle of the resolve code provides a better user experience and it would be messy to add the conditionals to get all of that working. I at least worked to keep the duplicated validation close to each other. ### How should we test and review this PR? ### Additional information
2 parents f892647 + f5892f2 commit 9e57a8d

File tree

1 file changed

+119
-81
lines changed

1 file changed

+119
-81
lines changed

src/cargo/util/toml/targets.rs

Lines changed: 119 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -261,48 +261,60 @@ pub fn resolve_bins(
261261
errors: &mut Vec<String>,
262262
has_lib: bool,
263263
) -> CargoResult<Vec<TomlBinTarget>> {
264-
let inferred = inferred_bins(package_root, package_name);
264+
if is_resolved(toml_bins, autodiscover) {
265+
let toml_bins = toml_bins.cloned().unwrap_or_default();
266+
for bin in &toml_bins {
267+
validate_bin_name(bin, warnings)?;
268+
validate_bin_crate_types(bin, edition, warnings, errors)?;
269+
validate_bin_proc_macro(bin, edition, warnings, errors)?;
270+
}
271+
Ok(toml_bins)
272+
} else {
273+
let inferred = inferred_bins(package_root, package_name);
265274

266-
let mut bins = toml_targets_and_inferred(
267-
toml_bins,
268-
&inferred,
269-
package_root,
270-
autodiscover,
271-
edition,
272-
warnings,
273-
"binary",
274-
"bin",
275-
"autobins",
276-
);
275+
let mut bins = toml_targets_and_inferred(
276+
toml_bins,
277+
&inferred,
278+
package_root,
279+
autodiscover,
280+
edition,
281+
warnings,
282+
"binary",
283+
"bin",
284+
"autobins",
285+
);
277286

278-
for bin in &mut bins {
279-
// Check early to improve error messages
280-
validate_bin_name(bin, warnings)?;
287+
for bin in &mut bins {
288+
// Check early to improve error messages
289+
validate_bin_name(bin, warnings)?;
281290

282-
validate_bin_crate_types(bin, edition, warnings, errors)?;
283-
validate_bin_proc_macro(bin, edition, warnings, errors)?;
291+
validate_bin_crate_types(bin, edition, warnings, errors)?;
292+
validate_bin_proc_macro(bin, edition, warnings, errors)?;
284293

285-
let path = target_path(bin, &inferred, "bin", package_root, edition, &mut |_| {
286-
if let Some(legacy_path) = legacy_bin_path(package_root, name_or_panic(bin), has_lib) {
287-
warnings.push(format!(
288-
"path `{}` was erroneously implicitly accepted for binary `{}`,\n\
294+
let path = target_path(bin, &inferred, "bin", package_root, edition, &mut |_| {
295+
if let Some(legacy_path) =
296+
legacy_bin_path(package_root, name_or_panic(bin), has_lib)
297+
{
298+
warnings.push(format!(
299+
"path `{}` was erroneously implicitly accepted for binary `{}`,\n\
289300
please set bin.path in Cargo.toml",
290-
legacy_path.display(),
291-
name_or_panic(bin)
292-
));
293-
Some(legacy_path)
294-
} else {
295-
None
296-
}
297-
});
298-
let path = match path {
299-
Ok(path) => path,
300-
Err(e) => anyhow::bail!("{}", e),
301-
};
302-
bin.path = Some(PathValue(path));
303-
}
301+
legacy_path.display(),
302+
name_or_panic(bin)
303+
));
304+
Some(legacy_path)
305+
} else {
306+
None
307+
}
308+
});
309+
let path = match path {
310+
Ok(path) => path,
311+
Err(e) => anyhow::bail!("{}", e),
312+
};
313+
bin.path = Some(PathValue(path));
314+
}
304315

305-
Ok(bins)
316+
Ok(bins)
317+
}
306318
}
307319

308320
#[tracing::instrument(skip_all)]
@@ -370,13 +382,13 @@ pub fn resolve_examples(
370382
warnings: &mut Vec<String>,
371383
errors: &mut Vec<String>,
372384
) -> CargoResult<Vec<TomlExampleTarget>> {
373-
let inferred = infer_from_directory(&package_root, Path::new(DEFAULT_EXAMPLE_DIR_NAME));
385+
let mut inferred = || infer_from_directory(&package_root, Path::new(DEFAULT_EXAMPLE_DIR_NAME));
374386

375387
let targets = resolve_targets(
376388
"example",
377389
"example",
378390
toml_examples,
379-
&inferred,
391+
&mut inferred,
380392
package_root,
381393
edition,
382394
autodiscover,
@@ -427,13 +439,13 @@ pub fn resolve_tests(
427439
warnings: &mut Vec<String>,
428440
errors: &mut Vec<String>,
429441
) -> CargoResult<Vec<TomlTestTarget>> {
430-
let inferred = infer_from_directory(&package_root, Path::new(DEFAULT_TEST_DIR_NAME));
442+
let mut inferred = || infer_from_directory(&package_root, Path::new(DEFAULT_TEST_DIR_NAME));
431443

432444
let targets = resolve_targets(
433445
"test",
434446
"test",
435447
toml_tests,
436-
&inferred,
448+
&mut inferred,
437449
package_root,
438450
edition,
439451
autodiscover,
@@ -492,13 +504,13 @@ pub fn resolve_benches(
492504
Some(legacy_path)
493505
};
494506

495-
let inferred = infer_from_directory(&package_root, Path::new(DEFAULT_BENCH_DIR_NAME));
507+
let mut inferred = || infer_from_directory(&package_root, Path::new(DEFAULT_BENCH_DIR_NAME));
496508

497509
let targets = resolve_targets_with_legacy_path(
498510
"benchmark",
499511
"bench",
500512
toml_benches,
501-
&inferred,
513+
&mut inferred,
502514
package_root,
503515
edition,
504516
autodiscover,
@@ -536,11 +548,24 @@ fn to_bench_targets(
536548
Ok(result)
537549
}
538550

551+
fn is_resolved(toml_targets: Option<&Vec<TomlTarget>>, autodiscover: Option<bool>) -> bool {
552+
if autodiscover != Some(false) {
553+
return false;
554+
}
555+
556+
let Some(toml_targets) = toml_targets else {
557+
return true;
558+
};
559+
toml_targets
560+
.iter()
561+
.all(|t| t.name.is_some() && t.path.is_some())
562+
}
563+
539564
fn resolve_targets(
540565
target_kind_human: &str,
541566
target_kind: &str,
542567
toml_targets: Option<&Vec<TomlTarget>>,
543-
inferred: &[(String, PathBuf)],
568+
inferred: &mut dyn FnMut() -> Vec<(String, PathBuf)>,
544569
package_root: &Path,
545570
edition: Edition,
546571
autodiscover: Option<bool>,
@@ -567,7 +592,7 @@ fn resolve_targets_with_legacy_path(
567592
target_kind_human: &str,
568593
target_kind: &str,
569594
toml_targets: Option<&Vec<TomlTarget>>,
570-
inferred: &[(String, PathBuf)],
595+
inferred: &mut dyn FnMut() -> Vec<(String, PathBuf)>,
571596
package_root: &Path,
572597
edition: Edition,
573598
autodiscover: Option<bool>,
@@ -576,47 +601,60 @@ fn resolve_targets_with_legacy_path(
576601
legacy_path: &mut dyn FnMut(&TomlTarget) -> Option<PathBuf>,
577602
autodiscover_flag_name: &str,
578603
) -> CargoResult<Vec<TomlTarget>> {
579-
let toml_targets = toml_targets_and_inferred(
580-
toml_targets,
581-
inferred,
582-
package_root,
583-
autodiscover,
584-
edition,
585-
warnings,
586-
target_kind_human,
587-
target_kind,
588-
autodiscover_flag_name,
589-
);
590-
591-
for target in &toml_targets {
592-
// Check early to improve error messages
593-
validate_target_name(target, target_kind_human, target_kind, warnings)?;
594-
595-
validate_proc_macro(target, target_kind_human, edition, warnings)?;
596-
validate_crate_types(target, target_kind_human, edition, warnings)?;
597-
}
598-
599-
let mut result = Vec::new();
600-
for mut target in toml_targets {
601-
let path = target_path(
602-
&target,
603-
inferred,
604-
target_kind,
604+
if is_resolved(toml_targets, autodiscover) {
605+
let toml_targets = toml_targets.cloned().unwrap_or_default();
606+
for target in &toml_targets {
607+
// Check early to improve error messages
608+
validate_target_name(target, target_kind_human, target_kind, warnings)?;
609+
610+
validate_proc_macro(target, target_kind_human, edition, warnings)?;
611+
validate_crate_types(target, target_kind_human, edition, warnings)?;
612+
}
613+
Ok(toml_targets)
614+
} else {
615+
let inferred = inferred();
616+
let toml_targets = toml_targets_and_inferred(
617+
toml_targets,
618+
&inferred,
605619
package_root,
620+
autodiscover,
606621
edition,
607-
legacy_path,
622+
warnings,
623+
target_kind_human,
624+
target_kind,
625+
autodiscover_flag_name,
608626
);
609-
let path = match path {
610-
Ok(path) => path,
611-
Err(e) => {
612-
errors.push(e);
613-
continue;
614-
}
615-
};
616-
target.path = Some(PathValue(path));
617-
result.push(target);
627+
628+
for target in &toml_targets {
629+
// Check early to improve error messages
630+
validate_target_name(target, target_kind_human, target_kind, warnings)?;
631+
632+
validate_proc_macro(target, target_kind_human, edition, warnings)?;
633+
validate_crate_types(target, target_kind_human, edition, warnings)?;
634+
}
635+
636+
let mut result = Vec::new();
637+
for mut target in toml_targets {
638+
let path = target_path(
639+
&target,
640+
&inferred,
641+
target_kind,
642+
package_root,
643+
edition,
644+
legacy_path,
645+
);
646+
let path = match path {
647+
Ok(path) => path,
648+
Err(e) => {
649+
errors.push(e);
650+
continue;
651+
}
652+
};
653+
target.path = Some(PathValue(path));
654+
result.push(target);
655+
}
656+
Ok(result)
618657
}
619-
Ok(result)
620658
}
621659

622660
fn inferred_lib(package_root: &Path) -> Option<PathBuf> {

0 commit comments

Comments
 (0)