From e47df2483e002189b07899913b00450a950b6dea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Le=C3=B3n=20Orell=20Valerian=20Liehr?= Date: Sun, 8 Nov 2026 17:48:05 +0100 Subject: [PATCH 1/7] compiletest: Inline fn `check_directive` and type `CheckDirectiveResult` --- src/tools/compiletest/src/directives.rs | 58 +++++++------------ src/tools/compiletest/src/directives/tests.rs | 51 +++++++--------- src/tools/compiletest/src/lib.rs | 2 +- 3 files changed, 42 insertions(+), 69 deletions(-) diff --git a/src/tools/compiletest/src/directives.rs b/src/tools/compiletest/src/directives.rs index 4e99ab99f6059..90aba6a30137d 100644 --- a/src/tools/compiletest/src/directives.rs +++ b/src/tools/compiletest/src/directives.rs @@ -443,23 +443,39 @@ impl TestProps { } } -pub(crate) fn do_early_directives_check( +pub(crate) fn validate_name( mode: TestMode, file_directives: &FileDirectives<'_>, ) -> Result<(), String> { let testfile = file_directives.path; - for directive_line @ DirectiveLine { line_number, .. } in &file_directives.lines { - let CheckDirectiveResult { is_known_directive, trailing_directive } = - check_directive(directive_line, mode); + for directive_line @ DirectiveLine { name, line_number, .. } in &file_directives.lines { + let is_known_directive = KNOWN_DIRECTIVE_NAMES_SET.contains(name) + || match mode { + TestMode::RustdocHtml => KNOWN_HTMLDOCCK_DIRECTIVE_NAMES.contains(name), + TestMode::RustdocJson => KNOWN_JSONDOCCK_DIRECTIVE_NAMES.contains(name), + _ => false, + }; if !is_known_directive { + // FIXME(Zalathar): Consider emitting specialized error/help messages for + // bogus directive names that are similar to real ones, e.g.: + // - *`compiler-flags` => `compile-flags` + // - *`compile-fail` => `check-fail` or `build-fail` return Err(format!( "ERROR: unknown compiletest directive `{directive}` at {testfile}:{line_number}", directive = directive_line.display(), )); } + // If it looks like the user tried to put two directives on the same line + // (e.g. `//@ only-linux only-x86_64`), signal an error, because the + // second "directive" would actually be ignored with no effect. + let trailing_directive = directive_line + .remark_after_space() + .map(|remark| remark.trim_start().split(' ').next().unwrap()) + .filter(|token| KNOWN_DIRECTIVE_NAMES_SET.contains(token)); + if let Some(trailing_directive) = &trailing_directive { return Err(format!( "ERROR: detected trailing compiletest directive `{trailing_directive}` at {testfile}:{line_number}\n\ @@ -471,40 +487,6 @@ pub(crate) fn do_early_directives_check( Ok(()) } -pub(crate) struct CheckDirectiveResult<'ln> { - is_known_directive: bool, - trailing_directive: Option<&'ln str>, -} - -fn check_directive<'a>( - directive_ln: &DirectiveLine<'a>, - mode: TestMode, -) -> CheckDirectiveResult<'a> { - let &DirectiveLine { name: directive_name, .. } = directive_ln; - - let is_known_directive = KNOWN_DIRECTIVE_NAMES_SET.contains(&directive_name) - || match mode { - TestMode::RustdocHtml => KNOWN_HTMLDOCCK_DIRECTIVE_NAMES.contains(&directive_name), - TestMode::RustdocJson => KNOWN_JSONDOCCK_DIRECTIVE_NAMES.contains(&directive_name), - _ => false, - }; - - // If it looks like the user tried to put two directives on the same line - // (e.g. `//@ only-linux only-x86_64`), signal an error, because the - // second "directive" would actually be ignored with no effect. - let trailing_directive = directive_ln - .remark_after_space() - .map(|remark| remark.trim_start().split(' ').next().unwrap()) - .filter(|token| KNOWN_DIRECTIVE_NAMES_SET.contains(token)); - - // FIXME(Zalathar): Consider emitting specialized error/help messages for - // bogus directive names that are similar to real ones, e.g.: - // - *`compiler-flags` => `compile-flags` - // - *`compile-fail` => `check-fail` or `build-fail` - - CheckDirectiveResult { is_known_directive, trailing_directive } -} - fn iter_directives( config: &Config, file_directives: &FileDirectives<'_>, diff --git a/src/tools/compiletest/src/directives/tests.rs b/src/tools/compiletest/src/directives/tests.rs index 03900706e042f..fcb548d648c4a 100644 --- a/src/tools/compiletest/src/directives/tests.rs +++ b/src/tools/compiletest/src/directives/tests.rs @@ -985,77 +985,68 @@ fn threads_support() { } } -fn run_path(poisoned: &mut bool, path: &Utf8Path, file_contents: &str) { +fn parse_directives_and_validate_names(path: &Utf8Path, file_contents: &str) -> Result<(), ()> { let file_directives = FileDirectives::from_file_contents(path, file_contents); - let result = directives::do_early_directives_check(TestMode::Ui, &file_directives); - if result.is_err() { - *poisoned = true; - } + directives::validate_name(TestMode::Ui, &file_directives).map_err(drop) } #[test] fn test_unknown_directive_check() { - let mut poisoned = false; - run_path( - &mut poisoned, + let result = parse_directives_and_validate_names( Utf8Path::new("a.rs"), include_str!("./test-auxillary/unknown_directive.rs"), ); - assert!(poisoned); + assert!(result.is_err()); } #[test] fn test_known_directive_check_no_error() { - let mut poisoned = false; - run_path( - &mut poisoned, + let result = parse_directives_and_validate_names( Utf8Path::new("a.rs"), include_str!("./test-auxillary/known_directive.rs"), ); - assert!(!poisoned); + assert!(result.is_ok()); } #[test] fn test_error_annotation_no_error() { - let mut poisoned = false; - run_path( - &mut poisoned, + let result = parse_directives_and_validate_names( Utf8Path::new("a.rs"), include_str!("./test-auxillary/error_annotation.rs"), ); - assert!(!poisoned); + assert!(result.is_ok()); } #[test] fn test_non_rs_unknown_directive_not_checked() { - let mut poisoned = false; - run_path( - &mut poisoned, + let result = parse_directives_and_validate_names( Utf8Path::new("a.Makefile"), include_str!("./test-auxillary/not_rs.Makefile"), ); - assert!(!poisoned); + assert!(result.is_ok()); } #[test] fn test_trailing_directive() { - let mut poisoned = false; - run_path(&mut poisoned, Utf8Path::new("a.rs"), "//@ only-x86 only-arm"); - assert!(poisoned); + let result = + parse_directives_and_validate_names(Utf8Path::new("a.rs"), "//@ only-x86 only-arm"); + assert!(result.is_err()); } #[test] fn test_trailing_directive_with_comment() { - let mut poisoned = false; - run_path(&mut poisoned, Utf8Path::new("a.rs"), "//@ only-x86 only-arm with comment"); - assert!(poisoned); + let result = parse_directives_and_validate_names( + Utf8Path::new("a.rs"), + "//@ only-x86 only-arm with comment", + ); + assert!(result.is_err()); } #[test] fn test_not_trailing_directive() { - let mut poisoned = false; - run_path(&mut poisoned, Utf8Path::new("a.rs"), "//@ revisions: incremental"); - assert!(!poisoned); + let result = + parse_directives_and_validate_names(Utf8Path::new("a.rs"), "//@ revisions: incremental"); + assert!(result.is_ok()); } #[test] diff --git a/src/tools/compiletest/src/lib.rs b/src/tools/compiletest/src/lib.rs index 8b5ec7c3ed531..fa4561746e72c 100644 --- a/src/tools/compiletest/src/lib.rs +++ b/src/tools/compiletest/src/lib.rs @@ -415,7 +415,7 @@ fn make_test(cx: &TestCollectorCx, collector: &mut TestCollector, testpaths: &Te fs::read_to_string(&test_path).expect("reading test file for directives should succeed"); let file_directives = FileDirectives::from_file_contents(&test_path, &file_contents); - if let Err(message) = directives::do_early_directives_check(cx.config.mode, &file_directives) { + if let Err(message) = directives::validate_name(cx.config.mode, &file_directives) { // FIXME(Zalathar): Overhaul compiletest error handling so that we // don't have to resort to ad-hoc panics everywhere. panic!("directives check failed:\n{message}"); From 9957581bb7e0ede790dfae563da0333d648d0edd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Le=C3=B3n=20Orell=20Valerian=20Liehr?= Date: Sun, 8 Nov 2026 15:21:22 +0100 Subject: [PATCH 2/7] compiletest: Slightly improve error handling of `validate_name` --- src/tools/compiletest/src/directives.rs | 75 ++++++++++++---- src/tools/compiletest/src/directives/line.rs | 10 +-- src/tools/compiletest/src/directives/tests.rs | 87 ++++++++++++++++--- src/tools/compiletest/src/lib.rs | 7 +- 4 files changed, 142 insertions(+), 37 deletions(-) diff --git a/src/tools/compiletest/src/directives.rs b/src/tools/compiletest/src/directives.rs index 90aba6a30137d..3c6d32307d833 100644 --- a/src/tools/compiletest/src/directives.rs +++ b/src/tools/compiletest/src/directives.rs @@ -443,11 +443,12 @@ impl TestProps { } } -pub(crate) fn validate_name( +pub(crate) fn validate_name<'a>( mode: TestMode, - file_directives: &FileDirectives<'_>, -) -> Result<(), String> { - let testfile = file_directives.path; + file_directives: &FileDirectives<'a>, +) -> Result<(), NameValidationError<'a>> { + use NameValidationError as Error; + use NameValidationErrorKind as ErrorKind; for directive_line @ DirectiveLine { name, line_number, .. } in &file_directives.lines { let is_known_directive = KNOWN_DIRECTIVE_NAMES_SET.contains(name) @@ -458,14 +459,11 @@ pub(crate) fn validate_name( }; if !is_known_directive { - // FIXME(Zalathar): Consider emitting specialized error/help messages for - // bogus directive names that are similar to real ones, e.g.: - // - *`compiler-flags` => `compile-flags` - // - *`compile-fail` => `check-fail` or `build-fail` - return Err(format!( - "ERROR: unknown compiletest directive `{directive}` at {testfile}:{line_number}", - directive = directive_line.display(), - )); + return Err(Error { + kind: ErrorKind::UnknownDirective { name }, + file_path: file_directives.path, + line_number: *line_number, + }); } // If it looks like the user tried to put two directives on the same line @@ -476,17 +474,60 @@ pub(crate) fn validate_name( .map(|remark| remark.trim_start().split(' ').next().unwrap()) .filter(|token| KNOWN_DIRECTIVE_NAMES_SET.contains(token)); - if let Some(trailing_directive) = &trailing_directive { - return Err(format!( - "ERROR: detected trailing compiletest directive `{trailing_directive}` at {testfile}:{line_number}\n\ - HELP: put the directive on its own line: `//@ {trailing_directive}`" - )); + if let Some(trailing_directive) = trailing_directive { + return Err(Error { + kind: ErrorKind::TrailingDirective { leading_directive: name, trailing_directive }, + file_path: file_directives.path, + line_number: *line_number, + }); } } Ok(()) } +#[cfg_attr(test, derive(Debug))] +pub(crate) struct NameValidationError<'a> { + kind: NameValidationErrorKind<'a>, + file_path: &'a Utf8Path, + line_number: LineNumber, +} + +#[cfg_attr(test, derive(Debug))] +enum NameValidationErrorKind<'a> { + UnknownDirective { name: &'a str }, + TrailingDirective { leading_directive: &'a str, trailing_directive: &'a str }, +} + +impl NameValidationError<'_> { + // FIXME: Just report the error, poison the context and keep going. + pub(crate) fn raise_fatal(self) -> ! { + let Self { kind, file_path, line_number } = self; + + let title = |message| crate::error!("{message} at {file_path}:{line_number}"); + + match kind { + NameValidationErrorKind::UnknownDirective { name } => { + title(format!("unknown directive `{name}`")); + // FIXME: Look for typo'ed names / suggest directives with a similar name. E.g, + // - *`compiler-flags` => `compile-flags` + // - *`compile-fail` => `check-fail` or `build-fail` + } + NameValidationErrorKind::TrailingDirective { + leading_directive, + trailing_directive, + } => { + title(format!( + "directive `{leading_directive}` is followed by another directive (`{trailing_directive}`) on the same line" + )); + crate::help!("put the directive on its own line: `//@ {trailing_directive}`"); + } + } + + panic!("fatal error") + } +} + fn iter_directives( config: &Config, file_directives: &FileDirectives<'_>, diff --git a/src/tools/compiletest/src/directives/line.rs b/src/tools/compiletest/src/directives/line.rs index 8c3fe05b6ef7c..31c8e2a7317ab 100644 --- a/src/tools/compiletest/src/directives/line.rs +++ b/src/tools/compiletest/src/directives/line.rs @@ -158,15 +158,15 @@ impl Error<'_> { pub(crate) fn raise_fatal(self) -> ! { let Self { kind, file_path, line_number } = self; - let error = |message| error!("malformed directive: {message} at {file_path}:{line_number}"); + let title = |message| error!("malformed directive: {message} at {file_path}:{line_number}"); match kind { ErrorKind::LeadingRevisionCondition { revision, context } => { - error("found revision condition before `@` instead of after"); + title("found revision condition before `@` instead of after"); help!("rewrite the start of the line to `//@[{revision}] ` (from `{context}`)"); } ErrorKind::WhitespaceBeforeAtSign => { - error("found whitespace between `//` and `@`"); + title("found whitespace between `//` and `@`"); help!("if you meant to write a directive remove the whitespace to form `//@`"); help!( "if you meant to literally write `@` in a code comment add `\\` before `@` or \ @@ -174,10 +174,10 @@ impl Error<'_> { ); } ErrorKind::UnclosedRevisionCondition => { - error("revision condition is missing a closing bracket `]`"); + title("revision condition is missing a closing bracket `]`"); } ErrorKind::CommaInRevisionCondition { revision, directive } => { - error( + title( "found commas in revision condition but multiple revisions aren't supported yet", ); diff --git a/src/tools/compiletest/src/directives/tests.rs b/src/tools/compiletest/src/directives/tests.rs index fcb548d648c4a..f9779ad25b7c1 100644 --- a/src/tools/compiletest/src/directives/tests.rs +++ b/src/tools/compiletest/src/directives/tests.rs @@ -8,8 +8,9 @@ use crate::common::{Config, Debugger, TestMode}; use crate::directives::line::{DirectiveLine, Error, ErrorKind}; use crate::directives::{ self, AuxProps, DIRECTIVE_HANDLERS_MAP, DirectivesCache, EarlyProps, Edition, EditionRange, - FileDirectives, KNOWN_DIRECTIVE_NAMES_SET, LineNumber, extract_llvm_version, - extract_version_range, line_directive, parse_edition, parse_normalize_rule, + FileDirectives, KNOWN_DIRECTIVE_NAMES_SET, LineNumber, NameValidationError, + NameValidationErrorKind, extract_llvm_version, extract_version_range, line_directive, + parse_edition, parse_normalize_rule, }; use crate::executor::{CollectedTestDesc, ShouldFail, TestVariant}; @@ -985,9 +986,13 @@ fn threads_support() { } } -fn parse_directives_and_validate_names(path: &Utf8Path, file_contents: &str) -> Result<(), ()> { +fn parse_directives_and_validate_names<'a>( + path: &'a Utf8Path, + file_contents: &'a str, +) -> Result>, NameValidationError<'a>> { let file_directives = FileDirectives::from_file_contents(path, file_contents); - directives::validate_name(TestMode::Ui, &file_directives).map_err(drop) + directives::validate_name(TestMode::Ui, &file_directives)?; + Ok(file_directives.lines) } #[test] @@ -996,7 +1001,13 @@ fn test_unknown_directive_check() { Utf8Path::new("a.rs"), include_str!("./test-auxillary/unknown_directive.rs"), ); - assert!(result.is_err()); + assert_matches!( + result, + Err(NameValidationError { + kind: NameValidationErrorKind::UnknownDirective { name: "needs-headpat" }, + .. + }) + ); } #[test] @@ -1005,7 +1016,13 @@ fn test_known_directive_check_no_error() { Utf8Path::new("a.rs"), include_str!("./test-auxillary/known_directive.rs"), ); - assert!(result.is_ok()); + assert_matches!( + result.as_deref(), + Ok([ + DirectiveLine { revision: None, raw_directive: "ignore-wasm", name: "ignore-wasm", .. }, + DirectiveLine { revision: None, raw_directive: "check-pass", name: "check-pass", .. }, + ]) + ); } #[test] @@ -1014,7 +1031,12 @@ fn test_error_annotation_no_error() { Utf8Path::new("a.rs"), include_str!("./test-auxillary/error_annotation.rs"), ); - assert!(result.is_ok()); + assert_matches!( + result.as_deref(), + Ok( + [DirectiveLine { revision: None, raw_directive: "check-pass", name: "check-pass", .. },] + ) + ); } #[test] @@ -1023,14 +1045,23 @@ fn test_non_rs_unknown_directive_not_checked() { Utf8Path::new("a.Makefile"), include_str!("./test-auxillary/not_rs.Makefile"), ); - assert!(result.is_ok()); + assert_matches!(result.as_deref(), Ok([])); } #[test] fn test_trailing_directive() { let result = parse_directives_and_validate_names(Utf8Path::new("a.rs"), "//@ only-x86 only-arm"); - assert!(result.is_err()); + assert_matches!( + result, + Err(NameValidationError { + kind: NameValidationErrorKind::TrailingDirective { + leading_directive: "only-x86", + trailing_directive: "only-arm", + }, + .. + }) + ); } #[test] @@ -1039,14 +1070,48 @@ fn test_trailing_directive_with_comment() { Utf8Path::new("a.rs"), "//@ only-x86 only-arm with comment", ); - assert!(result.is_err()); + assert_matches!( + result, + Err(NameValidationError { + kind: NameValidationErrorKind::TrailingDirective { + leading_directive: "only-x86", + // FIXME(fmease): This should also contain "with comment". + trailing_directive: "only-arm", + }, + .. + }) + ); +} + +#[test] +fn test_trailing_directive_with_colon() { + // FIXME(fmease): We should reject this, too! + let result = + parse_directives_and_validate_names(Utf8Path::new("a.rs"), "//@ check-pass edition: 2015"); + assert_matches!( + result.as_deref(), + Ok([DirectiveLine { + revision: None, + raw_directive: "check-pass edition: 2015", + name: "check-pass", + .. + },]) + ); } #[test] fn test_not_trailing_directive() { let result = parse_directives_and_validate_names(Utf8Path::new("a.rs"), "//@ revisions: incremental"); - assert!(result.is_ok()); + assert_matches!( + result.as_deref(), + Ok([DirectiveLine { + revision: None, + raw_directive: "revisions: incremental", + name: "revisions", + .. + },]) + ); } #[test] diff --git a/src/tools/compiletest/src/lib.rs b/src/tools/compiletest/src/lib.rs index fa4561746e72c..b91ad33b53128 100644 --- a/src/tools/compiletest/src/lib.rs +++ b/src/tools/compiletest/src/lib.rs @@ -415,11 +415,10 @@ fn make_test(cx: &TestCollectorCx, collector: &mut TestCollector, testpaths: &Te fs::read_to_string(&test_path).expect("reading test file for directives should succeed"); let file_directives = FileDirectives::from_file_contents(&test_path, &file_contents); - if let Err(message) = directives::validate_name(cx.config.mode, &file_directives) { - // FIXME(Zalathar): Overhaul compiletest error handling so that we - // don't have to resort to ad-hoc panics everywhere. - panic!("directives check failed:\n{message}"); + if let Err(e) = directives::validate_name(cx.config.mode, &file_directives) { + e.raise_fatal(); } + let early_props = EarlyProps::from_file_directives(&cx.config, &file_directives); // Normally we create one structure per revision, with two exceptions: From 85258b0dc103b7e26673ff3deec6a6ce63b76846 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Le=C3=B3n=20Orell=20Valerian=20Liehr?= Date: Sun, 8 Nov 2026 15:26:16 +0100 Subject: [PATCH 3/7] compiletest: Detect trailing key-value directives --- src/tools/compiletest/src/directives.rs | 24 ++++++++++--------- src/tools/compiletest/src/directives/tests.rs | 17 +++++++------ 2 files changed, 21 insertions(+), 20 deletions(-) diff --git a/src/tools/compiletest/src/directives.rs b/src/tools/compiletest/src/directives.rs index 3c6d32307d833..c4e5bb5cc583e 100644 --- a/src/tools/compiletest/src/directives.rs +++ b/src/tools/compiletest/src/directives.rs @@ -469,17 +469,19 @@ pub(crate) fn validate_name<'a>( // If it looks like the user tried to put two directives on the same line // (e.g. `//@ only-linux only-x86_64`), signal an error, because the // second "directive" would actually be ignored with no effect. - let trailing_directive = directive_line - .remark_after_space() - .map(|remark| remark.trim_start().split(' ').next().unwrap()) - .filter(|token| KNOWN_DIRECTIVE_NAMES_SET.contains(token)); - - if let Some(trailing_directive) = trailing_directive { - return Err(Error { - kind: ErrorKind::TrailingDirective { leading_directive: name, trailing_directive }, - file_path: file_directives.path, - line_number: *line_number, - }); + if let Some(remark) = directive_line.remark_after_space() { + let remark = remark.trim_start(); + let token = remark.split([':', ' ']).next().unwrap(); + if KNOWN_DIRECTIVE_NAMES_SET.contains(token) { + return Err(Error { + kind: ErrorKind::TrailingDirective { + leading_directive: name, + trailing_directive: remark, + }, + file_path: file_directives.path, + line_number: *line_number, + }); + } } } diff --git a/src/tools/compiletest/src/directives/tests.rs b/src/tools/compiletest/src/directives/tests.rs index f9779ad25b7c1..565dd5a60207a 100644 --- a/src/tools/compiletest/src/directives/tests.rs +++ b/src/tools/compiletest/src/directives/tests.rs @@ -1075,8 +1075,7 @@ fn test_trailing_directive_with_comment() { Err(NameValidationError { kind: NameValidationErrorKind::TrailingDirective { leading_directive: "only-x86", - // FIXME(fmease): This should also contain "with comment". - trailing_directive: "only-arm", + trailing_directive: "only-arm with comment", }, .. }) @@ -1085,17 +1084,17 @@ fn test_trailing_directive_with_comment() { #[test] fn test_trailing_directive_with_colon() { - // FIXME(fmease): We should reject this, too! let result = parse_directives_and_validate_names(Utf8Path::new("a.rs"), "//@ check-pass edition: 2015"); assert_matches!( - result.as_deref(), - Ok([DirectiveLine { - revision: None, - raw_directive: "check-pass edition: 2015", - name: "check-pass", + result, + Err(NameValidationError { + kind: NameValidationErrorKind::TrailingDirective { + leading_directive: "check-pass", + trailing_directive: "edition: 2015", + }, .. - },]) + }) ); } From eed25c5921496871b084f1426d15e9e5a4358550 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Le=C3=B3n=20Orell=20Valerian=20Liehr?= Date: Sun, 8 Nov 2026 14:05:40 +0100 Subject: [PATCH 4/7] compiletest: Don't pass synthetic directives through `line_directive` --- src/tools/compiletest/src/directives.rs | 49 +++++--------------- src/tools/compiletest/src/directives/line.rs | 4 ++ 2 files changed, 16 insertions(+), 37 deletions(-) diff --git a/src/tools/compiletest/src/directives.rs b/src/tools/compiletest/src/directives.rs index c4e5bb5cc583e..d74baded0e5a2 100644 --- a/src/tools/compiletest/src/directives.rs +++ b/src/tools/compiletest/src/directives.rs @@ -535,50 +535,25 @@ fn iter_directives( file_directives: &FileDirectives<'_>, it: &mut dyn FnMut(&DirectiveLine<'_>), ) { - let testfile = file_directives.path; - - let extra_directives = match config.mode { + // Inject certain directives depending on the test mode, so test writers don't need to specify + // then manually in every test file. + let synthetic = |name| DirectiveLine::synthetic(name, file_directives.path); + match config.mode { TestMode::CoverageRun => { - // Coverage tests in coverage-run mode always have these extra directives, without needing to - // specify them manually in every test file. - // // FIXME(jieyouxu): I feel like there's a better way to do this, leaving for later. - vec![ - "//@ needs-profiler-runtime", - // FIXME(pietroalbini): this test currently does not work on cross-compiled targets - // because remote-test is not capable of sending back the *.profraw files generated by - // the LLVM instrumentation. - "//@ ignore-cross-compile", - ] + it(&synthetic("needs-profiler-runtime")); + // FIXME(pietroalbini): These tests currently do not work on cross-compiled targets + // because remote-test is not capable of sending back the *.profraw files generated by + // the LLVM instrumentation. + it(&synthetic("ignore-cross-compile")); } TestMode::Codegen if !file_directives.has_explicit_no_std_core_attribute => { - // Note: affects all codegen test suites under test mode `codegen`, e.g. `codegen-llvm`. - // - // Codegen tests automatically receive implied `//@ needs-target-std`, unless - // `#![no_std]`/`#![no_core]` attribute was explicitly seen. The rationale is basically to avoid - // having to manually maintain a bunch of `//@ needs-target-std` directives esp. for targets - // tested/built out-of-tree. - vec!["//@ needs-target-std"] + it(&synthetic("needs-target-std")); } TestMode::Ui if config.parallel_frontend_enabled() => { - // UI tests in parallel-frontend mode always have this extra directive, without needing to - // specify it manually in every test file. - vec!["//@ compare-output-by-lines"] - } - - _ => { - // No extra directives for other test modes. - vec![] - } - }; - - for directive_str in extra_directives { - if let Some(Ok(directive_line)) = line_directive(testfile, LineNumber::ZERO, directive_str) - { - it(&directive_line); - } else { - panic!("bad extra-directive line: {directive_str:?}") + it(&synthetic("compare-output-by-lines")); } + _ => {} } for directive_line in &file_directives.lines { diff --git a/src/tools/compiletest/src/directives/line.rs b/src/tools/compiletest/src/directives/line.rs index 31c8e2a7317ab..777a6fa3212ba 100644 --- a/src/tools/compiletest/src/directives/line.rs +++ b/src/tools/compiletest/src/directives/line.rs @@ -104,6 +104,10 @@ pub(crate) struct DirectiveLine<'a> { } impl<'ln> DirectiveLine<'ln> { + pub(crate) fn synthetic(name: &'static str, file_path: &'ln Utf8Path) -> Self { + Self { file_path, line_number: LineNumber::ZERO, revision: None, raw_directive: name, name } + } + pub(crate) fn applies_to_test_revision(&self, test_revision: Option<&str>) -> bool { self.revision.is_none() || self.revision == test_revision } From 710ed9c4dc441890886f5665eaf3c47a231919f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Le=C3=B3n=20Orell=20Valerian=20Liehr?= Date: Sun, 8 Nov 2026 16:40:06 +0100 Subject: [PATCH 5/7] compiletest: Separate the payload from the name in `DirectiveLine` --- src/tools/compiletest/src/directives/line.rs | 97 +++++++++++-------- src/tools/compiletest/src/directives/tests.rs | 21 ++-- 2 files changed, 68 insertions(+), 50 deletions(-) diff --git a/src/tools/compiletest/src/directives/line.rs b/src/tools/compiletest/src/directives/line.rs index 777a6fa3212ba..05892ac8821e9 100644 --- a/src/tools/compiletest/src/directives/line.rs +++ b/src/tools/compiletest/src/directives/line.rs @@ -23,10 +23,10 @@ pub(crate) fn line_directive<'a>( .map(|(revision, after_close_bracket)| (Some(revision), after_close_bracket.trim_start())) .unwrap_or((None, trimmed_comment)); - let mut raw_directive = trimmed_comment.strip_prefix("@")?.trim_start(); + let mut directive = trimmed_comment.strip_prefix("@")?.trim_start(); if let Some(revision) = leading_revision { - let context = &line[..line.substr_range(raw_directive).unwrap().start]; + let context = &line[..line.substr_range(directive).unwrap().start]; return error(ErrorKind::LeadingRevisionCondition { revision, context }); } @@ -36,26 +36,36 @@ pub(crate) fn line_directive<'a>( let mut revision = None; - if let Some(after_open_bracket) = raw_directive.strip_prefix('[') { + if let Some(after_open_bracket) = directive.strip_prefix('[') { let Some((line_revision, after_close_bracket)) = after_open_bracket.split_once(']') else { return error(ErrorKind::UnclosedRevisionCondition); }; revision = Some(line_revision); - raw_directive = after_close_bracket.trim_start(); + directive = after_close_bracket.trim_start(); if line_revision.contains(",") { return error(ErrorKind::CommaInRevisionCondition { revision: line_revision, - directive: raw_directive, + directive, }); } } - // The directive name ends at the first occurrence of colon, space, or end-of-string. - let name = raw_directive.split([':', ' ']).next().expect("split is never empty"); - - Some(Ok(DirectiveLine { file_path, line_number, revision, raw_directive, name })) + let mut kind: fn(_) -> _ = DirectivePayload::Value; + let (name, payload) = directive + .split_once(|char| { + match char { + ' ' => kind = DirectivePayload::Remark, + ':' => {} + _ => return false, + }; + true + }) + .map(|(name, payload)| (name, Some(kind(payload)))) + .unwrap_or((directive, None)); + + Some(Ok(DirectiveLine { file_path, line_number, revision, name, payload })) } /// The (partly) broken-down contents of a line containing a test directive, @@ -82,66 +92,71 @@ pub(crate) struct DirectiveLine<'a> { pub(crate) file_path: &'a Utf8Path, pub(crate) line_number: LineNumber, - /// Some test directives start with a revision name in square brackets - /// (e.g. `[foo]`), and only apply to that revision of the test. - /// If present, this field contains the revision name (e.g. `foo`). - pub(crate) revision: Option<&'a str>, - - /// The main part of the directive, after removing the comment prefix - /// and the optional revision specifier. + /// The revision condition. /// - /// This is "raw" because the directive's name and colon-separated value - /// (if present) have not yet been extracted or checked. - #[cfg(not(test))] - raw_directive: &'a str, - #[cfg(test)] - pub(crate) raw_directive: &'a str, + /// The directive only applies to the given revision of a test. + pub(crate) revision: Option<&'a str>, /// Name of the directive. - /// - /// Invariant: `self.raw_directive.starts_with(self.name)` pub(crate) name: &'a str, + + /// The playload of the directive. + /// + /// If the payload was separated using a colon (`:`) it will get represented as a + /// [`DirectivePayload::Value`]. If the payload was separated using a space, it will be + /// represented as a [`DirectivePayload::Remark`]. If the directive doesn't contain either colon + /// or space, the payload will be `None`. + pub(crate) payload: Option>, } impl<'ln> DirectiveLine<'ln> { pub(crate) fn synthetic(name: &'static str, file_path: &'ln Utf8Path) -> Self { - Self { file_path, line_number: LineNumber::ZERO, revision: None, raw_directive: name, name } + Self { file_path, line_number: LineNumber::ZERO, revision: None, name, payload: None } } pub(crate) fn applies_to_test_revision(&self, test_revision: Option<&str>) -> bool { self.revision.is_none() || self.revision == test_revision } - /// Helper method used by `value_after_colon` and `remark_after_space`. - /// Don't call this directly. - fn rest_after_separator(&self, separator: u8) -> Option<&'ln str> { - let n = self.name.len(); - if self.raw_directive.as_bytes().get(n) != Some(&separator) { - return None; - } - - Some(&self.raw_directive[n + 1..]) - } - /// If this directive uses `name: value` syntax, returns the part after /// the colon character. pub(crate) fn value_after_colon(&self) -> Option<&'ln str> { - self.rest_after_separator(b':') + match self.payload { + Some(DirectivePayload::Value(value)) => Some(value), + _ => None, + } } /// If this directive uses `name remark` syntax, returns the part after /// the separating space. pub(crate) fn remark_after_space(&self) -> Option<&'ln str> { - self.rest_after_separator(b' ') + match self.payload { + Some(DirectivePayload::Remark(remark)) => Some(remark), + _ => None, + } } - /// Allows callers to print `raw_directive` if necessary, - /// without accessing the field directly. + /// Display the name & payload of the directive. pub(crate) fn display(&self) -> impl fmt::Display { - self.raw_directive + fmt::from_fn(|f| { + f.write_str(self.name)?; + if let Some(payload) = &self.payload { + match payload { + DirectivePayload::Value(value) => write!(f, ":{value}"), + DirectivePayload::Remark(remark) => write!(f, " {remark}"), + }?; + } + Ok(()) + }) } } +#[cfg_attr(test, derive(Debug))] +pub(crate) enum DirectivePayload<'a> { + Value(&'a str), + Remark(&'a str), +} + #[cfg_attr(test, derive(Debug))] pub(crate) struct Error<'a> { pub(crate) kind: ErrorKind<'a>, diff --git a/src/tools/compiletest/src/directives/tests.rs b/src/tools/compiletest/src/directives/tests.rs index 565dd5a60207a..591f91598f9bc 100644 --- a/src/tools/compiletest/src/directives/tests.rs +++ b/src/tools/compiletest/src/directives/tests.rs @@ -5,7 +5,7 @@ use camino::Utf8Path; use semver::Version; use crate::common::{Config, Debugger, TestMode}; -use crate::directives::line::{DirectiveLine, Error, ErrorKind}; +use crate::directives::line::{DirectiveLine, DirectivePayload, Error, ErrorKind}; use crate::directives::{ self, AuxProps, DIRECTIVE_HANDLERS_MAP, DirectivesCache, EarlyProps, Edition, EditionRange, FileDirectives, KNOWN_DIRECTIVE_NAMES_SET, LineNumber, NameValidationError, @@ -666,7 +666,7 @@ fn test_ok_directive() { Some(Ok(DirectiveLine { revision: None, name: "aux-build", - raw_directive: "aux-build: file.rs", + payload: Some(DirectivePayload::Value(" file.rs")), .. })) ); @@ -679,7 +679,12 @@ fn indented_directive() { let result = line_directive(Utf8Path::new("file.rs"), LineNumber::ZERO, line); assert_matches!( result, - Some(Ok(DirectiveLine { revision: None, name: "key", raw_directive: "key: value", .. })) + Some(Ok(DirectiveLine { + revision: None, + name: "key", + payload: Some(DirectivePayload::Value(" value")), + .. + })) ); } @@ -1019,8 +1024,8 @@ fn test_known_directive_check_no_error() { assert_matches!( result.as_deref(), Ok([ - DirectiveLine { revision: None, raw_directive: "ignore-wasm", name: "ignore-wasm", .. }, - DirectiveLine { revision: None, raw_directive: "check-pass", name: "check-pass", .. }, + DirectiveLine { revision: None, name: "ignore-wasm", payload: None, .. }, + DirectiveLine { revision: None, name: "check-pass", payload: None, .. }, ]) ); } @@ -1033,9 +1038,7 @@ fn test_error_annotation_no_error() { ); assert_matches!( result.as_deref(), - Ok( - [DirectiveLine { revision: None, raw_directive: "check-pass", name: "check-pass", .. },] - ) + Ok([DirectiveLine { revision: None, name: "check-pass", payload: None, .. },]) ); } @@ -1106,8 +1109,8 @@ fn test_not_trailing_directive() { result.as_deref(), Ok([DirectiveLine { revision: None, - raw_directive: "revisions: incremental", name: "revisions", + payload: Some(DirectivePayload::Value(" incremental")), .. },]) ); From 9917b49452ecff9edc1c6d334f28e564e1389794 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Le=C3=B3n=20Orell=20Valerian=20Liehr?= Date: Sun, 8 Nov 2026 17:11:08 +0100 Subject: [PATCH 6/7] compiletest: Inline `DirectiveLine::{remark_after_space,value_after_colon}` --- src/tools/compiletest/src/directives.rs | 50 ++++++++++++------- src/tools/compiletest/src/directives/cfg.rs | 11 ++-- src/tools/compiletest/src/directives/line.rs | 19 +------ src/tools/compiletest/src/directives/needs.rs | 7 +-- src/tools/compiletest/src/directives/tests.rs | 6 +-- src/tools/compiletest/src/runtest/debugger.rs | 9 +++- 6 files changed, 53 insertions(+), 49 deletions(-) diff --git a/src/tools/compiletest/src/directives.rs b/src/tools/compiletest/src/directives.rs index d74baded0e5a2..836612f763c7d 100644 --- a/src/tools/compiletest/src/directives.rs +++ b/src/tools/compiletest/src/directives.rs @@ -16,7 +16,7 @@ use crate::directives::directive_names::{ }; pub(crate) use crate::directives::file::FileDirectives; use crate::directives::handlers::DIRECTIVE_HANDLERS_MAP; -use crate::directives::line::DirectiveLine; +use crate::directives::line::{DirectiveLine, DirectivePayload}; use crate::directives::needs::PreparedNeedsConditions; use crate::edition::{Edition, parse_edition}; use crate::errors::ErrorKind; @@ -29,8 +29,7 @@ mod cfg; mod directive_names; mod file; mod handlers; -mod line; -pub(crate) use line::line_directive; +pub(crate) mod line; mod line_number; pub(crate) use line_number::LineNumber; mod needs; @@ -469,7 +468,7 @@ pub(crate) fn validate_name<'a>( // If it looks like the user tried to put two directives on the same line // (e.g. `//@ only-linux only-x86_64`), signal an error, because the // second "directive" would actually be ignored with no effect. - if let Some(remark) = directive_line.remark_after_space() { + if let Some(line::DirectivePayload::Remark(remark)) = directive_line.payload { let remark = remark.trim_start(); let token = remark.split([':', ' ']).next().unwrap(); if KNOWN_DIRECTIVE_NAMES_SET.contains(token) { @@ -623,7 +622,7 @@ impl Config { fn parse_pp_exact(&self, line: &DirectiveLine<'_>) -> Option { // Unusually, `//@ pp-exact` can be used with or without a colon, so to avoid a panic // in the parse method we need to make sure there is a colon before calling it. - if line.value_after_colon().is_some() + if let Some(DirectivePayload::Value(_)) = line.payload && let Some(s) = self.parse_name_value_directive(line, "pp-exact") { Some(Utf8PathBuf::from(&s)) @@ -645,8 +644,11 @@ impl Config { _ => return None, }; - let Some((regex, replacement)) = line.value_after_colon().and_then(parse_normalize_rule) - else { + let rule = match line.payload { + Some(DirectivePayload::Value(rule)) => parse_normalize_rule(rule), + _ => None, + }; + let Some((regex, replacement)) = rule else { error!("couldn't parse custom normalization rule: `{}`", line.display()); help!("expected syntax is: `{name}: \"REGEX\" -> \"REPLACEMENT\"`"); panic!("invalid normalization rule detected"); @@ -655,16 +657,18 @@ impl Config { } fn parse_name_directive(&self, line: &DirectiveLine<'_>, directive: &str) -> bool { - if line.name != directive { + let &DirectiveLine { file_path, line_number, name, payload, .. } = line; + + if name != directive { return false; } - if line.value_after_colon().is_some() { - let &DirectiveLine { file_path, line_number, .. } = line; + if let Some(DirectivePayload::Value(_)) = payload { panic!( "{file_path}:{line_number}: directive `{directive}` must not be followed by a colon" ); } + true } @@ -673,15 +677,18 @@ impl Config { line: &DirectiveLine<'_>, directive: &str, ) -> Option { - let &DirectiveLine { file_path, line_number, .. } = line; + let &DirectiveLine { file_path, line_number, payload, .. } = line; if line.name != directive { return None; }; - let value = line.value_after_colon().unwrap_or_else(|| { - panic!("{file_path}:{line_number}: directive `{directive}` must be followed by a colon and value"); - }); + let Some(DirectivePayload::Value(value)) = payload else { + panic!( + "{file_path}:{line_number}: \ + directive `{directive}` must be followed by a colon and value" + ); + }; debug!("{}: {}", directive, value); let value = expand_variables(value.to_owned(), self); @@ -1038,8 +1045,9 @@ fn ignore_cdb(config: &Config, variant: &TestVariant, line: &DirectiveLine<'_>) if let Some(actual_version) = config.cdb_version { if line.name == "min-cdb-version" - && let Some(rest) = line.value_after_colon().map(str::trim) + && let Some(DirectivePayload::Value(rest)) = line.payload { + let rest = rest.trim(); let min_version = extract_cdb_version(rest).unwrap_or_else(|| { panic!("couldn't parse version range: {:?}", rest); }); @@ -1071,8 +1079,9 @@ fn ignore_gdb(config: &Config, variant: &TestVariant, line: &DirectiveLine<'_>) if let Some(actual_version) = config.gdb_version { if line.name == "min-gdb-version" - && let Some(rest) = line.value_after_colon().map(str::trim) + && let Some(DirectivePayload::Value(rest)) = line.payload { + let rest = rest.trim(); let (start_ver, end_ver) = extract_version_range(rest, extract_gdb_version) .unwrap_or_else(|| { panic!("couldn't parse version range: {:?}", rest); @@ -1089,8 +1098,9 @@ fn ignore_gdb(config: &Config, variant: &TestVariant, line: &DirectiveLine<'_>) }; } } else if line.name == "ignore-gdb-version" - && let Some(rest) = line.value_after_colon().map(str::trim) + && let Some(DirectivePayload::Value(rest)) = line.payload { + let rest = rest.trim(); let (min_version, max_version) = extract_version_range(rest, extract_gdb_version) .unwrap_or_else(|| { panic!("couldn't parse version range: {:?}", rest); @@ -1132,9 +1142,10 @@ fn ignore_lldb(config: &Config, variant: &TestVariant, line: &DirectiveLine<'_>) if let Some(actual_version) = &config.lldb_version { match (line.name, actual_version) { ("min-apple-lldb-version", LldbVersion::Apple(vers)) => { - let Some(rest) = line.value_after_colon().map(str::trim) else { + let Some(DirectivePayload::Value(rest)) = line.payload else { return IgnoreDecision::Continue; }; + let rest = rest.trim(); let LldbVersion::Apple(min_vers) = LldbVersion::apple_from_str(rest) else { unreachable!() @@ -1150,9 +1161,10 @@ fn ignore_lldb(config: &Config, variant: &TestVariant, line: &DirectiveLine<'_>) } } ("min-llvm-lldb-version", LldbVersion::Llvm(vers)) => { - let Some(rest) = line.value_after_colon().map(str::trim) else { + let Some(DirectivePayload::Value(rest)) = line.payload else { return IgnoreDecision::Continue; }; + let rest = rest.trim(); let LldbVersion::Llvm(min_vers) = LldbVersion::llvm_from_str(rest) else { unreachable!() diff --git a/src/tools/compiletest/src/directives/cfg.rs b/src/tools/compiletest/src/directives/cfg.rs index 19e08de1d062a..06d0f89032acd 100644 --- a/src/tools/compiletest/src/directives/cfg.rs +++ b/src/tools/compiletest/src/directives/cfg.rs @@ -2,6 +2,7 @@ use std::collections::{HashMap, HashSet}; use std::sync::{Arc, LazyLock}; use crate::common::{CompareMode, Config}; +use crate::directives::line::DirectivePayload; use crate::directives::{DirectiveLine, IgnoreDecision}; const EXTRA_ARCHS: &[&str] = &["spirv"]; @@ -98,10 +99,12 @@ fn parse_cfg_name_directive<'a>( // FIXME(Zalathar): This currently allows either a space or a colon, and // treats any "value" after a colon as though it were a remark. // We should instead forbid the colon syntax for these directives. - let comment = line - .remark_after_space() - .or_else(|| line.value_after_colon()) - .map(|c| c.trim().trim_start_matches('-').trim()); + let comment = match line.payload { + Some(DirectivePayload::Value(comment) | DirectivePayload::Remark(comment)) => { + Some(comment.trim().trim_start_matches('-').trim()) + } + _ => None, + }; if let Some(cond) = conditions.conds.get(name) { ParsedNameDirective { diff --git a/src/tools/compiletest/src/directives/line.rs b/src/tools/compiletest/src/directives/line.rs index 05892ac8821e9..4aaa7d15a0e9a 100644 --- a/src/tools/compiletest/src/directives/line.rs +++ b/src/tools/compiletest/src/directives/line.rs @@ -118,24 +118,6 @@ impl<'ln> DirectiveLine<'ln> { self.revision.is_none() || self.revision == test_revision } - /// If this directive uses `name: value` syntax, returns the part after - /// the colon character. - pub(crate) fn value_after_colon(&self) -> Option<&'ln str> { - match self.payload { - Some(DirectivePayload::Value(value)) => Some(value), - _ => None, - } - } - - /// If this directive uses `name remark` syntax, returns the part after - /// the separating space. - pub(crate) fn remark_after_space(&self) -> Option<&'ln str> { - match self.payload { - Some(DirectivePayload::Remark(remark)) => Some(remark), - _ => None, - } - } - /// Display the name & payload of the directive. pub(crate) fn display(&self) -> impl fmt::Display { fmt::from_fn(|f| { @@ -151,6 +133,7 @@ impl<'ln> DirectiveLine<'ln> { } } +#[derive(Clone, Copy)] #[cfg_attr(test, derive(Debug))] pub(crate) enum DirectivePayload<'a> { Value(&'a str), diff --git a/src/tools/compiletest/src/directives/needs.rs b/src/tools/compiletest/src/directives/needs.rs index ce67b11fba211..78259cf0b3dad 100644 --- a/src/tools/compiletest/src/directives/needs.rs +++ b/src/tools/compiletest/src/directives/needs.rs @@ -3,6 +3,7 @@ use std::collections::HashMap; use crate::common::{ Config, KNOWN_CRATE_TYPES, KNOWN_TARGET_HAS_ATOMIC_WIDTHS, Sanitizer, query_rustc_output, }; +use crate::directives::line::DirectivePayload; use crate::directives::{DirectiveLine, IgnoreDecision, KNOWN_DIRECTIVE_NAMES_SET}; pub(super) fn handle_needs( @@ -17,7 +18,7 @@ pub(super) fn handle_needs( } if name == "needs-target-has-atomic" { - let Some(rest) = ln.value_after_colon() else { + let Some(DirectivePayload::Value(rest)) = ln.payload else { return IgnoreDecision::Error { message: "expected `needs-target-has-atomic` to have a comma-separated list of atomic widths".to_string(), }; @@ -59,7 +60,7 @@ pub(super) fn handle_needs( // FIXME(jieyouxu): share multi-value directive logic with `needs-target-has-atomic` above. if name == "needs-crate-type" { - let Some(rest) = ln.value_after_colon() else { + let Some(DirectivePayload::Value(rest)) = ln.payload else { return IgnoreDecision::Error { message: "expected `needs-crate-type` to have a comma-separated list of crate types" @@ -111,7 +112,7 @@ pub(super) fn handle_needs( IgnoreDecision::Continue } else { IgnoreDecision::Ignore { - reason: if let Some(comment) = ln.remark_after_space() { + reason: if let Some(DirectivePayload::Remark(comment)) = ln.payload { format!("{} ({})", need.ignore_reason, comment.trim()) } else { need.ignore_reason.into() diff --git a/src/tools/compiletest/src/directives/tests.rs b/src/tools/compiletest/src/directives/tests.rs index 591f91598f9bc..7ca87ddde29de 100644 --- a/src/tools/compiletest/src/directives/tests.rs +++ b/src/tools/compiletest/src/directives/tests.rs @@ -5,12 +5,12 @@ use camino::Utf8Path; use semver::Version; use crate::common::{Config, Debugger, TestMode}; -use crate::directives::line::{DirectiveLine, DirectivePayload, Error, ErrorKind}; +use crate::directives::line::{DirectiveLine, DirectivePayload, Error, ErrorKind, line_directive}; use crate::directives::{ self, AuxProps, DIRECTIVE_HANDLERS_MAP, DirectivesCache, EarlyProps, Edition, EditionRange, FileDirectives, KNOWN_DIRECTIVE_NAMES_SET, LineNumber, NameValidationError, - NameValidationErrorKind, extract_llvm_version, extract_version_range, line_directive, - parse_edition, parse_normalize_rule, + NameValidationErrorKind, extract_llvm_version, extract_version_range, parse_edition, + parse_normalize_rule, }; use crate::executor::{CollectedTestDesc, ShouldFail, TestVariant}; diff --git a/src/tools/compiletest/src/runtest/debugger.rs b/src/tools/compiletest/src/runtest/debugger.rs index d48097bacd556..1355b7ca72b93 100644 --- a/src/tools/compiletest/src/runtest/debugger.rs +++ b/src/tools/compiletest/src/runtest/debugger.rs @@ -4,7 +4,8 @@ use std::io::{BufRead, BufReader}; use camino::{Utf8Path, Utf8PathBuf}; -use crate::directives::{LineNumber, line_directive}; +use crate::directives::LineNumber; +use crate::directives::line::{DirectivePayload, line_directive}; use crate::runtest::ProcRes; /// Representation of information to invoke a debugger and check its output @@ -51,7 +52,11 @@ impl DebuggerCommands { continue; }; - match (directive_kind, directive.value_after_colon()) { + let value = directive.payload.and_then(|payload| match payload { + DirectivePayload::Value(value) => Some(value), + DirectivePayload::Remark(_) => None, + }); + match (directive_kind, value) { ("-command", Some(command)) => commands.push(command.to_string()), ("-check", Some(pattern)) => check_lines.push((line_number, pattern.to_string())), ("-repr", Some(var_name)) => { From 5f8f169c92bfa17081b51c530bd11b29dcb02e66 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Le=C3=B3n=20Orell=20Valerian=20Liehr?= Date: Sun, 8 Nov 2026 17:47:03 +0100 Subject: [PATCH 7/7] compiletest: Auto-trim the payload of directives --- src/tools/compiletest/src/directives.rs | 14 +++---------- .../compiletest/src/directives/auxiliary.rs | 2 +- src/tools/compiletest/src/directives/cfg.rs | 2 +- .../compiletest/src/directives/handlers.rs | 21 ++++++------------- src/tools/compiletest/src/directives/line.rs | 6 +++--- src/tools/compiletest/src/directives/needs.rs | 2 +- src/tools/compiletest/src/directives/tests.rs | 10 ++++----- src/tools/compiletest/src/runtest/debugger.rs | 2 +- 8 files changed, 21 insertions(+), 38 deletions(-) diff --git a/src/tools/compiletest/src/directives.rs b/src/tools/compiletest/src/directives.rs index 836612f763c7d..a9baa9ee968b8 100644 --- a/src/tools/compiletest/src/directives.rs +++ b/src/tools/compiletest/src/directives.rs @@ -469,7 +469,6 @@ pub(crate) fn validate_name<'a>( // (e.g. `//@ only-linux only-x86_64`), signal an error, because the // second "directive" would actually be ignored with no effect. if let Some(line::DirectivePayload::Remark(remark)) = directive_line.payload { - let remark = remark.trim_start(); let token = remark.split([':', ' ']).next().unwrap(); if KNOWN_DIRECTIVE_NAMES_SET.contains(token) { return Err(Error { @@ -706,15 +705,14 @@ impl Config { *value = *value || self.parse_name_directive(line, directive); } - fn set_name_value_directive( + fn set_name_value_directive( &self, line: &DirectiveLine<'_>, directive: &str, - value: &mut Option, - parse: impl FnOnce(String) -> T, + value: &mut Option, ) { if value.is_none() { - *value = self.parse_name_value_directive(line, directive).map(parse); + *value = self.parse_name_value_directive(line, directive); } } @@ -798,7 +796,6 @@ fn parse_normalize_rule(raw_value: &str) -> Option<(String, String)> { let captures = static_regex!( r#"(?x) # (verbose mode regex) ^ - \s* # (leading whitespace) "(?[^"]*)" # "REGEX" \s+->\s+ # -> "(?[^"]*)" # "REPLACEMENT" @@ -1047,7 +1044,6 @@ fn ignore_cdb(config: &Config, variant: &TestVariant, line: &DirectiveLine<'_>) if line.name == "min-cdb-version" && let Some(DirectivePayload::Value(rest)) = line.payload { - let rest = rest.trim(); let min_version = extract_cdb_version(rest).unwrap_or_else(|| { panic!("couldn't parse version range: {:?}", rest); }); @@ -1081,7 +1077,6 @@ fn ignore_gdb(config: &Config, variant: &TestVariant, line: &DirectiveLine<'_>) if line.name == "min-gdb-version" && let Some(DirectivePayload::Value(rest)) = line.payload { - let rest = rest.trim(); let (start_ver, end_ver) = extract_version_range(rest, extract_gdb_version) .unwrap_or_else(|| { panic!("couldn't parse version range: {:?}", rest); @@ -1100,7 +1095,6 @@ fn ignore_gdb(config: &Config, variant: &TestVariant, line: &DirectiveLine<'_>) } else if line.name == "ignore-gdb-version" && let Some(DirectivePayload::Value(rest)) = line.payload { - let rest = rest.trim(); let (min_version, max_version) = extract_version_range(rest, extract_gdb_version) .unwrap_or_else(|| { panic!("couldn't parse version range: {:?}", rest); @@ -1145,7 +1139,6 @@ fn ignore_lldb(config: &Config, variant: &TestVariant, line: &DirectiveLine<'_>) let Some(DirectivePayload::Value(rest)) = line.payload else { return IgnoreDecision::Continue; }; - let rest = rest.trim(); let LldbVersion::Apple(min_vers) = LldbVersion::apple_from_str(rest) else { unreachable!() @@ -1164,7 +1157,6 @@ fn ignore_lldb(config: &Config, variant: &TestVariant, line: &DirectiveLine<'_>) let Some(DirectivePayload::Value(rest)) = line.payload else { return IgnoreDecision::Continue; }; - let rest = rest.trim(); let LldbVersion::Llvm(min_vers) = LldbVersion::llvm_from_str(rest) else { unreachable!() diff --git a/src/tools/compiletest/src/directives/auxiliary.rs b/src/tools/compiletest/src/directives/auxiliary.rs index b88ddc00a535a..d86103464d9a1 100644 --- a/src/tools/compiletest/src/directives/auxiliary.rs +++ b/src/tools/compiletest/src/directives/auxiliary.rs @@ -88,7 +88,7 @@ pub(super) fn parse_and_update_aux( config.push_name_value_directive(ln, PROC_MACRO, &mut aux.proc_macros, parse_proc_macro); if let Some(r) = config.parse_name_value_directive(ln, AUX_CODEGEN_BACKEND) { - aux.codegen_backend = Some(r.trim().to_owned()); + aux.codegen_backend = Some(r.to_owned()); } } diff --git a/src/tools/compiletest/src/directives/cfg.rs b/src/tools/compiletest/src/directives/cfg.rs index 06d0f89032acd..b6679c2ff62f6 100644 --- a/src/tools/compiletest/src/directives/cfg.rs +++ b/src/tools/compiletest/src/directives/cfg.rs @@ -101,7 +101,7 @@ fn parse_cfg_name_directive<'a>( // We should instead forbid the colon syntax for these directives. let comment = match line.payload { Some(DirectivePayload::Value(comment) | DirectivePayload::Remark(comment)) => { - Some(comment.trim().trim_start_matches('-').trim()) + Some(comment.trim_start_matches('-').trim()) } _ => None, }; diff --git a/src/tools/compiletest/src/directives/handlers.rs b/src/tools/compiletest/src/directives/handlers.rs index 0e80d6d865f22..f2688be15ebf5 100644 --- a/src/tools/compiletest/src/directives/handlers.rs +++ b/src/tools/compiletest/src/directives/handlers.rs @@ -184,9 +184,7 @@ fn make_directive_handlers_map() -> HashMap<&'static str, Handler> { config.push_name_value_directive(ln, EXEC_ENV, &mut props.exec_env, Config::parse_env); }), handler(UNSET_EXEC_ENV, |config, ln, props| { - config.push_name_value_directive(ln, UNSET_EXEC_ENV, &mut props.unset_exec_env, |r| { - r.trim().to_owned() - }); + config.push_name_value_directive(ln, UNSET_EXEC_ENV, &mut props.unset_exec_env, |r| r); }), handler(RUSTC_ENV, |config, ln, props| { config.push_name_value_directive( @@ -201,7 +199,7 @@ fn make_directive_handlers_map() -> HashMap<&'static str, Handler> { ln, UNSET_RUSTC_ENV, &mut props.unset_rustc_env, - |r| r.trim().to_owned(), + |r| r, ); }), handler(FORBID_OUTPUT, |config, ln, props| { @@ -252,7 +250,7 @@ fn make_directive_handlers_map() -> HashMap<&'static str, Handler> { handler(FAILURE_STATUS, |config, ln, props| { if let Some(code) = config .parse_name_value_directive(ln, FAILURE_STATUS) - .and_then(|code| code.trim().parse::().ok()) + .and_then(|code| code.parse::().ok()) { props.failure_status = Some(code); } @@ -275,9 +273,7 @@ fn make_directive_handlers_map() -> HashMap<&'static str, Handler> { ); }), handler(ASSEMBLY_OUTPUT, |config, ln, props| { - config.set_name_value_directive(ln, ASSEMBLY_OUTPUT, &mut props.assembly_output, |r| { - r.trim().to_string() - }); + config.set_name_value_directive(ln, ASSEMBLY_OUTPUT, &mut props.assembly_output); }), handler(STDERR_PER_BITWIDTH, |config, ln, props| { config.set_name_directive(ln, STDERR_PER_BITWIDTH, &mut props.stderr_per_bitwidth); @@ -289,7 +285,6 @@ fn make_directive_handlers_map() -> HashMap<&'static str, Handler> { // Unlike the other `name_value_directive`s this needs to be handled manually, // because it sets a `bool` flag. if let Some(known_bug) = config.parse_name_value_directive(ln, KNOWN_BUG) { - let known_bug = known_bug.trim(); if known_bug == "unknown" || known_bug.split(',').all(|issue_ref| { issue_ref @@ -312,9 +307,7 @@ fn make_directive_handlers_map() -> HashMap<&'static str, Handler> { } }), handler(TEST_MIR_PASS, |config, ln, props| { - config.set_name_value_directive(ln, TEST_MIR_PASS, &mut props.mir_unit_test, |s| { - s.trim().to_string() - }); + config.set_name_value_directive(ln, TEST_MIR_PASS, &mut props.mir_unit_test); }), handler(REMAP_SRC_BASE, |config, ln, props| { config.set_name_directive(ln, REMAP_SRC_BASE, &mut props.remap_src_base); @@ -363,9 +356,7 @@ fn make_directive_handlers_map() -> HashMap<&'static str, Handler> { handler(DONT_REQUIRE_ANNOTATIONS, |config, ln, props| { if let Some(err_kind) = config.parse_name_value_directive(ln, DONT_REQUIRE_ANNOTATIONS) { - props - .dont_require_annotations - .insert(ErrorKind::expect_from_user_str(err_kind.trim())); + props.dont_require_annotations.insert(ErrorKind::expect_from_user_str(&err_kind)); } }), handler(DISABLE_GDB_PRETTY_PRINTERS, |config, ln, props| { diff --git a/src/tools/compiletest/src/directives/line.rs b/src/tools/compiletest/src/directives/line.rs index 4aaa7d15a0e9a..48e3ab603a8b3 100644 --- a/src/tools/compiletest/src/directives/line.rs +++ b/src/tools/compiletest/src/directives/line.rs @@ -62,7 +62,7 @@ pub(crate) fn line_directive<'a>( }; true }) - .map(|(name, payload)| (name, Some(kind(payload)))) + .map(|(name, payload)| (name, Some(kind(payload.trim())))) .unwrap_or((directive, None)); Some(Ok(DirectiveLine { file_path, line_number, revision, name, payload })) @@ -100,7 +100,7 @@ pub(crate) struct DirectiveLine<'a> { /// Name of the directive. pub(crate) name: &'a str, - /// The playload of the directive. + /// The (whitespace-trimmed) playload of the directive. /// /// If the payload was separated using a colon (`:`) it will get represented as a /// [`DirectivePayload::Value`]. If the payload was separated using a space, it will be @@ -124,7 +124,7 @@ impl<'ln> DirectiveLine<'ln> { f.write_str(self.name)?; if let Some(payload) = &self.payload { match payload { - DirectivePayload::Value(value) => write!(f, ":{value}"), + DirectivePayload::Value(value) => write!(f, ": {value}"), DirectivePayload::Remark(remark) => write!(f, " {remark}"), }?; } diff --git a/src/tools/compiletest/src/directives/needs.rs b/src/tools/compiletest/src/directives/needs.rs index 78259cf0b3dad..087e9006f8455 100644 --- a/src/tools/compiletest/src/directives/needs.rs +++ b/src/tools/compiletest/src/directives/needs.rs @@ -113,7 +113,7 @@ pub(super) fn handle_needs( } else { IgnoreDecision::Ignore { reason: if let Some(DirectivePayload::Remark(comment)) = ln.payload { - format!("{} ({})", need.ignore_reason, comment.trim()) + format!("{} ({})", need.ignore_reason, comment) } else { need.ignore_reason.into() }, diff --git a/src/tools/compiletest/src/directives/tests.rs b/src/tools/compiletest/src/directives/tests.rs index 7ca87ddde29de..e0be71354d8b3 100644 --- a/src/tools/compiletest/src/directives/tests.rs +++ b/src/tools/compiletest/src/directives/tests.rs @@ -83,7 +83,7 @@ fn test_parse_normalize_rule() { "something (32 bits)", "something ($WORD bits)", ), - (r#" " with whitespace" -> " replacement""#, " with whitespace", " replacement"), + (r#"" with whitespace" -> " replacement""#, " with whitespace", " replacement"), ]; for &(input, expected_regex, expected_replacement) in good_data { @@ -616,7 +616,7 @@ fn test_extract_version_range() { } #[test] -#[should_panic(expected = "duplicate revision: `rpass1` in line ` rpass1 rpass1`")] +#[should_panic(expected = "duplicate revision: `rpass1` in line `rpass1 rpass1`")] fn test_duplicate_revisions() { let config: Config = cfg().build(); parse_early_props(&config, "//@ revisions: rpass1 rpass1"); @@ -666,7 +666,7 @@ fn test_ok_directive() { Some(Ok(DirectiveLine { revision: None, name: "aux-build", - payload: Some(DirectivePayload::Value(" file.rs")), + payload: Some(DirectivePayload::Value("file.rs")), .. })) ); @@ -682,7 +682,7 @@ fn indented_directive() { Some(Ok(DirectiveLine { revision: None, name: "key", - payload: Some(DirectivePayload::Value(" value")), + payload: Some(DirectivePayload::Value("value")), .. })) ); @@ -1110,7 +1110,7 @@ fn test_not_trailing_directive() { Ok([DirectiveLine { revision: None, name: "revisions", - payload: Some(DirectivePayload::Value(" incremental")), + payload: Some(DirectivePayload::Value("incremental")), .. },]) ); diff --git a/src/tools/compiletest/src/runtest/debugger.rs b/src/tools/compiletest/src/runtest/debugger.rs index 1355b7ca72b93..817ff6368ed00 100644 --- a/src/tools/compiletest/src/runtest/debugger.rs +++ b/src/tools/compiletest/src/runtest/debugger.rs @@ -62,7 +62,7 @@ impl DebuggerCommands { ("-repr", Some(var_name)) => { // pseudo-command intercepted by `debugger_tester` to run custom variable // inspection logic. - commands.push(format!("repr {}", var_name.trim())); + commands.push(format!("repr {var_name}")); // Artificially output by `debugger_tester` to confirm that the inspection logic // encountered no errors. check_lines.push((line_number, format!("{var_name}: Ok")));