Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Delete tests/crashes/23707.rs because it's flaky #132312

Merged
merged 1 commit into from
Oct 29, 2024

Conversation

jieyouxu
Copy link
Member

It's conditioned on only-x86_64 because it doesn't reliably fail on other platforms, it's optimization dependent and failed to ICE post-PGO in
#132300 (comment). Remove this test for now without prejudice against relanding the test in a more reliable form.

I removed the S-bug-has-test label from #23707.

r? compiler

It's conditioned on `only-x86_64` because it doesn't reliably fail on
other platforms, it's optimization dependent and failed to ICE post-PGO
in
<rust-lang#132300 (comment)>.
Remove this test for now without prejudice against relanding the test in
a more reliable form.
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 29, 2024
@matthiaskrgr
Copy link
Member

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Oct 29, 2024

📌 Commit 6c93c65 has been approved by matthiaskrgr

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 29, 2024
@matthiaskrgr
Copy link
Member

otoh, maybe we can change the recursion limit to make it fail more reliable?

@jieyouxu
Copy link
Member Author

otoh, maybe we can change the recursion limit to make it fail more reliable?

I don't want to investigate tuning this test atm, follow-ups welcomed!

@workingjubilee
Copy link
Member

while that can hypothetically be done, it presumes LLVM cannot find, and that the code does not change to enable, an optimization that allows rustc to push through even millions of iterations before reaching the error.

@workingjubilee
Copy link
Member

workingjubilee commented Oct 29, 2024

in general, resource-exhaustion-related tests are inappropriate for crashtests, because they are intrinsically optimization-dependent (and because they then may only fail when the compiler successfully finds an optimization it might not later find in a later version... essentially nondeterministically).

it may be best to simply add a regression test for a lower level of recursion that the compiler can reliably clear, declare victory, and move on.

@jieyouxu jieyouxu assigned matthiaskrgr and unassigned TaKO8Ki Oct 29, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 29, 2024
…kingjubilee

Rollup of 12 pull requests

Successful merges:

 - rust-lang#131375 (compiler: apply clippy::clone_on_ref_ptr for CI)
 - rust-lang#131520 (Mark `str::is_char_boundary` and `str::split_at*` unstably `const`.)
 - rust-lang#132119 (Hack out effects support for old solver)
 - rust-lang#132194 (Collect item bounds for RPITITs from trait where clauses just like associated types)
 - rust-lang#132216 (correct LLVMRustCreateThinLTOData arg types)
 - rust-lang#132233 (Split `boxed.rs` into a few modules)
 - rust-lang#132266 (riscv-soft-abi-with-float-features.rs: adapt for LLVM 20)
 - rust-lang#132270 (clarified doc for `std::fs::OpenOptions.truncate()`)
 - rust-lang#132284 (Remove my ping for rustdoc/clean/types.rs)
 - rust-lang#132293 (Remove myself from mentions inside `tests/ui/check-cfg` directory)
 - rust-lang#132312 (Delete `tests/crashes/23707.rs` because it's flaky)
 - rust-lang#132313 (compiletest: Rename `command-list.rs` to `directive-list.rs`)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 0963636 into rust-lang:master Oct 29, 2024
6 checks passed
@rustbot rustbot added this to the 1.84.0 milestone Oct 29, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Oct 29, 2024
Rollup merge of rust-lang#132312 - jieyouxu:delete-crashes-23707, r=matthiaskrgr

Delete `tests/crashes/23707.rs` because it's flaky

It's conditioned on `only-x86_64` because it doesn't reliably fail on other platforms, it's optimization dependent and failed to ICE post-PGO in
<rust-lang#132300 (comment)>. Remove this test for now without prejudice against relanding the test in a more reliable form.

I removed the `S-bug-has-test` label from rust-lang#23707.

r? compiler
djkoloski pushed a commit to djkoloski/rust that referenced this pull request Oct 29, 2024
…kingjubilee

Rollup of 12 pull requests

Successful merges:

 - rust-lang#131375 (compiler: apply clippy::clone_on_ref_ptr for CI)
 - rust-lang#131520 (Mark `str::is_char_boundary` and `str::split_at*` unstably `const`.)
 - rust-lang#132119 (Hack out effects support for old solver)
 - rust-lang#132194 (Collect item bounds for RPITITs from trait where clauses just like associated types)
 - rust-lang#132216 (correct LLVMRustCreateThinLTOData arg types)
 - rust-lang#132233 (Split `boxed.rs` into a few modules)
 - rust-lang#132266 (riscv-soft-abi-with-float-features.rs: adapt for LLVM 20)
 - rust-lang#132270 (clarified doc for `std::fs::OpenOptions.truncate()`)
 - rust-lang#132284 (Remove my ping for rustdoc/clean/types.rs)
 - rust-lang#132293 (Remove myself from mentions inside `tests/ui/check-cfg` directory)
 - rust-lang#132312 (Delete `tests/crashes/23707.rs` because it's flaky)
 - rust-lang#132313 (compiletest: Rename `command-list.rs` to `directive-list.rs`)

r? `@ghost`
`@rustbot` modify labels: rollup
@jieyouxu jieyouxu deleted the delete-crashes-23707 branch October 30, 2024 03:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants