From 891b8475bd31da09f78f08123347514fbd0491ff Mon Sep 17 00:00:00 2001 From: Zalathar Date: Tue, 21 Oct 2025 16:52:44 +1100 Subject: [PATCH 1/2] Remove a confused use of `revision` from `update_pass_mode` --- src/tools/compiletest/src/directives.rs | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/tools/compiletest/src/directives.rs b/src/tools/compiletest/src/directives.rs index 3bb5933553f12..343ae854b7d5e 100644 --- a/src/tools/compiletest/src/directives.rs +++ b/src/tools/compiletest/src/directives.rs @@ -504,7 +504,7 @@ impl TestProps { &mut self.check_test_line_numbers_match, ); - self.update_pass_mode(ln, test_revision, config); + self.update_pass_mode(ln, config); self.update_fail_mode(ln, config); config.set_name_directive(ln, IGNORE_PASS, &mut self.ignore_pass); @@ -714,18 +714,15 @@ impl TestProps { } } - fn update_pass_mode( - &mut self, - ln: &DirectiveLine<'_>, - revision: Option<&str>, - config: &Config, - ) { + fn update_pass_mode(&mut self, ln: &DirectiveLine<'_>, config: &Config) { let check_no_run = |s| match (config.mode, s) { (TestMode::Ui, _) => (), (TestMode::Crashes, _) => (), (TestMode::Codegen, "build-pass") => (), (TestMode::Incremental, _) => { - if revision.is_some() && !self.revisions.iter().all(|r| r.starts_with("cfail")) { + // FIXME(Zalathar): This only detects forbidden directives that are + // declared _after_ the incompatible `//@ revisions:` directive(s). + if self.revisions.iter().any(|r| !r.starts_with("cfail")) { panic!("`{s}` directive is only supported in `cfail` incremental tests") } } From 860a84c04afb53e2c32e566c7726a377e1f2e9ab Mon Sep 17 00:00:00 2001 From: Zalathar Date: Tue, 21 Oct 2025 17:10:04 +1100 Subject: [PATCH 2/2] Remove bad-directive detection that has been subsumed by the allowlist Now that all directive names are checked against a list of known-good names, these ad-hoc checks can never fire. --- src/tools/compiletest/src/directives.rs | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/tools/compiletest/src/directives.rs b/src/tools/compiletest/src/directives.rs index 343ae854b7d5e..51eac58c971eb 100644 --- a/src/tools/compiletest/src/directives.rs +++ b/src/tools/compiletest/src/directives.rs @@ -256,8 +256,6 @@ mod directives { pub const NO_AUTO_CHECK_CFG: &'static str = "no-auto-check-cfg"; pub const ADD_CORE_STUBS: &'static str = "add-core-stubs"; pub const CORE_STUBS_COMPILE_FLAGS: &'static str = "core-stubs-compile-flags"; - // This isn't a real directive, just one that is probably mistyped often - pub const INCORRECT_COMPILER_FLAGS: &'static str = "compiler-flags"; pub const DISABLE_GDB_PRETTY_PRINTERS: &'static str = "disable-gdb-pretty-printers"; pub const COMPARE_OUTPUT_BY_LINES: &'static str = "compare-output-by-lines"; } @@ -418,9 +416,6 @@ impl TestProps { } self.compile_flags.extend(flags); } - if config.parse_name_value_directive(ln, INCORRECT_COMPILER_FLAGS).is_some() { - panic!("`compiler-flags` directive should be spelled `compile-flags`"); - } if let Some(range) = parse_edition_range(config, ln) { self.edition = Some(range.edition_to_test(config.edition)); @@ -686,9 +681,6 @@ impl TestProps { panic!("`{}-fail` directive is only supported in UI tests", mode); } }; - if config.mode == TestMode::Ui && config.parse_name_directive(ln, "compile-fail") { - panic!("`compile-fail` directive is useless in UI tests"); - } let fail_mode = if config.parse_name_directive(ln, "check-fail") { check_ui("check"); Some(FailMode::Check) @@ -809,6 +801,11 @@ fn check_directive<'a>( .map(|remark| remark.trim_start().split(' ').next().unwrap()) .filter(|token| KNOWN_DIRECTIVE_NAMES.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 } }