From 891041fbc994648f3ab771e9e9147a9fddc6006f Mon Sep 17 00:00:00 2001 From: Davis Muro Date: Mon, 30 Dec 2024 08:18:22 -0800 Subject: [PATCH 1/2] deny usage of FileCheck prefixes as revision names --- src/tools/compiletest/src/header.rs | 10 ++++++++++ src/tools/compiletest/src/header/tests.rs | 15 +++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/src/tools/compiletest/src/header.rs b/src/tools/compiletest/src/header.rs index 48149e3b897e1..3111793ab6a81 100644 --- a/src/tools/compiletest/src/header.rs +++ b/src/tools/compiletest/src/header.rs @@ -934,6 +934,9 @@ fn iter_header( impl Config { fn parse_and_update_revisions(&self, testfile: &Path, line: &str, existing: &mut Vec) { + const FORBIDDEN_REVISION_NAMES: [&str; 9] = + ["CHECK", "COM", "NEXT", "SAME", "EMPTY", "NOT", "COUNT", "DAG", "LABEL"]; + if let Some(raw) = self.parse_name_value_directive(line, "revisions") { if self.mode == Mode::RunMake { panic!("`run-make` tests do not support revisions: {}", testfile.display()); @@ -948,6 +951,13 @@ impl Config { raw, testfile.display() ); + } else if FORBIDDEN_REVISION_NAMES.contains(&revision.as_str()) { + panic!( + "invalid revision: `{}` in line `{}`: {}", + revision, + raw, + testfile.display() + ); } existing.push(revision); } diff --git a/src/tools/compiletest/src/header/tests.rs b/src/tools/compiletest/src/header/tests.rs index 618b66dfd4cb6..52bbd0314985f 100644 --- a/src/tools/compiletest/src/header/tests.rs +++ b/src/tools/compiletest/src/header/tests.rs @@ -553,6 +553,21 @@ fn test_duplicate_revisions() { parse_rs(&config, "//@ revisions: rpass1 rpass1"); } +#[test] +fn test_forbidden_revisions() { + let config: Config = cfg().build(); + let revisions = ["CHECK", "COM", "NEXT", "SAME", "EMPTY", "NOT", "COUNT", "DAG", "LABEL"]; + for rev in revisions { + let res = std::panic::catch_unwind(|| { + parse_rs(&config, format!("//@ revisions: {rev}").as_str()); + }); + assert!(res.is_err()); + if let Some(msg) = res.unwrap_err().downcast_ref::() { + assert!(msg.contains(format!("invalid revision: `{rev}` in line ` {rev}`").as_str())) + } + } +} + #[test] fn ignore_arch() { let archs = [ From 4a5e76a70e3d33b0eb4a233f60de5578b22bfba5 Mon Sep 17 00:00:00 2001 From: Davis Muro Date: Thu, 2 Jan 2025 18:27:07 -0800 Subject: [PATCH 2/2] limit special `FileCheck` revision checks --- src/tools/compiletest/src/header.rs | 8 ++-- src/tools/compiletest/src/header/tests.rs | 54 +++++++++++++++++++---- 2 files changed, 51 insertions(+), 11 deletions(-) diff --git a/src/tools/compiletest/src/header.rs b/src/tools/compiletest/src/header.rs index 3111793ab6a81..8c96554738e68 100644 --- a/src/tools/compiletest/src/header.rs +++ b/src/tools/compiletest/src/header.rs @@ -951,10 +951,12 @@ impl Config { raw, testfile.display() ); - } else if FORBIDDEN_REVISION_NAMES.contains(&revision.as_str()) { + } else if matches!(self.mode, Mode::Assembly | Mode::Codegen | Mode::MirOpt) + && FORBIDDEN_REVISION_NAMES.contains(&revision.as_str()) + { panic!( - "invalid revision: `{}` in line `{}`: {}", - revision, + "revision name `{revision}` is not permitted in a test suite that uses `FileCheck` annotations\n\ + as it is confusing when used as custom `FileCheck` prefix: `{revision}` in line `{}`: {}", raw, testfile.display() ); diff --git a/src/tools/compiletest/src/header/tests.rs b/src/tools/compiletest/src/header/tests.rs index 52bbd0314985f..25bb1a5f42882 100644 --- a/src/tools/compiletest/src/header/tests.rs +++ b/src/tools/compiletest/src/header/tests.rs @@ -554,16 +554,54 @@ fn test_duplicate_revisions() { } #[test] -fn test_forbidden_revisions() { - let config: Config = cfg().build(); +#[should_panic( + expected = "revision name `CHECK` is not permitted in a test suite that uses `FileCheck` annotations" +)] +fn test_assembly_mode_forbidden_revisions() { + let config = cfg().mode("assembly").build(); + parse_rs(&config, "//@ revisions: CHECK"); +} + +#[test] +#[should_panic( + expected = "revision name `CHECK` is not permitted in a test suite that uses `FileCheck` annotations" +)] +fn test_codegen_mode_forbidden_revisions() { + let config = cfg().mode("codegen").build(); + parse_rs(&config, "//@ revisions: CHECK"); +} + +#[test] +#[should_panic( + expected = "revision name `CHECK` is not permitted in a test suite that uses `FileCheck` annotations" +)] +fn test_miropt_mode_forbidden_revisions() { + let config = cfg().mode("mir-opt").build(); + parse_rs(&config, "//@ revisions: CHECK"); +} + +#[test] +fn test_forbidden_revisions_allowed_in_non_filecheck_dir() { let revisions = ["CHECK", "COM", "NEXT", "SAME", "EMPTY", "NOT", "COUNT", "DAG", "LABEL"]; + let modes = [ + "pretty", + "debuginfo", + "rustdoc", + "rustdoc-json", + "codegen-units", + "incremental", + "ui", + "js-doc-test", + "coverage-map", + "coverage-run", + "crashes", + ]; + for rev in revisions { - let res = std::panic::catch_unwind(|| { - parse_rs(&config, format!("//@ revisions: {rev}").as_str()); - }); - assert!(res.is_err()); - if let Some(msg) = res.unwrap_err().downcast_ref::() { - assert!(msg.contains(format!("invalid revision: `{rev}` in line ` {rev}`").as_str())) + let content = format!("//@ revisions: {rev}"); + for mode in modes { + let config = cfg().mode(mode).build(); + parse_rs(&config, &content); } } }