Skip to content

Commit 6fa435d

Browse files
authored
Rollup merge of rust-lang#156237 - lqd:compiletest-revisions, r=wesleywiser,jieyouxu
compiletest: prevent directives from having multiple revisions Right now, a compiletest directive like `//@ [foo,bar] compile-flags: -Z hello` is accepted. While it looks similar to a `//[foo,bar]` error annotation matching multiple revisions, it will instead model a directive for a single revision named `foo,bar` and not for two revisions. Slightly inconsistent/confusing. I don't know that it's common enough to make bigger changes to support it fully (`miri`'s test harness seems to support that use-case), but since I hit that today, this PR prevents that case for now. Otherwise, I think we'd have to turn all uses of `line_directive` as possibly creating multiple line directives, one per revision, etc. Such a directive will now yield an error, with a best-effort suggestion to split it into multiple single-revision directives: ``` multiple revisions aren't supported yet in `//@ [foo,bar] compile-flags: -Z hello`, split them like //@ [foo]: compile-flags: -Z hello //@ [bar]: compile-flags: -Z hello ```
2 parents 99e0e37 + 325b013 commit 6fa435d

2 files changed

Lines changed: 29 additions & 0 deletions

File tree

src/tools/compiletest/src/directives/line.rs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,20 @@ pub(crate) fn line_directive<'a>(
3030

3131
revision = Some(line_revision);
3232
raw_directive = after_close_bracket.trim_start();
33+
34+
if line_revision.contains(",") {
35+
let suggestion: Vec<_> = line_revision
36+
.split(",")
37+
.map(|revision| {
38+
format!("{COMPILETEST_DIRECTIVE_PREFIX} [{revision}]: {raw_directive}")
39+
})
40+
.collect();
41+
panic!(
42+
"malformed condition directive: multiple revisions aren't supported yet in `{}`, split them like\n{}",
43+
original_line,
44+
suggestion.join("\n"),
45+
);
46+
}
3347
} else {
3448
revision = None;
3549
raw_directive = after_comment;

src/tools/compiletest/src/directives/tests.rs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -632,6 +632,21 @@ fn test_miropt_mode_forbidden_revisions() {
632632
parse_early_props(&config, "//@ revisions: CHECK");
633633
}
634634

635+
#[test]
636+
#[should_panic(expected = "malformed condition directive: multiple revisions aren't supported yet")]
637+
fn test_multiple_revisions_in_directive() {
638+
let directive = "//@ [foo,bar] compile-flags: -Z hello";
639+
640+
// The problem: this is seen as a single revision.
641+
let line_directive = line_directive(Utf8Path::new("foo.txt"), LineNumber::ZERO, directive);
642+
assert!(line_directive.is_some());
643+
assert_eq!(Some("foo,bar"), line_directive.unwrap().revision);
644+
645+
// The solution for now: forbid directives from having multiple revisions.
646+
let config: Config = cfg().build();
647+
parse_early_props(&config, directive);
648+
}
649+
635650
#[test]
636651
fn test_forbidden_revisions_allowed_in_non_filecheck_dir() {
637652
let revisions = ["CHECK", "COM", "NEXT", "SAME", "EMPTY", "NOT", "COUNT", "DAG", "LABEL"];

0 commit comments

Comments
 (0)