Skip to content

Commit c09e2bd

Browse files
authored
Unrolled build for rust-lang#133736
Rollup merge of rust-lang#133736 - jieyouxu:needs-target-has-atomic, r=compiler-errors Add `needs-target-has-atomic` directive Before this PR, the test writer has to specify platforms and architectures by hand for targets that have differing atomic width support. `#[cfg(target_has_atomic="...")]` is not quite the same because (1) you may have to specify additional matchers manually which has to be maintained individually, and (2) the `#[cfg]` blocks does not communicate to compiletest that a test would be ignored for a given target. This PR implements a `//@ needs-target-has-atomic` directive which admits a comma-separated list of required atomic widths that the target must satisfy in order for the test to run. ``` //@ needs-target-has-atomic: 8, 16, ptr ``` See <rust-lang#87377>. This PR supersedes rust-lang#133095 and is co-authored by `@kei519,` because it was somewhat subtle, and it turned out easier to implement than to review. rustc-dev-guide docs PR: rust-lang/rustc-dev-guide#2154
2 parents 32eea2f + 99c2322 commit c09e2bd

File tree

4 files changed

+106
-5
lines changed

4 files changed

+106
-5
lines changed

src/tools/compiletest/src/common.rs

+21-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use std::collections::{HashMap, HashSet};
1+
use std::collections::{BTreeSet, HashMap, HashSet};
22
use std::ffi::OsString;
33
use std::path::{Path, PathBuf};
44
use std::process::Command;
@@ -486,6 +486,9 @@ impl Config {
486486
}
487487
}
488488

489+
/// Known widths of `target_has_atomic`.
490+
pub const KNOWN_TARGET_HAS_ATOMIC_WIDTHS: &[&str] = &["8", "16", "32", "64", "128", "ptr"];
491+
489492
#[derive(Debug, Clone)]
490493
pub struct TargetCfgs {
491494
pub current: TargetCfg,
@@ -611,6 +614,17 @@ impl TargetCfgs {
611614
("panic", Some("abort")) => cfg.panic = PanicStrategy::Abort,
612615
("panic", Some("unwind")) => cfg.panic = PanicStrategy::Unwind,
613616
("panic", other) => panic!("unexpected value for panic cfg: {other:?}"),
617+
618+
("target_has_atomic", Some(width))
619+
if KNOWN_TARGET_HAS_ATOMIC_WIDTHS.contains(&width) =>
620+
{
621+
cfg.target_has_atomic.insert(width.to_string());
622+
}
623+
("target_has_atomic", Some(other)) => {
624+
panic!("unexpected value for `target_has_atomic` cfg: {other:?}")
625+
}
626+
// Nightly-only std-internal impl detail.
627+
("target_has_atomic", None) => {}
614628
_ => {}
615629
}
616630
}
@@ -645,6 +659,12 @@ pub struct TargetCfg {
645659
pub(crate) xray: bool,
646660
#[serde(default = "default_reloc_model")]
647661
pub(crate) relocation_model: String,
662+
663+
// Not present in target cfg json output, additional derived information.
664+
#[serde(skip)]
665+
/// Supported target atomic widths: e.g. `8` to `128` or `ptr`. This is derived from the builtin
666+
/// `target_has_atomic` `cfg`s e.g. `target_has_atomic="8"`.
667+
pub(crate) target_has_atomic: BTreeSet<String>,
648668
}
649669

650670
impl TargetCfg {

src/tools/compiletest/src/directive-list.rs

+1
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,7 @@ const KNOWN_DIRECTIVE_NAMES: &[&str] = &[
154154
"needs-sanitizer-thread",
155155
"needs-std-debug-assertions",
156156
"needs-symlink",
157+
"needs-target-has-atomic",
157158
"needs-threads",
158159
"needs-unwind",
159160
"needs-wasmtime",

src/tools/compiletest/src/header/needs.rs

+47-4
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use crate::common::{Config, Sanitizer};
1+
use crate::common::{Config, KNOWN_TARGET_HAS_ATOMIC_WIDTHS, Sanitizer};
22
use crate::header::{IgnoreDecision, llvm_has_libzstd};
33

44
pub(super) fn handle_needs(
@@ -171,11 +171,54 @@ pub(super) fn handle_needs(
171171
},
172172
];
173173

174-
let (name, comment) = match ln.split_once([':', ' ']) {
175-
Some((name, comment)) => (name, Some(comment)),
174+
let (name, rest) = match ln.split_once([':', ' ']) {
175+
Some((name, rest)) => (name, Some(rest)),
176176
None => (ln, None),
177177
};
178178

179+
// FIXME(jieyouxu): tighten up this parsing to reject using both `:` and ` ` as means to
180+
// delineate value.
181+
if name == "needs-target-has-atomic" {
182+
let Some(rest) = rest else {
183+
return IgnoreDecision::Error {
184+
message: "expected `needs-target-has-atomic` to have a comma-separated list of atomic widths".to_string(),
185+
};
186+
};
187+
188+
// Expect directive value to be a list of comma-separated atomic widths.
189+
let specified_widths = rest
190+
.split(',')
191+
.map(|width| width.trim())
192+
.map(ToString::to_string)
193+
.collect::<Vec<String>>();
194+
195+
for width in &specified_widths {
196+
if !KNOWN_TARGET_HAS_ATOMIC_WIDTHS.contains(&width.as_str()) {
197+
return IgnoreDecision::Error {
198+
message: format!(
199+
"unknown width specified in `needs-target-has-atomic`: `{width}` is not a \
200+
known `target_has_atomic_width`, known values are `{:?}`",
201+
KNOWN_TARGET_HAS_ATOMIC_WIDTHS
202+
),
203+
};
204+
}
205+
}
206+
207+
let satisfies_all_specified_widths = specified_widths
208+
.iter()
209+
.all(|specified| config.target_cfg().target_has_atomic.contains(specified));
210+
if satisfies_all_specified_widths {
211+
return IgnoreDecision::Continue;
212+
} else {
213+
return IgnoreDecision::Ignore {
214+
reason: format!(
215+
"skipping test as target does not support all of the required `target_has_atomic` widths `{:?}`",
216+
specified_widths
217+
),
218+
};
219+
}
220+
}
221+
179222
if !name.starts_with("needs-") {
180223
return IgnoreDecision::Continue;
181224
}
@@ -193,7 +236,7 @@ pub(super) fn handle_needs(
193236
break;
194237
} else {
195238
return IgnoreDecision::Ignore {
196-
reason: if let Some(comment) = comment {
239+
reason: if let Some(comment) = rest {
197240
format!("{} ({})", need.ignore_reason, comment.trim())
198241
} else {
199242
need.ignore_reason.into()

src/tools/compiletest/src/header/tests.rs

+37
Original file line numberDiff line numberDiff line change
@@ -787,3 +787,40 @@ fn test_not_trailing_directive() {
787787
run_path(&mut poisoned, Path::new("a.rs"), b"//@ revisions: incremental");
788788
assert!(!poisoned);
789789
}
790+
791+
#[test]
792+
fn test_needs_target_has_atomic() {
793+
use std::collections::BTreeSet;
794+
795+
// `x86_64-unknown-linux-gnu` supports `["8", "16", "32", "64", "ptr"]` but not `128`.
796+
797+
let config = cfg().target("x86_64-unknown-linux-gnu").build();
798+
// Expectation sanity check.
799+
assert_eq!(
800+
config.target_cfg().target_has_atomic,
801+
BTreeSet::from([
802+
"8".to_string(),
803+
"16".to_string(),
804+
"32".to_string(),
805+
"64".to_string(),
806+
"ptr".to_string()
807+
]),
808+
"expected `x86_64-unknown-linux-gnu` to not have 128-bit atomic support"
809+
);
810+
811+
assert!(!check_ignore(&config, "//@ needs-target-has-atomic: 8"));
812+
assert!(!check_ignore(&config, "//@ needs-target-has-atomic: 16"));
813+
assert!(!check_ignore(&config, "//@ needs-target-has-atomic: 32"));
814+
assert!(!check_ignore(&config, "//@ needs-target-has-atomic: 64"));
815+
assert!(!check_ignore(&config, "//@ needs-target-has-atomic: ptr"));
816+
817+
assert!(check_ignore(&config, "//@ needs-target-has-atomic: 128"));
818+
819+
assert!(!check_ignore(&config, "//@ needs-target-has-atomic: 8,16,32,64,ptr"));
820+
821+
assert!(check_ignore(&config, "//@ needs-target-has-atomic: 8,16,32,64,ptr,128"));
822+
823+
// Check whitespace between widths is permitted.
824+
assert!(!check_ignore(&config, "//@ needs-target-has-atomic: 8, ptr"));
825+
assert!(check_ignore(&config, "//@ needs-target-has-atomic: 8, ptr, 128"));
826+
}

0 commit comments

Comments
 (0)