diff --git a/src/tools/compiletest/src/directives.rs b/src/tools/compiletest/src/directives.rs index f25294d8d198d..7c1bbe940321f 100644 --- a/src/tools/compiletest/src/directives.rs +++ b/src/tools/compiletest/src/directives.rs @@ -11,7 +11,7 @@ use crate::debuggers::{extract_cdb_version, extract_gdb_version}; pub(crate) use crate::directives::auxiliary::AuxProps; use crate::directives::auxiliary::parse_and_update_aux; use crate::directives::directive_names::{ - KNOWN_DIRECTIVE_NAMES, KNOWN_HTMLDOCCK_DIRECTIVE_NAMES, KNOWN_JSONDOCCK_DIRECTIVE_NAMES, + KNOWN_DIRECTIVE_NAMES_SET, KNOWN_HTMLDOCCK_DIRECTIVE_NAMES, KNOWN_JSONDOCCK_DIRECTIVE_NAMES, }; pub(crate) use crate::directives::file::FileDirectives; use crate::directives::line::{DirectiveLine, line_directive}; @@ -53,13 +53,10 @@ impl EarlyProps { config: &Config, file_directives: &FileDirectives<'_>, ) -> Self { - let testfile = file_directives.path; let mut props = EarlyProps::default(); - let mut poisoned = false; iter_directives( config.mode, - &mut poisoned, file_directives, // (dummy comment to force args into vertical layout) &mut |ln: &DirectiveLine<'_>| { @@ -67,11 +64,6 @@ impl EarlyProps { }, ); - if poisoned { - eprintln!("errors encountered during EarlyProps parsing: {}", testfile); - panic!("errors encountered during EarlyProps parsing"); - } - props } } @@ -358,12 +350,10 @@ impl TestProps { let file_contents = fs::read_to_string(testfile).unwrap(); let file_directives = FileDirectives::from_file_contents(testfile, &file_contents); - let mut poisoned = false; - iter_directives( config.mode, - &mut poisoned, &file_directives, + // (dummy comment to force args into vertical layout) &mut |ln: &DirectiveLine<'_>| { if !ln.applies_to_test_revision(test_revision) { return; @@ -634,11 +624,6 @@ impl TestProps { ); }, ); - - if poisoned { - eprintln!("errors encountered during TestProps parsing: {}", testfile); - panic!("errors encountered during TestProps parsing"); - } } if self.should_ice { @@ -775,6 +760,34 @@ impl TestProps { } } +pub(crate) fn do_early_directives_check( + 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); + + if !is_known_directive { + return Err(format!( + "ERROR: unknown compiletest directive `{directive}` at {testfile}:{line_number}", + directive = directive_line.display(), + )); + } + + 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}`" + )); + } + } + + Ok(()) +} + pub(crate) struct CheckDirectiveResult<'ln> { is_known_directive: bool, trailing_directive: Option<&'ln str>, @@ -786,7 +799,7 @@ fn check_directive<'a>( ) -> CheckDirectiveResult<'a> { let &DirectiveLine { name: directive_name, .. } = directive_ln; - let is_known_directive = KNOWN_DIRECTIVE_NAMES.contains(&directive_name) + let is_known_directive = KNOWN_DIRECTIVE_NAMES_SET.contains(&directive_name) || match mode { TestMode::Rustdoc => KNOWN_HTMLDOCCK_DIRECTIVE_NAMES.contains(&directive_name), TestMode::RustdocJson => KNOWN_JSONDOCCK_DIRECTIVE_NAMES.contains(&directive_name), @@ -799,7 +812,7 @@ fn check_directive<'a>( let trailing_directive = directive_ln .remark_after_space() .map(|remark| remark.trim_start().split(' ').next().unwrap()) - .filter(|token| KNOWN_DIRECTIVE_NAMES.contains(token)); + .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.: @@ -811,7 +824,6 @@ fn check_directive<'a>( fn iter_directives( mode: TestMode, - poisoned: &mut bool, file_directives: &FileDirectives<'_>, it: &mut dyn FnMut(&DirectiveLine<'_>), ) { @@ -837,36 +849,7 @@ fn iter_directives( } } - for directive_line @ &DirectiveLine { line_number, .. } in &file_directives.lines { - // Perform unknown directive check on Rust files. - if testfile.extension() == Some("rs") { - let CheckDirectiveResult { is_known_directive, trailing_directive } = - check_directive(directive_line, mode); - - if !is_known_directive { - *poisoned = true; - - error!( - "{testfile}:{line_number}: detected unknown compiletest test directive `{}`", - directive_line.display(), - ); - - return; - } - - if let Some(trailing_directive) = &trailing_directive { - *poisoned = true; - - error!( - "{testfile}:{line_number}: detected trailing compiletest test directive `{}`", - trailing_directive, - ); - help!("put the trailing directive in its own line: `//@ {}`", trailing_directive); - - return; - } - } - + for directive_line in &file_directives.lines { it(directive_line); } } @@ -1304,12 +1287,9 @@ pub(crate) fn make_test_description( let mut ignore_message = None; let mut should_fail = false; - let mut local_poisoned = false; - // Scan through the test file to handle `ignore-*`, `only-*`, and `needs-*` directives. iter_directives( config.mode, - &mut local_poisoned, file_directives, &mut |ln @ &DirectiveLine { line_number, .. }| { if !ln.applies_to_test_revision(test_revision) { @@ -1358,11 +1338,6 @@ pub(crate) fn make_test_description( }, ); - if local_poisoned { - eprintln!("errors encountered when trying to make test description: {}", path); - panic!("errors encountered when trying to make test description"); - } - // The `should-fail` annotation doesn't apply to pretty tests, // since we run the pretty printer across all tests by default. // If desired, we could add a `should-fail-pretty` annotation. diff --git a/src/tools/compiletest/src/directives/directive_names.rs b/src/tools/compiletest/src/directives/directive_names.rs index 1474df146f9ce..b8cb53fca20d0 100644 --- a/src/tools/compiletest/src/directives/directive_names.rs +++ b/src/tools/compiletest/src/directives/directive_names.rs @@ -1,3 +1,9 @@ +use std::collections::HashSet; +use std::sync::LazyLock; + +pub(crate) static KNOWN_DIRECTIVE_NAMES_SET: LazyLock> = + LazyLock::new(|| KNOWN_DIRECTIVE_NAMES.iter().copied().collect()); + /// This was originally generated by collecting directives from ui tests and then extracting their /// directive names. This is **not** an exhaustive list of all possible directives. Instead, this is /// a best-effort approximation for diagnostics. Add new directives to this list when needed. diff --git a/src/tools/compiletest/src/directives/tests.rs b/src/tools/compiletest/src/directives/tests.rs index 0bf2bcd3af434..e221c3a2daf2e 100644 --- a/src/tools/compiletest/src/directives/tests.rs +++ b/src/tools/compiletest/src/directives/tests.rs @@ -3,8 +3,8 @@ use semver::Version; use crate::common::{Config, Debugger, TestMode}; use crate::directives::{ - AuxProps, DirectivesCache, EarlyProps, Edition, EditionRange, FileDirectives, - extract_llvm_version, extract_version_range, iter_directives, line_directive, parse_edition, + self, AuxProps, DirectivesCache, EarlyProps, Edition, EditionRange, FileDirectives, + extract_llvm_version, extract_version_range, line_directive, parse_edition, parse_normalize_rule, }; use crate::executor::{CollectedTestDesc, ShouldFail}; @@ -767,7 +767,10 @@ fn threads_support() { fn run_path(poisoned: &mut bool, path: &Utf8Path, file_contents: &str) { let file_directives = FileDirectives::from_file_contents(path, file_contents); - iter_directives(TestMode::Ui, poisoned, &file_directives, &mut |_| {}); + let result = directives::do_early_directives_check(TestMode::Ui, &file_directives); + if result.is_err() { + *poisoned = true; + } } #[test] diff --git a/src/tools/compiletest/src/lib.rs b/src/tools/compiletest/src/lib.rs index 89f5623c1fc99..a3a40a1a7ee87 100644 --- a/src/tools/compiletest/src/lib.rs +++ b/src/tools/compiletest/src/lib.rs @@ -869,6 +869,12 @@ fn make_test(cx: &TestCollectorCx, collector: &mut TestCollector, testpaths: &Te let file_contents = 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) { + // 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}"); + } let early_props = EarlyProps::from_file_directives(&cx.config, &file_directives); // Normally we create one structure per revision, with two exceptions: