Skip to content

Commit

Permalink
Unrolled build for rust-lang#123753
Browse files Browse the repository at this point in the history
Rollup merge of rust-lang#123753 - Urgau:compiletest-trailing-directive, r=jieyouxu

compiletest: error when finding a trailing directive

This PR introduce a supplementary check that when checking for a compiletest directive, will also check that the next "word" after that directive is not also by itself a directive.

This is done to avoid situations like this `//@ only-linux only-x86_64` where one might think that both directives are being taken into account while in fact the second in a comment, and so was ignored, until now.

Related to rust-lang#123730
cc ``@scottmcm``
r? ``@jieyouxu``
  • Loading branch information
rust-timer committed Apr 11, 2024
2 parents 05ccc49 + acd4e94 commit 84a2e23
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 5 deletions.
36 changes: 31 additions & 5 deletions src/tools/compiletest/src/header.rs
Original file line number Diff line number Diff line change
Expand Up @@ -935,16 +935,25 @@ struct HeaderLine<'ln> {
pub(crate) struct CheckDirectiveResult<'ln> {
is_known_directive: bool,
directive_name: &'ln str,
trailing_directive: Option<&'ln str>,
}

// Returns `(is_known_directive, directive_name)`.
pub(crate) fn check_directive(directive_ln: &str) -> CheckDirectiveResult<'_> {
let directive_name =
directive_ln.split_once([':', ' ']).map(|(pre, _)| pre).unwrap_or(directive_ln);
let (directive_name, post) = directive_ln.split_once([':', ' ']).unwrap_or((directive_ln, ""));

let trailing = post.trim().split_once(' ').map(|(pre, _)| pre).unwrap_or(post);
let trailing_directive = {
// 1. is the directive name followed by a space? (to exclude `:`)
matches!(directive_ln.get(directive_name.len()..), Some(s) if s.starts_with(" "))
// 2. is what is after that directive also a directive (ex: "only-x86 only-arm")
&& KNOWN_DIRECTIVE_NAMES.contains(&trailing)
}
.then_some(trailing);

CheckDirectiveResult {
is_known_directive: KNOWN_DIRECTIVE_NAMES.contains(&directive_name),
directive_name: directive_ln,
trailing_directive,
}
}

Expand Down Expand Up @@ -1014,7 +1023,8 @@ fn iter_header(
if testfile.extension().map(|e| e == "rs").unwrap_or(false) {
let directive_ln = non_revisioned_directive_line.trim();

let CheckDirectiveResult { is_known_directive, .. } = check_directive(directive_ln);
let CheckDirectiveResult { is_known_directive, trailing_directive, .. } =
check_directive(directive_ln);

if !is_known_directive {
*poisoned = true;
Expand All @@ -1028,6 +1038,21 @@ fn iter_header(

return;
}

if let Some(trailing_directive) = &trailing_directive {
*poisoned = true;

eprintln!(
"error: detected trailing compiletest test directive `{}` in {}:{}\n \
help: put the trailing directive in it's own line: `//@ {}`",
trailing_directive,
testfile.display(),
line_number,
trailing_directive,
);

return;
}
}

it(HeaderLine {
Expand All @@ -1051,7 +1076,8 @@ fn iter_header(

let rest = rest.trim_start();

let CheckDirectiveResult { is_known_directive, directive_name } = check_directive(rest);
let CheckDirectiveResult { is_known_directive, directive_name, .. } =
check_directive(rest);

if is_known_directive {
*poisoned = true;
Expand Down
21 changes: 21 additions & 0 deletions src/tools/compiletest/src/header/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -667,3 +667,24 @@ fn test_non_rs_unknown_directive_not_checked() {
);
assert!(!poisoned);
}

#[test]
fn test_trailing_directive() {
let mut poisoned = false;
run_path(&mut poisoned, Path::new("a.rs"), b"//@ only-x86 only-arm");
assert!(poisoned);
}

#[test]
fn test_trailing_directive_with_comment() {
let mut poisoned = false;
run_path(&mut poisoned, Path::new("a.rs"), b"//@ only-x86 only-arm with comment");
assert!(poisoned);
}

#[test]
fn test_not_trailing_directive() {
let mut poisoned = false;
run_path(&mut poisoned, Path::new("a.rs"), b"//@ revisions: incremental");
assert!(!poisoned);
}

0 comments on commit 84a2e23

Please sign in to comment.