Skip to content

Commit

Permalink
Auto merge of #14567 - Urgau:check-cfg-cleanup-lints, r=epage
Browse files Browse the repository at this point in the history 
Cleanup duplicated check-cfg lint logic

### What does this PR try to resolve?

This PR clean-ups some duplication left by #13958, because of Cargo MSRV.

Fixes #13975

### How should we test and review this PR?

The tests in `tests/testsuite/check_cfg.rs` show no change in behaviours (except for the better error messages). I suggest maybe reviewing commit by commit.
  • Loading branch information
@bors
bors committed Sep 19, 2024
2 parents f1d3500 + 3058f50 commit 9d5c149
Show file tree
Hide file tree
Showing 5 changed files with 43 additions and 63 deletions.
2 changes: 1 addition & 1 deletion src/cargo/core/compiler/build_runner/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ impl<'a, 'gctx> BuildRunner<'a, 'gctx> {
let mut args = compiler::extern_args(&self, unit, &mut unstable_opts)?;
args.extend(compiler::lto_args(&self, unit));
args.extend(compiler::features_args(unit));
args.extend(compiler::check_cfg_args(unit)?);
args.extend(compiler::check_cfg_args(unit));

let script_meta = self.find_build_script_metadata(unit);
if let Some(meta) = script_meta {
Expand Down
22 changes: 0 additions & 22 deletions src/cargo/core/compiler/fingerprint/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1421,34 +1421,12 @@ fn calculate_normal(
}
.to_vec();

// Include all the args from `[lints.rust.unexpected_cfgs.check-cfg]`
//
// HACK(#13975): duplicating the lookup logic here until `--check-cfg` is supported
// on Cargo's MSRV and we can centralize the logic in `lints_to_rustflags`
let mut lint_check_cfg = Vec::new();
if let Ok(Some(lints)) = unit.pkg.manifest().normalized_toml().normalized_lints() {
if let Some(rust_lints) = lints.get("rust") {
if let Some(unexpected_cfgs) = rust_lints.get("unexpected_cfgs") {
if let Some(config) = unexpected_cfgs.config() {
if let Some(check_cfg) = config.get("check-cfg") {
if let Ok(check_cfgs) =
toml::Value::try_into::<Vec<String>>(check_cfg.clone())
{
lint_check_cfg = check_cfgs;
}
}
}
}
}
}

let profile_hash = util::hash_u64((
&unit.profile,
unit.mode,
build_runner.bcx.extra_args_for(unit),
build_runner.lto[unit],
unit.pkg.manifest().lint_rustflags(),
lint_check_cfg,
));
// Include metadata since it is exposed as environment variables.
let m = unit.pkg.manifest().metadata();
Expand Down
39 changes: 6 additions & 33 deletions src/cargo/core/compiler/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ use std::io::{BufRead, Write};
use std::path::{Path, PathBuf};
use std::sync::Arc;

use anyhow::{bail, Context as _, Error};
use anyhow::{Context as _, Error};
use lazycell::LazyCell;
use tracing::{debug, trace};

Expand Down Expand Up @@ -743,7 +743,7 @@ fn prepare_rustdoc(build_runner: &BuildRunner<'_, '_>, unit: &Unit) -> CargoResu
let doc_dir = build_runner.files().out_dir(unit);
rustdoc.arg("-o").arg(&doc_dir);
rustdoc.args(&features_args(unit));
rustdoc.args(&check_cfg_args(unit)?);
rustdoc.args(&check_cfg_args(unit));

add_error_format_and_color(build_runner, &mut rustdoc);
add_allow_features(build_runner, &mut rustdoc);
Expand Down Expand Up @@ -1140,7 +1140,7 @@ fn build_base_args(
}

cmd.args(&features_args(unit));
cmd.args(&check_cfg_args(unit)?);
cmd.args(&check_cfg_args(unit));

let meta = build_runner.files().metadata(unit);
cmd.arg("-C").arg(&format!("metadata={}", meta));
Expand Down Expand Up @@ -1354,7 +1354,7 @@ fn package_remap(build_runner: &BuildRunner<'_, '_>, unit: &Unit) -> OsString {
}

/// Generates the `--check-cfg` arguments for the `unit`.
fn check_cfg_args(unit: &Unit) -> CargoResult<Vec<OsString>> {
fn check_cfg_args(unit: &Unit) -> Vec<OsString> {
// The routine below generates the --check-cfg arguments. Our goals here are to
// enable the checking of conditionals and pass the list of declared features.
//
Expand Down Expand Up @@ -1391,39 +1391,12 @@ fn check_cfg_args(unit: &Unit) -> CargoResult<Vec<OsString>> {
// Cargo and docs.rs than rustc and docs.rs. In particular, all users of docs.rs use
// Cargo, but not all users of rustc (like Rust-for-Linux) use docs.rs.

let mut args = vec![
vec![
OsString::from("--check-cfg"),
OsString::from("cfg(docsrs)"),
OsString::from("--check-cfg"),
arg_feature,
];

// Also include the custom arguments specified in `[lints.rust.unexpected_cfgs.check_cfg]`
if let Ok(Some(lints)) = unit.pkg.manifest().normalized_toml().normalized_lints() {
if let Some(rust_lints) = lints.get("rust") {
if let Some(unexpected_cfgs) = rust_lints.get("unexpected_cfgs") {
if let Some(config) = unexpected_cfgs.config() {
if let Some(check_cfg) = config.get("check-cfg") {
if let Ok(check_cfgs) =
toml::Value::try_into::<Vec<String>>(check_cfg.clone())
{
for check_cfg in check_cfgs {
args.push(OsString::from("--check-cfg"));
args.push(OsString::from(check_cfg));
}
// error about `check-cfg` not being a list-of-string
} else {
bail!(
"`lints.rust.unexpected_cfgs.check-cfg` must be a list of string"
);
}
}
}
}
}
}

Ok(args)
]
}

/// Adds LTO related codegen flags.
Expand Down
29 changes: 26 additions & 3 deletions src/cargo/util/toml/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1446,7 +1446,7 @@ pub fn to_real_manifest(
.normalized_lints()
.expect("previously normalized")
.unwrap_or(&default),
);
)?;

let metadata = ManifestMetadata {
description: normalized_package
Expand Down Expand Up @@ -2545,7 +2545,7 @@ switch to nightly channel you can pass
warnings.push(message);
}

fn lints_to_rustflags(lints: &manifest::TomlLints) -> Vec<String> {
fn lints_to_rustflags(lints: &manifest::TomlLints) -> CargoResult<Vec<String>> {
let mut rustflags = lints
.iter()
// We don't want to pass any of the `cargo` lints to `rustc`
Expand Down Expand Up @@ -2575,7 +2575,30 @@ fn lints_to_rustflags(lints: &manifest::TomlLints) -> Vec<String> {
})
.collect::<Vec<_>>();
rustflags.sort();
rustflags.into_iter().map(|(_, _, option)| option).collect()

let mut rustflags: Vec<_> = rustflags.into_iter().map(|(_, _, option)| option).collect();

// Also include the custom arguments specified in `[lints.rust.unexpected_cfgs.check_cfg]`
if let Some(rust_lints) = lints.get("rust") {
if let Some(unexpected_cfgs) = rust_lints.get("unexpected_cfgs") {
if let Some(config) = unexpected_cfgs.config() {
if let Some(check_cfg) = config.get("check-cfg") {
if let Ok(check_cfgs) = toml::Value::try_into::<Vec<String>>(check_cfg.clone())
{
for check_cfg in check_cfgs {
rustflags.push("--check-cfg".to_string());
rustflags.push(check_cfg);
}
// error about `check-cfg` not being a list-of-string
} else {
bail!("`lints.rust.unexpected_cfgs.check-cfg` must be a list of string");
}
}
}
}
}

Ok(rustflags)
}

fn emit_diagnostic(
Expand Down
14 changes: 10 additions & 4 deletions tests/testsuite/check_cfg.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -707,8 +707,11 @@ fn config_invalid_not_list() {
p.cargo("check")
.with_status(101)
.with_stderr_data(str![[r#"
[ERROR] `lints.rust.unexpected_cfgs.check-cfg` must be a list of string
...
[ERROR] failed to parse manifest at `[ROOT]/foo/Cargo.toml`
Caused by:
`lints.rust.unexpected_cfgs.check-cfg` must be a list of string
"#]])
.run();
}
Expand All @@ -734,8 +737,11 @@ fn config_invalid_not_list_string() {
p.cargo("check")
.with_status(101)
.with_stderr_data(str![[r#"
[ERROR] `lints.rust.unexpected_cfgs.check-cfg` must be a list of string
...
[ERROR] failed to parse manifest at `[ROOT]/foo/Cargo.toml`
Caused by:
`lints.rust.unexpected_cfgs.check-cfg` must be a list of string
"#]])
.run();
}
Expand Down

0 comments on commit 9d5c149

Please sign in to comment.