From f62753f84f355d2c8c67b257db6746c7ae58be2e Mon Sep 17 00:00:00 2001 From: Jieyou Xu Date: Tue, 26 Nov 2024 00:37:33 +0800 Subject: [PATCH] compiletest: remove `pretty-expanded` directive and infra Foreword ======== Let us begin the journey to rediscover what the `//@ pretty-expanded` directive does, brave traveller -- "My good friend, [..] when I wrote that passage, God and I knew what it meant. It is possible that God knows it still; but as for me, I have totally forgotten." -- Johann Paul Friedrich Richter, 1826 We must retrace the steps of those before us, for history shall guide us in the present and inform us of the future. The Past ======== Originally there was some effort to introduce more test coverage for `-Z unpretty=expanded` (in 2015 this was called `--pretty=expanded`). In [Make it an error to not declare used features #23598][pr-23598], there was a flip from `//@ no-pretty-expanded` (opt-out of `-Z unpretty=expanded` test) to `//@ pretty-expanded` (opt-in to `-Z unpretty=expanded` test). This was needed because back then the dedicated `tests/pretty` ("pretty") test suite did not existed, and the pretty tests were grouped together under `run-pass` tests (I believe `ui` test suite didn't exist back then either). Unfortunately, in this process the replacement `//@ pretty-expanded` directives contained a `FIXME #23616` linking to [There are very few tests for `-Z unpretty` expansion #23616][issue-23616]. But this was arguably backwards and somewhat misleading, as noted in : The attribute is off by default and things just work if you don't test it, people have not been adding the `pretty-expanded` annotation to new tests even if it would work. Which basically renders this useless. The Present =========== As of Nov 2024, we have a dedicated `pretty` test suite, and some time over the years the split between `run-pass` into `ui` and `pretty` test suites caused all of the `//@ pretty-expanded` in `ui` tests to do absolutely nothing -- the compiletest logic for `pretty-expanded` only triggered in the *pretty* test suite, but none of the pretty tests use it. Oops. The Future ========== Nobody remembers this, nobody uses this, it's misleading in ui tests. Let's get rid of this directive altogether. [pr-23598]: https://github.com/rust-lang/rust/pull/23598 [issue-23616]: https://github.com/rust-lang/rust/issues/23616 --- src/tools/compiletest/src/directive-list.rs | 1 - src/tools/compiletest/src/header.rs | 5 ----- src/tools/compiletest/src/runtest/pretty.rs | 16 ---------------- 3 files changed, 22 deletions(-) diff --git a/src/tools/compiletest/src/directive-list.rs b/src/tools/compiletest/src/directive-list.rs index bdd0b80b395..0c47ef871d2 100644 --- a/src/tools/compiletest/src/directive-list.rs +++ b/src/tools/compiletest/src/directive-list.rs @@ -214,7 +214,6 @@ const KNOWN_DIRECTIVE_NAMES: &[&str] = &[ "only-x86_64-unknown-linux-gnu", "pp-exact", "pretty-compare-only", - "pretty-expanded", "pretty-mode", "reference", "regex-error-pattern", diff --git a/src/tools/compiletest/src/header.rs b/src/tools/compiletest/src/header.rs index 3539e85ba63..e945797647e 100644 --- a/src/tools/compiletest/src/header.rs +++ b/src/tools/compiletest/src/header.rs @@ -124,8 +124,6 @@ pub struct TestProps { // a proc-macro and needs `#![crate_type = "proc-macro"]`. This ensures // that the aux file is compiled as a `proc-macro` and not as a `dylib`. pub no_prefer_dynamic: bool, - // Run -Zunpretty expanded when running pretty printing tests - pub pretty_expanded: bool, // Which pretty mode are we testing with, default to 'normal' pub pretty_mode: String, // Only compare pretty output and don't try compiling @@ -218,7 +216,6 @@ mod directives { pub const DONT_CHECK_COMPILER_STDOUT: &'static str = "dont-check-compiler-stdout"; pub const DONT_CHECK_COMPILER_STDERR: &'static str = "dont-check-compiler-stderr"; pub const NO_PREFER_DYNAMIC: &'static str = "no-prefer-dynamic"; - pub const PRETTY_EXPANDED: &'static str = "pretty-expanded"; pub const PRETTY_MODE: &'static str = "pretty-mode"; pub const PRETTY_COMPARE_ONLY: &'static str = "pretty-compare-only"; pub const AUX_BIN: &'static str = "aux-bin"; @@ -278,7 +275,6 @@ impl TestProps { dont_check_compiler_stderr: false, compare_output_lines_by_subset: false, no_prefer_dynamic: false, - pretty_expanded: false, pretty_mode: "normal".to_string(), pretty_compare_only: false, forbid_output: vec![], @@ -425,7 +421,6 @@ impl TestProps { &mut self.dont_check_compiler_stderr, ); config.set_name_directive(ln, NO_PREFER_DYNAMIC, &mut self.no_prefer_dynamic); - config.set_name_directive(ln, PRETTY_EXPANDED, &mut self.pretty_expanded); if let Some(m) = config.parse_name_value_directive(ln, PRETTY_MODE) { self.pretty_mode = m; diff --git a/src/tools/compiletest/src/runtest/pretty.rs b/src/tools/compiletest/src/runtest/pretty.rs index 40e767e84ef..e3b07f1d63d 100644 --- a/src/tools/compiletest/src/runtest/pretty.rs +++ b/src/tools/compiletest/src/runtest/pretty.rs @@ -84,21 +84,5 @@ impl TestCx<'_> { if !proc_res.status.success() { self.fatal_proc_rec("pretty-printed source does not typecheck", &proc_res); } - - if !self.props.pretty_expanded { - return; - } - - // additionally, run `-Zunpretty=expanded` and try to build it. - let proc_res = self.print_source(ReadFrom::Path, "expanded"); - if !proc_res.status.success() { - self.fatal_proc_rec("pretty-printing (expanded) failed", &proc_res); - } - - let ProcRes { stdout: expanded_src, .. } = proc_res; - let proc_res = self.typecheck_source(expanded_src); - if !proc_res.status.success() { - self.fatal_proc_rec("pretty-printed source (expanded) does not typecheck", &proc_res); - } } }