From a5181d2fd6d149dac10a7f888be0c46ba4a34d89 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Fri, 3 Apr 2026 20:37:45 +1100 Subject: [PATCH] Remove `rfail` support from incremental tests Incremental revisions beginning with `rfail` would cause the incremental test runner to build the test program, expecting success, and then run the test program, expecting failure. Expecting incremental tests to fail at runtime is of questionable utility, because in almost all cases an equivalent test program can be made to succeed at runtime instead. Removing `rfail` support is a small step towards cleaning up compiletest's incremental test runner, and its overall handling of pass/fail expectations. There was one existing regression test using `rfail` revisions: `tests/incremental/issue-80691-bad-eval-cache.rs`. The test code is complex, and reverting the fix in RUST-83220 does not actually cause the test to fail, suggesting that it is no longer a useful regression test. This commit therefore deletes that test. --- .../rustc-dev-guide/src/tests/compiletest.md | 4 +- src/tools/compiletest/src/runtest.rs | 7 +- .../compiletest/src/runtest/incremental.rs | 32 +-- .../incremental/issue-80691-bad-eval-cache.rs | 186 ------------------ 4 files changed, 7 insertions(+), 222 deletions(-) delete mode 100644 tests/incremental/issue-80691-bad-eval-cache.rs diff --git a/src/doc/rustc-dev-guide/src/tests/compiletest.md b/src/doc/rustc-dev-guide/src/tests/compiletest.md index a239310d124e..83fcfecc1de7 100644 --- a/src/doc/rustc-dev-guide/src/tests/compiletest.md +++ b/src/doc/rustc-dev-guide/src/tests/compiletest.md @@ -158,9 +158,9 @@ then runs the compiler for each revision, reusing the incremental results from p The revisions should start with: -* `rpass` — the test should compile and run successfully -* `rfail` — the test should compile successfully, but the executable should fail to run * `cfail` — the test should fail to compile +* `cpass` — the test should compile successully +* `rpass` — the test should compile and run successfully To make the revisions unique, you should add a suffix like `rpass1` and `rpass2`. diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index 53599179cfce..5fc3dbbfbc54 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -333,15 +333,12 @@ fn should_compile_successfully(&self, pm: Option) -> bool { TestMode::Incremental => { let revision = self.revision.expect("incremental tests require a list of revisions"); - if revision.starts_with("cpass") - || revision.starts_with("rpass") - || revision.starts_with("rfail") - { + if revision.starts_with("cpass") || revision.starts_with("rpass") { true } else if revision.starts_with("cfail") { pm.is_some() } else { - panic!("revision name must begin with cpass, rpass, rfail, or cfail"); + panic!("revision name must begin with `cfail`, `cpass`, or `rpass`"); } } mode => panic!("unimplemented for mode {:?}", mode), diff --git a/src/tools/compiletest/src/runtest/incremental.rs b/src/tools/compiletest/src/runtest/incremental.rs index 5e7698e24858..75e43006486e 100644 --- a/src/tools/compiletest/src/runtest/incremental.rs +++ b/src/tools/compiletest/src/runtest/incremental.rs @@ -5,17 +5,16 @@ impl TestCx<'_> { pub(super) fn run_incremental_test(&self) { // Basic plan for a test incremental/foo/bar.rs: // - load list of revisions rpass1, cfail2, rpass3 - // - each should begin with `cpass`, `rpass`, `cfail`, or `rfail` + // - each should begin with `cfail`, `cpass`, or `rpass` // - if `cpass`, expect compilation to succeed, don't execute // - if `rpass`, expect compilation and execution to succeed // - if `cfail`, expect compilation to fail - // - if `rfail`, expect compilation to succeed and execution to fail // - create a directory build/foo/bar.incremental // - compile foo/bar.rs with -C incremental=.../foo/bar.incremental and -C rpass1 // - because name of revision starts with "rpass", expect success // - compile foo/bar.rs with -C incremental=.../foo/bar.incremental and -C cfail2 // - because name of revision starts with "cfail", expect an error - // - load expected errors as usual, but filter for those that end in `[rfail2]` + // - load expected errors as usual, but filter for those with `[cfail2]` // - compile foo/bar.rs with -C incremental=.../foo/bar.incremental and -C rpass3 // - because name of revision starts with "rpass", expect success // - execute build/foo/bar.exe and save output @@ -43,15 +42,10 @@ pub(super) fn run_incremental_test(&self) { self.fatal("can only use should-ice in cfail tests"); } self.run_rpass_test(); - } else if revision.starts_with("rfail") { - if self.props.should_ice { - self.fatal("can only use should-ice in cfail tests"); - } - self.run_rfail_test(); } else if revision.starts_with("cfail") { self.run_cfail_test(); } else { - self.fatal("revision name must begin with cpass, rpass, rfail, or cfail"); + self.fatal("revision name must begin with `cfail`, `cpass`, or `rpass`"); } } @@ -111,24 +105,4 @@ fn run_cfail_test(&self) { self.check_forbid_output(&output_to_check, &proc_res); } - - fn run_rfail_test(&self) { - let pm = self.pass_mode(); - let should_run = self.run_if_enabled(); - let proc_res = self.compile_test(should_run, self.should_emit_metadata(pm)); - - if !proc_res.status.success() { - self.fatal_proc_rec("compilation failed!", &proc_res); - } - - if let WillExecute::Disabled = should_run { - return; - } - - let proc_res = self.exec_compiled_test(); - - let output_to_check = self.get_output(&proc_res); - self.check_correct_failure_status(&proc_res); - self.check_all_error_patterns(&output_to_check, &proc_res); - } } diff --git a/tests/incremental/issue-80691-bad-eval-cache.rs b/tests/incremental/issue-80691-bad-eval-cache.rs deleted file mode 100644 index 2d3dbf6fed66..000000000000 --- a/tests/incremental/issue-80691-bad-eval-cache.rs +++ /dev/null @@ -1,186 +0,0 @@ -//@ revisions: rfail1 rfail2 -//@ failure-status: 101 -//@ error-pattern: not implemented -//@ needs-unwind -Cpanic=abort causes abort instead of exit(101) -//@ ignore-backends: gcc - -pub trait Interner { - type InternedVariableKinds; -} - -trait RustIrDatabase { - fn associated_ty_data(&self) -> AssociatedTyDatum; - fn impl_datum(&self) -> ImplDatum; -} - -trait Fold { - type Result; -} -impl Fold for Binders -where - T: HasInterner + Fold, - >::Result: HasInterner, - I: Interner, -{ - type Result = Binders; -} -impl Fold for WhereClause { - type Result = Binders>; -} - -trait HasInterner { - type Interner: Interner; -} -impl HasInterner for Vec { - type Interner = T::Interner; -} -impl HasInterner for &T { - type Interner = T::Interner; -} - -pub struct VariableKind { - _marker: std::marker::PhantomData, -} - -struct VariableKinds { - _interned: I::InternedVariableKinds, -} - -struct WhereClause { - _marker: std::marker::PhantomData, -} -impl HasInterner for WhereClause { - type Interner = I; -} - -struct Binders { - _marker: std::marker::PhantomData, -} -impl HasInterner for Binders { - type Interner = T::Interner; -} -impl Binders<&T> { - fn cloned(self) -> Binders { - unimplemented!() - } -} -impl Binders { - fn map_ref<'a, U, OP>(&'a self, _op: OP) -> Binders - where - OP: FnOnce(&'a T) -> U, - U: HasInterner, - { - unimplemented!() - } -} -impl Binders -where - T: Fold + HasInterner, - I: Interner, -{ - fn substitute(self) -> T::Result { - unimplemented!() - } -} -impl IntoIterator for Binders -where - V: HasInterner + IntoIterator, - U: HasInterner, -{ - type Item = Binders; - type IntoIter = BindersIntoIterator; - fn into_iter(self) -> Self::IntoIter { - unimplemented!() - } -} -struct BindersIntoIterator { - _binders: VariableKinds, -} -impl Iterator for BindersIntoIterator -where - V: HasInterner + IntoIterator, - ::Item: HasInterner, -{ - type Item = Binders<::Item>; - fn next(&mut self) -> Option { - unimplemented!() - } -} - -struct ImplDatum { - binders: Binders>, -} -struct ImplDatumBound { - where_clauses: Vec>>, -} -impl HasInterner for ImplDatumBound { - type Interner = I; -} - -struct AssociatedTyDatum { - binders: Binders>, -} - -struct AssociatedTyDatumBound { - where_clauses: Vec>>, -} -impl HasInterner for AssociatedTyDatumBound { - type Interner = I; -} - -struct ClauseBuilder<'me, I: Interner> { - db: &'me dyn RustIrDatabase, -} -impl<'me, I: Interner> ClauseBuilder<'me, I> { - fn new() -> Self { - unimplemented!() - } - fn push_clause(&mut self, _conditions: impl Iterator>>>) { - unimplemented!() - } -} - -pub(crate) struct Forest { - _marker: std::marker::PhantomData, -} - -impl Forest { - fn iter_answers<'f>(&'f self) { - let builder = &mut ClauseBuilder::::new(); - let impl_datum = builder.db.impl_datum(); - let impl_where_clauses = impl_datum - .binders - .map_ref(|b| &b.where_clauses) - .into_iter() - .map(|wc| wc.cloned().substitute()); - let associated_ty = builder.db.associated_ty_data(); - let assoc_ty_where_clauses = associated_ty - .binders - .map_ref(|b| &b.where_clauses) - .into_iter() - .map(|wc| wc.cloned().substitute()); - builder.push_clause(impl_where_clauses.chain(assoc_ty_where_clauses)); - } -} - -pub struct SLGSolver { - pub(crate) forest: Forest, -} -impl SLGSolver { - fn new() -> Self { - unimplemented!() - } - fn solve_multiple(&self) { - let _answers = self.forest.iter_answers(); - } -} - -pub struct ChalkIr; -impl Interner for ChalkIr { - type InternedVariableKinds = Vec>; -} - -fn main() { - let solver = SLGSolver::new(); - solver.solve_multiple(); -}