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.

Sign up for GitHub

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

Jump to bottom

Check for stale files in src/test/mir-opt #97564

Closed
JakobDegen opened this issue May 30, 2022 · 8 comments · Fixed by #103781
Closed

Check for stale files in src/test/mir-opt #97564

JakobDegen opened this issue May 30, 2022 · 8 comments · Fixed by #103781
Assignees
@JakobDegen
Labels
A-mir-opt Area: MIR optimizations A-testsuite Area: The testsuite used to check the correctness of rustc C-enhancement Category: An issue proposing an enhancement or a PR with one.

Comments

@JakobDegen
Copy link
Contributor

JakobDegen commented May 30, 2022 •
edited by rustbot
Loading

This folder currently contains a number of inst_combine_deref*.diff files that are completely unused - the associated .rs file that generated them was removed all the way back in #81101 . In addition to deleting these files, we should also add a check to compiletest that makes sure that the mir-opt folder contains no files that do not belong to any tests (with the exception of the readme).

@rustbot label +C-enhancement +A-test-suite +A-mir-opt
@JakobDegen

This comment was marked as outdated.

Sign in to view
@JakobDegen

This comment was marked as outdated.

Sign in to view
1 similar comment
@JakobDegen

This comment was marked as outdated.

Sign in to view
@JakobDegen
Copy link
Contributor Author

I thought I was getting the command wrong, maybe the bot is down?

@compiler-errors compiler-errors added A-testsuite Area: The testsuite used to check the correctness of rustc C-enhancement Category: An issue proposing an enhancement or a PR with one. A-mir-opt Area: MIR optimizations labels May 30, 2022
@compiler-errors
Copy link
Member

compiler-errorsbot added them for you because rustbot is on holiday 😝

@JakobDegen
Copy link
Contributor Author

🤣 ❤️ thank you!

@JakobDegen JakobDegen mentioned this issue May 30, 2022
Merged
@ehuss
Copy link
Contributor

ehuss commented May 31, 2022

See also #77498.

Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue May 31, 2022
@Dylan-DPC
Rollup merge of rust-lang#97570 - JakobDegen:dse-test, r=tmiasko
ca4a9ac 
Fix TLS access mir opt test and remove stale files

Thanks `@pietroalbini` for noticing that the TLS test was not doing what it was supposed to. Switched to `PreCodegen` because `SimplifyCfg` does not run on opt level 0.

Also addresses the easy part of rust-lang#97564 .

r? rust-lang/mir-opt
bors added a commit to rust-lang-ci/rust that referenced this issue May 31, 2022
@bors
Auto merge of rust-lang#97570 - JakobDegen:dse-test, r=tmiasko
0595ea1 
Fix TLS access mir opt test and remove stale files

Thanks `@pietroalbini` for noticing that the TLS test was not doing what it was supposed to. Switched to `PreCodegen` because `SimplifyCfg` does not run on opt level 0.

Also addresses the easy part of rust-lang#97564 .

r? rust-lang/mir-opt
@JakobDegen JakobDegen mentioned this issue Sep 5, 2022
Merged
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue Sep 7, 2022
@Dylan-DPC
Rollup merge of rust-lang#101435 - JakobDegen:bitwidth-tests, r=wesle…
d81aa36 
…ywiser

Remove unnecessary `EMIT_MIR_FOR_EACH_BITWIDTH`

This commit removes the annotation only for those tests where the 32 bit and 64 bit files were exactly identical. I didn't touch anything in the `mir-opt/const` directory, since having this annotation there seems more principled, even if it doesn't make a difference.

This also removes four additional files related to the `separate_const_switch.rs` test. The associated annotations were removed in rust-lang#100827 , but I forgot to remove the files as well. (rust-lang#97564 is the issue tracking an automated check here)

r? ``@wesleywiser``
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Sep 7, 2022
@matthiaskrgr
Rollup merge of rust-lang#101435 - JakobDegen:bitwidth-tests, r=wesle…
1d65e96 
…ywiser

Remove unnecessary `EMIT_MIR_FOR_EACH_BITWIDTH`

This commit removes the annotation only for those tests where the 32 bit and 64 bit files were exactly identical. I didn't touch anything in the `mir-opt/const` directory, since having this annotation there seems more principled, even if it doesn't make a difference.

This also removes four additional files related to the `separate_const_switch.rs` test. The associated annotations were removed in rust-lang#100827 , but I forgot to remove the files as well. (rust-lang#97564 is the issue tracking an automated check here)

r? ```@wesleywiser```
@JakobDegen
Copy link
Contributor Author

@rustbot claim

This has bitten me once too many times now in #101428 , gonna fix it

@JakobDegen JakobDegen mentioned this issue Oct 31, 2022
Merged
@bors bors closed this as completed in e2a6d28 Nov 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@JakobDegen JakobDegen

Labels
A-mir-opt Area: MIR optimizations A-testsuite Area: The testsuite used to check the correctness of rustc C-enhancement Category: An issue proposing an enhancement or a PR with one.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Detect unused files in src/test/mir-opt and error on them in tidy. JakobDegen/rust
3 participants
@ehuss @compiler-errors @JakobDegen