From 5877e7e8d230314c56b5d6c2e21796c4122c1ae6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=AE=B8=E6=9D=B0=E5=8F=8B=20Jieyou=20Xu=20=28Joe=29?= Date: Sat, 24 Feb 2024 17:27:10 +0000 Subject: [PATCH 1/2] Error on invalid compiletest directives in Rust test files --- src/tools/compiletest/src/header.rs | 128 +++++++++++++----- .../header/test-auxillary/error_annotation.rs | 6 + .../header/test-auxillary/known_directive.rs | 4 + .../test-auxillary/known_legacy_directive.rs | 1 + .../src/header/test-auxillary/not_rs.Makefile | 1 + .../test-auxillary/unknown_directive.rs | 1 + src/tools/compiletest/src/header/tests.rs | 62 +++++++++ 7 files changed, 172 insertions(+), 31 deletions(-) create mode 100644 src/tools/compiletest/src/header/test-auxillary/error_annotation.rs create mode 100644 src/tools/compiletest/src/header/test-auxillary/known_directive.rs create mode 100644 src/tools/compiletest/src/header/test-auxillary/known_legacy_directive.rs create mode 100644 src/tools/compiletest/src/header/test-auxillary/not_rs.Makefile create mode 100644 src/tools/compiletest/src/header/test-auxillary/unknown_directive.rs diff --git a/src/tools/compiletest/src/header.rs b/src/tools/compiletest/src/header.rs index f15761354f7ab..213e2a63517e5 100644 --- a/src/tools/compiletest/src/header.rs +++ b/src/tools/compiletest/src/header.rs @@ -680,7 +680,8 @@ pub fn line_directive<'line>( /// This is 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. -const DIAGNOSTICS_DIRECTIVE_NAMES: &[&str] = &[ +const KNOWN_DIRECTIVE_NAMES: &[&str] = &[ + // tidy-alphabetical-start "assembly-output", "aux-build", "aux-crate", @@ -693,6 +694,7 @@ const DIAGNOSTICS_DIRECTIVE_NAMES: &[&str] = &[ "check-stdout", "check-test-line-numbers-match", "compile-flags", + "count", "dont-check-compiler-stderr", "dont-check-compiler-stdout", "dont-check-failure-status", @@ -700,6 +702,7 @@ const DIAGNOSTICS_DIRECTIVE_NAMES: &[&str] = &[ "error-pattern", "exec-env", "failure-status", + "filecheck-flags", "forbid-output", "force-host", "ignore-16bit", @@ -716,6 +719,7 @@ const DIAGNOSTICS_DIRECTIVE_NAMES: &[&str] = &[ "ignore-compare-mode-polonius", "ignore-cross-compile", "ignore-debug", + "ignore-eabi", "ignore-emscripten", "ignore-endian-big", "ignore-freebsd", @@ -731,14 +735,30 @@ const DIAGNOSTICS_DIRECTIVE_NAMES: &[&str] = &[ "ignore-lldb", "ignore-llvm-version", "ignore-loongarch64", + "ignore-macabi", "ignore-macos", + "ignore-mode-assembly", + "ignore-mode-codegen", + "ignore-mode-codegen-units", "ignore-mode-coverage-map", "ignore-mode-coverage-run", + "ignore-mode-debuginfo", + "ignore-mode-incremental", + "ignore-mode-js-doc-test", + "ignore-mode-mir-opt", + "ignore-mode-pretty", + "ignore-mode-run-make", + "ignore-mode-run-pass-valgrind", + "ignore-mode-rustdoc", + "ignore-mode-rustdoc-json", + "ignore-mode-ui", + "ignore-mode-ui-fulldeps", "ignore-msp430", "ignore-msvc", "ignore-musl", "ignore-netbsd", "ignore-nightly", + "ignore-none", "ignore-nto", "ignore-nvptx64", "ignore-openbsd", @@ -750,18 +770,27 @@ const DIAGNOSTICS_DIRECTIVE_NAMES: &[&str] = &[ "ignore-spirv", "ignore-stable", "ignore-stage1", + "ignore-stage2", "ignore-test", + "ignore-thumb", "ignore-thumbv8m.base-none-eabi", "ignore-thumbv8m.main-none-eabi", + "ignore-unix", + "ignore-unknown", "ignore-uwp", "ignore-vxworks", + "ignore-wasi", "ignore-wasm", "ignore-wasm32", "ignore-wasm32-bare", + "ignore-wasm64", "ignore-windows", "ignore-windows-gnu", + "ignore-x32", "ignore-x86", + "ignore-x86_64", "ignore-x86_64-apple-darwin", + "ignore-x86_64-unknown-linux-gnu", "incremental", "known-bug", "llvm-cov-flags", @@ -769,9 +798,11 @@ const DIAGNOSTICS_DIRECTIVE_NAMES: &[&str] = &[ "min-gdb-version", "min-lldb-version", "min-llvm-version", + "min-system-llvm-version", "needs-asm-support", "needs-dlltool", "needs-dynamic-linking", + "needs-git-hash", "needs-llvm-components", "needs-profiler-support", "needs-relocation-model-pic", @@ -779,6 +810,7 @@ const DIAGNOSTICS_DIRECTIVE_NAMES: &[&str] = &[ "needs-rust-lldb", "needs-sanitizer-address", "needs-sanitizer-cfi", + "needs-sanitizer-dataflow", "needs-sanitizer-hwaddress", "needs-sanitizer-leak", "needs-sanitizer-memory", @@ -801,6 +833,7 @@ const DIAGNOSTICS_DIRECTIVE_NAMES: &[&str] = &[ "only-aarch64", "only-arm", "only-avr", + "only-beta", "only-bpf", "only-cdb", "only-gnu", @@ -818,6 +851,7 @@ const DIAGNOSTICS_DIRECTIVE_NAMES: &[&str] = &[ "only-riscv64", "only-sparc", "only-sparc64", + "only-stable", "only-thumb", "only-wasm32", "only-wasm32-bare", @@ -825,6 +859,7 @@ const DIAGNOSTICS_DIRECTIVE_NAMES: &[&str] = &[ "only-x86", "only-x86_64", "only-x86_64-fortanix-unknown-sgx", + "only-x86_64-pc-windows-gnu", "only-x86_64-pc-windows-msvc", "only-x86_64-unknown-linux-gnu", "pp-exact", @@ -846,6 +881,7 @@ const DIAGNOSTICS_DIRECTIVE_NAMES: &[&str] = &[ "unit-test", "unset-exec-env", "unset-rustc-env", + // tidy-alphabetical-end ]; /// The broken-down contents of a line containing a test header directive, @@ -876,6 +912,22 @@ struct HeaderLine<'ln> { directive: &'ln str, } +pub(crate) struct CheckDirectiveResult<'ln> { + is_known_directive: bool, + directive_name: &'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); + + CheckDirectiveResult { + is_known_directive: KNOWN_DIRECTIVE_NAMES.contains(&directive_name), + directive_name: directive_ln, + } +} + fn iter_header( mode: Mode, _suite: &str, @@ -915,6 +967,7 @@ fn iter_header( let mut ln = String::new(); let mut line_number = 0; + // Match on error annotations like `//~ERROR`. static REVISION_MAGIC_COMMENT_RE: Lazy = Lazy::new(|| Regex::new("//(\\[.*\\])?~.*").unwrap()); @@ -933,9 +986,38 @@ fn iter_header( if ln.starts_with("fn") || ln.starts_with("mod") { return; - // First try to accept `ui_test` style comments - } else if let Some((header_revision, directive)) = line_directive(comment, ln) { - it(HeaderLine { line_number, original_line, header_revision, directive }); + // First try to accept `ui_test` style comments (`//@`) + } else if let Some((header_revision, non_revisioned_directive_line)) = + line_directive(comment, ln) + { + // Perform unknown directive check on Rust files. + 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); + + if !is_known_directive { + *poisoned = true; + + eprintln!( + "error: detected unknown compiletest test directive `{}` in {}:{}", + directive_ln, + testfile.display(), + line_number, + ); + + return; + } + } + + it(HeaderLine { + line_number, + original_line, + header_revision, + directive: non_revisioned_directive_line, + }); + // Then we try to check for legacy-style candidates, which are not the magic ~ERROR family + // error annotations. } else if !REVISION_MAGIC_COMMENT_RE.is_match(ln) { let Some((_, rest)) = line_directive("//", ln) else { continue; @@ -949,34 +1031,18 @@ fn iter_header( let rest = rest.trim_start(); - for candidate in DIAGNOSTICS_DIRECTIVE_NAMES.iter() { - if rest.starts_with(candidate) { - let Some(prefix_removed) = rest.strip_prefix(candidate) else { - // We have a comment that's *successfully* parsed as an legacy-style - // directive. We emit an error here to warn the user. - *poisoned = true; - eprintln!( - "error: detected legacy-style directives in compiletest test: {}:{}, please use `ui_test`-style directives `//@` instead:{:#?}", - testfile.display(), - line_number, - line_directive("//", ln), - ); - return; - }; + let CheckDirectiveResult { is_known_directive, directive_name } = check_directive(rest); - if prefix_removed.starts_with([' ', ':']) { - // We have a comment that's *successfully* parsed as an legacy-style - // directive. We emit an error here to warn the user. - *poisoned = true; - eprintln!( - "error: detected legacy-style directives in compiletest test: {}:{}, please use `ui_test`-style directives `//@` instead:{:#?}", - testfile.display(), - line_number, - line_directive("//", ln), - ); - return; - } - } + if is_known_directive { + *poisoned = true; + eprintln!( + "error: detected legacy-style directive {} in compiletest test: {}:{}, please use `ui_test`-style directives `//@` instead: {:#?}", + directive_name, + testfile.display(), + line_number, + line_directive("//", ln), + ); + return; } } } diff --git a/src/tools/compiletest/src/header/test-auxillary/error_annotation.rs b/src/tools/compiletest/src/header/test-auxillary/error_annotation.rs new file mode 100644 index 0000000000000..fea66a5e07b4d --- /dev/null +++ b/src/tools/compiletest/src/header/test-auxillary/error_annotation.rs @@ -0,0 +1,6 @@ +//@ check-pass + +//~ HELP +fn main() {} //~ERROR +//~^ ERROR +//~| ERROR diff --git a/src/tools/compiletest/src/header/test-auxillary/known_directive.rs b/src/tools/compiletest/src/header/test-auxillary/known_directive.rs new file mode 100644 index 0000000000000..99834b14c1ea3 --- /dev/null +++ b/src/tools/compiletest/src/header/test-auxillary/known_directive.rs @@ -0,0 +1,4 @@ +//! ignore-wasm +//@ ignore-wasm +//@ check-pass +// regular comment diff --git a/src/tools/compiletest/src/header/test-auxillary/known_legacy_directive.rs b/src/tools/compiletest/src/header/test-auxillary/known_legacy_directive.rs new file mode 100644 index 0000000000000..108ca432e1308 --- /dev/null +++ b/src/tools/compiletest/src/header/test-auxillary/known_legacy_directive.rs @@ -0,0 +1 @@ +// ignore-wasm diff --git a/src/tools/compiletest/src/header/test-auxillary/not_rs.Makefile b/src/tools/compiletest/src/header/test-auxillary/not_rs.Makefile new file mode 100644 index 0000000000000..4b565e0e6dfd0 --- /dev/null +++ b/src/tools/compiletest/src/header/test-auxillary/not_rs.Makefile @@ -0,0 +1 @@ +# ignore-owo diff --git a/src/tools/compiletest/src/header/test-auxillary/unknown_directive.rs b/src/tools/compiletest/src/header/test-auxillary/unknown_directive.rs new file mode 100644 index 0000000000000..d440603104304 --- /dev/null +++ b/src/tools/compiletest/src/header/test-auxillary/unknown_directive.rs @@ -0,0 +1 @@ +//@ needs-headpat diff --git a/src/tools/compiletest/src/header/tests.rs b/src/tools/compiletest/src/header/tests.rs index 815ac3839df82..433f3e8b5559c 100644 --- a/src/tools/compiletest/src/header/tests.rs +++ b/src/tools/compiletest/src/header/tests.rs @@ -5,6 +5,8 @@ use std::str::FromStr; use crate::common::{Config, Debugger, Mode}; use crate::header::{parse_normalization_string, EarlyProps, HeadersCache}; +use super::iter_header; + fn make_test_description( config: &Config, name: test::TestName, @@ -612,3 +614,63 @@ fn threads_support() { assert_eq!(check_ignore(&config, "//@ needs-threads"), !has_threads) } } + +fn run_path(poisoned: &mut bool, path: &Path, buf: &[u8]) { + let rdr = std::io::Cursor::new(&buf); + iter_header(Mode::Ui, "ui", poisoned, path, rdr, &mut |_| {}); +} + +#[test] +fn test_unknown_directive_check() { + let mut poisoned = false; + run_path( + &mut poisoned, + Path::new("a.rs"), + include_bytes!("./test-auxillary/unknown_directive.rs"), + ); + assert!(poisoned); +} + +#[test] +fn test_known_legacy_directive_check() { + let mut poisoned = false; + run_path( + &mut poisoned, + Path::new("a.rs"), + include_bytes!("./test-auxillary/known_legacy_directive.rs"), + ); + assert!(poisoned); +} + +#[test] +fn test_known_directive_check_no_error() { + let mut poisoned = false; + run_path( + &mut poisoned, + Path::new("a.rs"), + include_bytes!("./test-auxillary/known_directive.rs"), + ); + assert!(!poisoned); +} + +#[test] +fn test_error_annotation_no_error() { + let mut poisoned = false; + run_path( + &mut poisoned, + Path::new("a.rs"), + include_bytes!("./test-auxillary/error_annotation.rs"), + ); + assert!(!poisoned); +} + +#[test] +fn test_non_rs_unknown_directive_not_checked() { + let mut poisoned = false; + run_path( + &mut poisoned, + Path::new("a.Makefile"), + include_bytes!("./test-auxillary/not_rs.Makefile"), + ); + assert!(!poisoned); +} From 64dda8c8378d985136dea2839bbb91c390aba356 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=AE=B8=E6=9D=B0=E5=8F=8B=20Jieyou=20Xu=20=28Joe=29?= Date: Sat, 24 Feb 2024 17:04:37 +0000 Subject: [PATCH 2/2] Fix invalid compiletest directives in tests - Fix invalid directive in `normalize-hidden-types` - Update legacy directive in `two-phase-reservation-sharing-interference` --- .../two-phase-reservation-sharing-interference.rs | 2 +- .../normalize-hidden-types.current.stderr | 12 ++++++------ .../type-alias-impl-trait/normalize-hidden-types.rs | 6 +----- 3 files changed, 8 insertions(+), 12 deletions(-) diff --git a/tests/ui/borrowck/two-phase-reservation-sharing-interference.rs b/tests/ui/borrowck/two-phase-reservation-sharing-interference.rs index ac0d4f6e09975..b6bcf7b66173e 100644 --- a/tests/ui/borrowck/two-phase-reservation-sharing-interference.rs +++ b/tests/ui/borrowck/two-phase-reservation-sharing-interference.rs @@ -2,7 +2,7 @@ // The nll_beyond revision is disabled due to missing support from two-phase beyond autorefs //@[nll_beyond]compile-flags: -Z two-phase-beyond-autoref -//[nll_beyond]should-fail +//@[nll_beyond]should-fail // This is a corner case that the current implementation is (probably) // treating more conservatively than is necessary. But it also does diff --git a/tests/ui/type-alias-impl-trait/normalize-hidden-types.current.stderr b/tests/ui/type-alias-impl-trait/normalize-hidden-types.current.stderr index dd2737c706d6b..d9d5dd4ece3b1 100644 --- a/tests/ui/type-alias-impl-trait/normalize-hidden-types.current.stderr +++ b/tests/ui/type-alias-impl-trait/normalize-hidden-types.current.stderr @@ -5,25 +5,25 @@ LL | fn define() -> Opaque { | ^^^^^^ expected `*const (dyn FnOnce(()) + 'static)`, got `*const dyn for<'a> FnOnce(::Gat<'a>)` | note: previous use here - --> $DIR/normalize-hidden-types.rs:27:9 + --> $DIR/normalize-hidden-types.rs:26:9 | LL | dyn_hoops::<_>(0) | ^^^^^^^^^^^^^^^^^ error: concrete type differs from previous defining opaque type use - --> $DIR/normalize-hidden-types.rs:34:22 + --> $DIR/normalize-hidden-types.rs:33:22 | LL | fn define_1() -> Opaque { dyn_hoops::<_>(0) } | ^^^^^^ expected `*const (dyn FnOnce(()) + 'static)`, got `*const dyn for<'a> FnOnce(::Gat<'a>)` | note: previous use here - --> $DIR/normalize-hidden-types.rs:34:31 + --> $DIR/normalize-hidden-types.rs:33:31 | LL | fn define_1() -> Opaque { dyn_hoops::<_>(0) } | ^^^^^^^^^^^^^^^^^ error[E0308]: mismatched types - --> $DIR/normalize-hidden-types.rs:44:25 + --> $DIR/normalize-hidden-types.rs:42:25 | LL | type Opaque = impl Sized; | ---------- the expected opaque type @@ -39,13 +39,13 @@ LL | let _: Opaque = dyn_hoops::(0); = note: for more information, visit https://doc.rust-lang.org/book/ch19-03-advanced-traits.html error: concrete type differs from previous defining opaque type use - --> $DIR/normalize-hidden-types.rs:54:25 + --> $DIR/normalize-hidden-types.rs:51:25 | LL | let _: Opaque = dyn_hoops::<_>(0); | ^^^^^^^^^^^^^^^^^ expected `*const (dyn FnOnce(()) + 'static)`, got `*const dyn for<'a> FnOnce(::Gat<'a>)` | note: previous use here - --> $DIR/normalize-hidden-types.rs:56:9 + --> $DIR/normalize-hidden-types.rs:52:9 | LL | None | ^^^^ diff --git a/tests/ui/type-alias-impl-trait/normalize-hidden-types.rs b/tests/ui/type-alias-impl-trait/normalize-hidden-types.rs index e78e6cf7690e7..d6800694e5133 100644 --- a/tests/ui/type-alias-impl-trait/normalize-hidden-types.rs +++ b/tests/ui/type-alias-impl-trait/normalize-hidden-types.rs @@ -3,7 +3,7 @@ //@ revisions: current next //@ [next] compile-flags: -Znext-solver //@ [next] check-pass -//@ [current]: known-bug: #112691 +//@ [current] known-bug: #112691 #![feature(type_alias_impl_trait)] @@ -23,7 +23,6 @@ mod typeof_1 { use super::*; type Opaque = impl Sized; fn define() -> Opaque { - //[current]~^ ERROR concrete type differs dyn_hoops::<_>(0) } } @@ -32,7 +31,6 @@ mod typeof_2 { use super::*; type Opaque = impl Sized; fn define_1() -> Opaque { dyn_hoops::<_>(0) } - //[current]~^ ERROR concrete type differs fn define_2() -> Opaque { dyn_hoops::(0) } } @@ -42,7 +40,6 @@ mod typeck { fn define() -> Option { let _: Opaque = dyn_hoops::<_>(0); let _: Opaque = dyn_hoops::(0); - //[current]~^ ERROR mismatched types None } } @@ -52,7 +49,6 @@ mod borrowck { type Opaque = impl Sized; fn define() -> Option { let _: Opaque = dyn_hoops::<_>(0); - //[current]~^ ERROR concrete type differs None } }