From 6457df3d4b945f2b56ada653e30116bb2267c004 Mon Sep 17 00:00:00 2001 From: Pietro Albini Date: Thu, 20 Oct 2022 11:35:28 +0200 Subject: [PATCH 1/5] ensure that compile-flags arguments are the last in ui tests Before this commit, compiletest would add `-L path/to/aux` at the end of the rustc flags, even after the custom ones set with the compile-flags header comment. This made it impossible to check how rustc would behave when a flag requiring an argument was passed without the argument, because the argument would become `-L`. This PR fixes that by adding the `-L path/to/aux` before the arguments defined in compile-flags, at least for UI tests. Other test suites might either be fixed as well by this change, or still present the old behavior. --- src/tools/compiletest/src/runtest.rs | 60 +++++++++++++++++++--------- 1 file changed, 42 insertions(+), 18 deletions(-) diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index 8f289876f73..ab0d568ffa7 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -1499,10 +1499,13 @@ fn compile_test_general( _ => AllowUnused::No, }; - let mut rustc = - self.make_compile_args(&self.testpaths.file, output_file, emit_metadata, allow_unused); - - rustc.arg("-L").arg(&self.aux_output_dir_name()); + let rustc = self.make_compile_args( + &self.testpaths.file, + output_file, + emit_metadata, + allow_unused, + LinkToAux::Yes, + ); self.compose_and_run_compiler(rustc, None) } @@ -1729,8 +1732,13 @@ fn build_auxiliary(&self, source_path: &str, aux_dir: &Path) -> bool { // Create the directory for the stdout/stderr files. create_dir_all(aux_cx.output_base_dir()).unwrap(); let input_file = &aux_testpaths.file; - let mut aux_rustc = - aux_cx.make_compile_args(input_file, aux_output, EmitMetadata::No, AllowUnused::No); + let mut aux_rustc = aux_cx.make_compile_args( + input_file, + aux_output, + EmitMetadata::No, + AllowUnused::No, + LinkToAux::No, + ); for key in &aux_props.unset_rustc_env { aux_rustc.env_remove(key); @@ -1869,6 +1877,7 @@ fn make_compile_args( output_file: TargetLocation, emit_metadata: EmitMetadata, allow_unused: AllowUnused, + link_to_aux: LinkToAux, ) -> Command { let is_aux = input_file.components().map(|c| c.as_os_str()).any(|c| c == "auxiliary"); let is_rustdoc = self.is_rustdoc() && !is_aux; @@ -2056,6 +2065,10 @@ fn make_compile_args( rustc.arg("-Ctarget-feature=-crt-static"); } + if let LinkToAux::Yes = link_to_aux { + rustc.arg("-L").arg(self.aux_output_dir_name()); + } + rustc.args(&self.props.compile_flags); rustc @@ -2247,13 +2260,16 @@ fn fatal_proc_rec_with_ctx( // codegen tests (using FileCheck) fn compile_test_and_save_ir(&self) -> ProcRes { - let aux_dir = self.aux_output_dir_name(); - let output_file = TargetLocation::ThisDirectory(self.output_base_dir()); let input_file = &self.testpaths.file; - let mut rustc = - self.make_compile_args(input_file, output_file, EmitMetadata::No, AllowUnused::No); - rustc.arg("-L").arg(aux_dir).arg("--emit=llvm-ir"); + let mut rustc = self.make_compile_args( + input_file, + output_file, + EmitMetadata::No, + AllowUnused::No, + LinkToAux::Yes, + ); + rustc.arg("--emit=llvm-ir"); self.compose_and_run_compiler(rustc, None) } @@ -2265,10 +2281,13 @@ fn compile_test_and_save_assembly(&self) -> (ProcRes, PathBuf) { let output_file = TargetLocation::ThisFile(output_path.clone()); let input_file = &self.testpaths.file; - let mut rustc = - self.make_compile_args(input_file, output_file, EmitMetadata::No, AllowUnused::No); - - rustc.arg("-L").arg(self.aux_output_dir_name()); + let mut rustc = self.make_compile_args( + input_file, + output_file, + EmitMetadata::No, + AllowUnused::No, + LinkToAux::Yes, + ); match self.props.assembly_output.as_ref().map(AsRef::as_ref) { Some("emit-asm") => { @@ -2409,8 +2428,8 @@ fn compare_to_default_rustdoc(&mut self, out_dir: &Path) { output_file, EmitMetadata::No, AllowUnused::Yes, + LinkToAux::Yes, ); - rustc.arg("-L").arg(&new_rustdoc.aux_output_dir_name()); new_rustdoc.build_all_auxiliary(&mut rustc); let proc_res = new_rustdoc.document(&compare_dir); @@ -3354,13 +3373,13 @@ fn run_ui_test(&self) { if self.props.run_rustfix && self.config.compare_mode.is_none() { // And finally, compile the fixed code and make sure it both // succeeds and has no diagnostics. - let mut rustc = self.make_compile_args( + let rustc = self.make_compile_args( &self.testpaths.file.with_extension(UI_FIXED), TargetLocation::ThisFile(self.make_exe_name()), emit_metadata, AllowUnused::No, + LinkToAux::Yes, ); - rustc.arg("-L").arg(&self.aux_output_dir_name()); let res = self.compose_and_run_compiler(rustc, None); if !res.status.success() { self.fatal_proc_rec("failed to compile fixed code", &res); @@ -3948,3 +3967,8 @@ enum AllowUnused { Yes, No, } + +enum LinkToAux { + Yes, + No, +} From 954649069ffe4a9d42bc0a83e07e6af4732300ea Mon Sep 17 00:00:00 2001 From: Pietro Albini Date: Mon, 24 Oct 2022 09:56:05 +0200 Subject: [PATCH 2/5] add a test to verify that compile-flags are provided last --- src/test/ui/compiletest-compile-flags-last.rs | 6 ++++++ src/test/ui/compiletest-compile-flags-last.stderr | 2 ++ src/tools/tidy/src/ui_tests.rs | 2 +- 3 files changed, 9 insertions(+), 1 deletion(-) create mode 100644 src/test/ui/compiletest-compile-flags-last.rs create mode 100644 src/test/ui/compiletest-compile-flags-last.stderr diff --git a/src/test/ui/compiletest-compile-flags-last.rs b/src/test/ui/compiletest-compile-flags-last.rs new file mode 100644 index 00000000000..9c78e4c5e9d --- /dev/null +++ b/src/test/ui/compiletest-compile-flags-last.rs @@ -0,0 +1,6 @@ +// Check that the arguments provided through `// compile-flags` are added last to the command line +// in UI tests. To ensure that we invoke rustc with a flag that expects an argument withut actually +// providing it. If the compile-flags are not last, the test will fail as rustc will interpret the +// next flag as the argument of this flag. +// +// compile-flags: --cap-lints diff --git a/src/test/ui/compiletest-compile-flags-last.stderr b/src/test/ui/compiletest-compile-flags-last.stderr new file mode 100644 index 00000000000..d8d40a7d9f1 --- /dev/null +++ b/src/test/ui/compiletest-compile-flags-last.stderr @@ -0,0 +1,2 @@ +error: Argument to option 'cap-lints' missing + diff --git a/src/tools/tidy/src/ui_tests.rs b/src/tools/tidy/src/ui_tests.rs index c600f99c2c4..d307d6f43b1 100644 --- a/src/tools/tidy/src/ui_tests.rs +++ b/src/tools/tidy/src/ui_tests.rs @@ -7,7 +7,7 @@ const ENTRY_LIMIT: usize = 1000; // FIXME: The following limits should be reduced eventually. -const ROOT_ENTRY_LIMIT: usize = 948; +const ROOT_ENTRY_LIMIT: usize = 949; const ISSUES_ENTRY_LIMIT: usize = 2117; fn check_entries(path: &Path, bad: &mut bool) { From 1adf83b128de3043ff0c24863e398c59e4ebb743 Mon Sep 17 00:00:00 2001 From: Pietro Albini Date: Thu, 3 Nov 2022 11:47:05 +0100 Subject: [PATCH 3/5] check for pattern in compiletest-compile-flags-last --- src/test/ui/compiletest-compile-flags-last.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test/ui/compiletest-compile-flags-last.rs b/src/test/ui/compiletest-compile-flags-last.rs index 9c78e4c5e9d..232df10f1a8 100644 --- a/src/test/ui/compiletest-compile-flags-last.rs +++ b/src/test/ui/compiletest-compile-flags-last.rs @@ -4,3 +4,4 @@ // next flag as the argument of this flag. // // compile-flags: --cap-lints +// error-pattern: Argument to option 'cap-lints' missing From 5ed753cb0db4defb4bb94178daf53f9bba707a8c Mon Sep 17 00:00:00 2001 From: Pietro Albini Date: Thu, 3 Nov 2022 11:48:33 +0100 Subject: [PATCH 4/5] move compiletest's own ui tests into a subdirectory --- .../compile-flags-last.rs} | 0 .../compile-flags-last.stderr} | 0 src/test/ui/{ => compiletest-self-test}/ui-testing-optout.rs | 0 .../ui/{ => compiletest-self-test}/ui-testing-optout.stderr | 0 src/tools/tidy/src/ui_tests.rs | 2 +- 5 files changed, 1 insertion(+), 1 deletion(-) rename src/test/ui/{compiletest-compile-flags-last.rs => compiletest-self-test/compile-flags-last.rs} (100%) rename src/test/ui/{compiletest-compile-flags-last.stderr => compiletest-self-test/compile-flags-last.stderr} (100%) rename src/test/ui/{ => compiletest-self-test}/ui-testing-optout.rs (100%) rename src/test/ui/{ => compiletest-self-test}/ui-testing-optout.stderr (100%) diff --git a/src/test/ui/compiletest-compile-flags-last.rs b/src/test/ui/compiletest-self-test/compile-flags-last.rs similarity index 100% rename from src/test/ui/compiletest-compile-flags-last.rs rename to src/test/ui/compiletest-self-test/compile-flags-last.rs diff --git a/src/test/ui/compiletest-compile-flags-last.stderr b/src/test/ui/compiletest-self-test/compile-flags-last.stderr similarity index 100% rename from src/test/ui/compiletest-compile-flags-last.stderr rename to src/test/ui/compiletest-self-test/compile-flags-last.stderr diff --git a/src/test/ui/ui-testing-optout.rs b/src/test/ui/compiletest-self-test/ui-testing-optout.rs similarity index 100% rename from src/test/ui/ui-testing-optout.rs rename to src/test/ui/compiletest-self-test/ui-testing-optout.rs diff --git a/src/test/ui/ui-testing-optout.stderr b/src/test/ui/compiletest-self-test/ui-testing-optout.stderr similarity index 100% rename from src/test/ui/ui-testing-optout.stderr rename to src/test/ui/compiletest-self-test/ui-testing-optout.stderr diff --git a/src/tools/tidy/src/ui_tests.rs b/src/tools/tidy/src/ui_tests.rs index d307d6f43b1..c600f99c2c4 100644 --- a/src/tools/tidy/src/ui_tests.rs +++ b/src/tools/tidy/src/ui_tests.rs @@ -7,7 +7,7 @@ const ENTRY_LIMIT: usize = 1000; // FIXME: The following limits should be reduced eventually. -const ROOT_ENTRY_LIMIT: usize = 949; +const ROOT_ENTRY_LIMIT: usize = 948; const ISSUES_ENTRY_LIMIT: usize = 2117; fn check_entries(path: &Path, bad: &mut bool) { From 4c55b29349e9367daad49317cef642c41d5608ac Mon Sep 17 00:00:00 2001 From: Pietro Albini Date: Thu, 3 Nov 2022 12:00:17 +0100 Subject: [PATCH 5/5] put custom flags as last in codegen and asm tests --- src/tools/compiletest/src/runtest.rs | 74 +++++++++++++++------------- 1 file changed, 41 insertions(+), 33 deletions(-) diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index ab0d568ffa7..4a59dca49fe 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -220,11 +220,13 @@ enum WillExecute { Disabled, } -/// Should `--emit metadata` be used? +/// What value should be passed to `--emit`? #[derive(Copy, Clone)] -enum EmitMetadata { - Yes, - No, +enum Emit { + None, + Metadata, + LlvmIr, + Asm, } impl<'test> TestCx<'test> { @@ -424,7 +426,7 @@ fn run_valgrind_test(&self) { } let should_run = self.run_if_enabled(); - let mut proc_res = self.compile_test(should_run, EmitMetadata::No); + let mut proc_res = self.compile_test(should_run, Emit::None); if !proc_res.status.success() { self.fatal_proc_rec("compilation failed!", &proc_res); @@ -676,7 +678,7 @@ fn run_debuginfo_cdb_test_no_opt(&self) { // compile test file (it should have 'compile-flags:-g' in the header) let should_run = self.run_if_enabled(); - let compile_result = self.compile_test(should_run, EmitMetadata::No); + let compile_result = self.compile_test(should_run, Emit::None); if !compile_result.status.success() { self.fatal_proc_rec("compilation failed!", &compile_result); } @@ -796,7 +798,7 @@ fn run_debuginfo_gdb_test_no_opt(&self) { // compile test file (it should have 'compile-flags:-g' in the header) let should_run = self.run_if_enabled(); - let compiler_run_result = self.compile_test(should_run, EmitMetadata::No); + let compiler_run_result = self.compile_test(should_run, Emit::None); if !compiler_run_result.status.success() { self.fatal_proc_rec("compilation failed!", &compiler_run_result); } @@ -1028,7 +1030,7 @@ fn run_debuginfo_lldb_test(&self) { fn run_debuginfo_lldb_test_no_opt(&self) { // compile test file (it should have 'compile-flags:-g' in the header) let should_run = self.run_if_enabled(); - let compile_result = self.compile_test(should_run, EmitMetadata::No); + let compile_result = self.compile_test(should_run, Emit::None); if !compile_result.status.success() { self.fatal_proc_rec("compilation failed!", &compile_result); } @@ -1453,21 +1455,21 @@ fn is_unexpected_compiler_message( } } - fn should_emit_metadata(&self, pm: Option) -> EmitMetadata { + fn should_emit_metadata(&self, pm: Option) -> Emit { match (pm, self.props.fail_mode, self.config.mode) { - (Some(PassMode::Check), ..) | (_, Some(FailMode::Check), Ui) => EmitMetadata::Yes, - _ => EmitMetadata::No, + (Some(PassMode::Check), ..) | (_, Some(FailMode::Check), Ui) => Emit::Metadata, + _ => Emit::None, } } - fn compile_test(&self, will_execute: WillExecute, emit_metadata: EmitMetadata) -> ProcRes { - self.compile_test_general(will_execute, emit_metadata, self.props.local_pass_mode()) + fn compile_test(&self, will_execute: WillExecute, emit: Emit) -> ProcRes { + self.compile_test_general(will_execute, emit, self.props.local_pass_mode()) } fn compile_test_general( &self, will_execute: WillExecute, - emit_metadata: EmitMetadata, + emit: Emit, local_pm: Option, ) -> ProcRes { // Only use `make_exe_name` when the test ends up being executed. @@ -1502,7 +1504,7 @@ fn compile_test_general( let rustc = self.make_compile_args( &self.testpaths.file, output_file, - emit_metadata, + emit, allow_unused, LinkToAux::Yes, ); @@ -1735,7 +1737,7 @@ fn build_auxiliary(&self, source_path: &str, aux_dir: &Path) -> bool { let mut aux_rustc = aux_cx.make_compile_args( input_file, aux_output, - EmitMetadata::No, + Emit::None, AllowUnused::No, LinkToAux::No, ); @@ -1875,7 +1877,7 @@ fn make_compile_args( &self, input_file: &Path, output_file: TargetLocation, - emit_metadata: EmitMetadata, + emit: Emit, allow_unused: AllowUnused, link_to_aux: LinkToAux, ) -> Command { @@ -1992,8 +1994,18 @@ fn make_compile_args( } } - if let (false, EmitMetadata::Yes) = (is_rustdoc, emit_metadata) { - rustc.args(&["--emit", "metadata"]); + match emit { + Emit::None => {} + Emit::Metadata if is_rustdoc => {} + Emit::Metadata => { + rustc.args(&["--emit", "metadata"]); + } + Emit::LlvmIr => { + rustc.args(&["--emit", "llvm-ir"]); + } + Emit::Asm => { + rustc.args(&["--emit", "asm"]); + } } if !is_rustdoc { @@ -2262,14 +2274,13 @@ fn fatal_proc_rec_with_ctx( fn compile_test_and_save_ir(&self) -> ProcRes { let output_file = TargetLocation::ThisDirectory(self.output_base_dir()); let input_file = &self.testpaths.file; - let mut rustc = self.make_compile_args( + let rustc = self.make_compile_args( input_file, output_file, - EmitMetadata::No, + Emit::LlvmIr, AllowUnused::No, LinkToAux::Yes, ); - rustc.arg("--emit=llvm-ir"); self.compose_and_run_compiler(rustc, None) } @@ -2281,17 +2292,11 @@ fn compile_test_and_save_assembly(&self) -> (ProcRes, PathBuf) { let output_file = TargetLocation::ThisFile(output_path.clone()); let input_file = &self.testpaths.file; - let mut rustc = self.make_compile_args( - input_file, - output_file, - EmitMetadata::No, - AllowUnused::No, - LinkToAux::Yes, - ); + let mut emit = Emit::None; match self.props.assembly_output.as_ref().map(AsRef::as_ref) { Some("emit-asm") => { - rustc.arg("--emit=asm"); + emit = Emit::Asm; } Some("ptx-linker") => { @@ -2302,6 +2307,9 @@ fn compile_test_and_save_assembly(&self) -> (ProcRes, PathBuf) { None => self.fatal("missing 'assembly-output' header"), } + let rustc = + self.make_compile_args(input_file, output_file, emit, AllowUnused::No, LinkToAux::Yes); + (self.compose_and_run_compiler(rustc, None), output_path) } @@ -2426,7 +2434,7 @@ fn compare_to_default_rustdoc(&mut self, out_dir: &Path) { let mut rustc = new_rustdoc.make_compile_args( &new_rustdoc.testpaths.file, output_file, - EmitMetadata::No, + Emit::None, AllowUnused::Yes, LinkToAux::Yes, ); @@ -2702,7 +2710,7 @@ fn check_rustdoc_test_option(&self, res: ProcRes) { fn run_codegen_units_test(&self) { assert!(self.revision.is_none(), "revisions not relevant here"); - let proc_res = self.compile_test(WillExecute::No, EmitMetadata::No); + let proc_res = self.compile_test(WillExecute::No, Emit::None); if !proc_res.status.success() { self.fatal_proc_rec("compilation failed!", &proc_res); @@ -3215,7 +3223,7 @@ fn run_ui_test(&self) { if let Some(FailMode::Build) = self.props.fail_mode { // Make sure a build-fail test cannot fail due to failing analysis (e.g. typeck). let pm = Some(PassMode::Check); - let proc_res = self.compile_test_general(WillExecute::No, EmitMetadata::Yes, pm); + let proc_res = self.compile_test_general(WillExecute::No, Emit::Metadata, pm); self.check_if_test_should_compile(&proc_res, pm); }