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

Add FileCheck annotations to dataflow-const-prop tests #119759

Merged
merged 19 commits into from
Feb 5, 2024

Conversation

sfzhu93
Copy link
Contributor

@sfzhu93 sfzhu93 commented Jan 9, 2024

part of #116971.

A few shadowing variable names are changed, so that it is easier to match the variable names in MIR using FileCheck syntax.

Also, there's a FIXME in enum.rs because the MIR looks suspicious to me. It has been explained in the comments.

r? cjgillot

@rustbot
Copy link
Collaborator

rustbot commented Jan 9, 2024

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @cjgillot (or someone else) soon.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

  • @rustbot author: the review is finished, PR author should check the comments and take action accordingly
  • @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 9, 2024
@rust-log-analyzer

This comment has been minimized.

Copy link
Contributor

@cjgillot cjgillot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot @sfzhu93!
Some remarks are generic, about simplifications to the patterns.
Some others are more substantial about what we are trying to test.

tests/mir-opt/dataflow-const-prop/array_index.rs Outdated Show resolved Hide resolved
tests/mir-opt/dataflow-const-prop/checked.rs Show resolved Hide resolved
tests/mir-opt/dataflow-const-prop/enum.rs Outdated Show resolved Hide resolved
tests/mir-opt/dataflow-const-prop/enum.rs Show resolved Hide resolved
tests/mir-opt/dataflow-const-prop/if.rs Outdated Show resolved Hide resolved
tests/mir-opt/dataflow-const-prop/if.rs Outdated Show resolved Hide resolved
tests/mir-opt/dataflow-const-prop/if.rs Show resolved Hide resolved
tests/mir-opt/dataflow-const-prop/repeat.rs Outdated Show resolved Hide resolved
Copy link
Contributor Author

@sfzhu93 sfzhu93 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cjgillot Thank you for your comment! I updated according to your comments. For some trick cases that we may need to take a closer look or even file a issue/pr in the future, I added a few fixmes.

tests/mir-opt/dataflow-const-prop/array_index.rs Outdated Show resolved Hide resolved
tests/mir-opt/dataflow-const-prop/checked.rs Outdated Show resolved Hide resolved
tests/mir-opt/dataflow-const-prop/checked.rs Outdated Show resolved Hide resolved
tests/mir-opt/dataflow-const-prop/default_boxed_slice.rs Outdated Show resolved Hide resolved
tests/mir-opt/dataflow-const-prop/enum.rs Outdated Show resolved Hide resolved
tests/mir-opt/dataflow-const-prop/enum.rs Outdated Show resolved Hide resolved
tests/mir-opt/dataflow-const-prop/inherit_overflow.rs Outdated Show resolved Hide resolved
tests/mir-opt/dataflow-const-prop/large_array_index.rs Outdated Show resolved Hide resolved
tests/mir-opt/dataflow-const-prop/repeat.rs Outdated Show resolved Hide resolved
@cjgillot
Copy link
Contributor

Great @sfzhu93!
@bors r+

@bors
Copy link
Contributor

bors commented Jan 26, 2024

📌 Commit 65b1083 has been approved by cjgillot

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 Jan 26, 2024
Nadrieril added a commit to Nadrieril/rust that referenced this pull request Jan 27, 2024
Add FileCheck annotations to dataflow-const-prop tests

part of rust-lang#116971.

A few shadowing variable names are changed, so that it is easier to match the variable names in MIR using FileCheck syntax.

Also, there's a FIXME in [enum.rs](https://github.com/rust-lang/rust/pull/119759/files#diff-7621f55327838e489a95ac99ae1e6126b37c57aff582594e6bee9d7e7e56fc58) because the MIR looks suspicious to me. It has been explained in the comments.

r? cjgillot
Nadrieril added a commit to Nadrieril/rust that referenced this pull request Jan 27, 2024
Add FileCheck annotations to dataflow-const-prop tests

part of rust-lang#116971.

A few shadowing variable names are changed, so that it is easier to match the variable names in MIR using FileCheck syntax.

Also, there's a FIXME in [enum.rs](https://github.com/rust-lang/rust/pull/119759/files#diff-7621f55327838e489a95ac99ae1e6126b37c57aff582594e6bee9d7e7e56fc58) because the MIR looks suspicious to me. It has been explained in the comments.

r? cjgillot
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 27, 2024
Rollup of 9 pull requests

Successful merges:

 - rust-lang#111379 (Boost iterator intersperse(_with) performance)
 - rust-lang#118182 (Properly recover from trailing attr in body)
 - rust-lang#119641 (Remove feature not required by `Ipv6Addr::to_cononical` doctest)
 - rust-lang#119759 (Add FileCheck annotations to dataflow-const-prop tests)
 - rust-lang#120275 (Avoid ICE in trait without `dyn` lint)
 - rust-lang#120376 (Update codegen test for LLVM 18)
 - rust-lang#120386 (ScopeTree: remove destruction_scopes as unused)
 - rust-lang#120398 (Improve handling of numbers in `IntoDiagnosticArg`)
 - rust-lang#120399 (Remove myself from review rotation)

r? `@ghost`
`@rustbot` modify labels: rollup
@Nadrieril
Copy link
Member

Got this error in rollup, which I assume is from this

@sfzhu93 sfzhu93 deleted the branch rust-lang:master January 27, 2024 06:04
@sfzhu93 sfzhu93 closed this Jan 27, 2024
@sfzhu93 sfzhu93 deleted the master branch January 27, 2024 06:05
@sfzhu93
Copy link
Contributor Author

sfzhu93 commented Jan 27, 2024

I closed the PR by mistake during rebase and test, and cannot reopen. I submitted a ticket to GitHub customer support and see if it can be reopened.

The problem is, it should be unwind unreachable for wasm32. In the original test, it was unwind continue.

@cjgillot
Copy link
Contributor

2 ways to do this:

  • either you force-push the former commit, reopen, and force-push the rebased branch;
  • or you open a new PR.

@sfzhu93 sfzhu93 restored the master branch January 29, 2024 06:32
@sfzhu93 sfzhu93 reopened this Jan 29, 2024
@sfzhu93
Copy link
Contributor Author

sfzhu93 commented Jan 29, 2024

Successfully restored the original repo! I used GitHub's CLI to restore: gh pr checkout 119759. I have tested against wasm32-unknown-unknown locally, which should make bors rollup work. The CI we used here is less strict than bors.

@sfzhu93
Copy link
Contributor Author

sfzhu93 commented Jan 31, 2024

@rustbot ready
@cjgillot I guess I need your approval for bors rollup

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 31, 2024
@cjgillot
Copy link
Contributor

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Jan 31, 2024

📌 Commit 699b59c has been approved by cjgillot

It is now in the queue for this repository.

@bors bors removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 31, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Feb 1, 2024
Add FileCheck annotations to dataflow-const-prop tests

part of rust-lang#116971.

A few shadowing variable names are changed, so that it is easier to match the variable names in MIR using FileCheck syntax.

Also, there's a FIXME in [enum.rs](https://github.com/rust-lang/rust/pull/119759/files#diff-7621f55327838e489a95ac99ae1e6126b37c57aff582594e6bee9d7e7e56fc58) because the MIR looks suspicious to me. It has been explained in the comments.

r? cjgillot
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Feb 1, 2024
Add FileCheck annotations to dataflow-const-prop tests

part of rust-lang#116971.

A few shadowing variable names are changed, so that it is easier to match the variable names in MIR using FileCheck syntax.

Also, there's a FIXME in [enum.rs](https://github.com/rust-lang/rust/pull/119759/files#diff-7621f55327838e489a95ac99ae1e6126b37c57aff582594e6bee9d7e7e56fc58) because the MIR looks suspicious to me. It has been explained in the comments.

r? cjgillot
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 3, 2024
…iaskrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#119759 (Add FileCheck annotations to dataflow-const-prop tests)
 - rust-lang#120323 (On E0277 be clearer about implicit `Sized` bounds on type params and assoc types)
 - rust-lang#120473 (Only suggest removal of `as_*` and `to_` conversion methods on E0308)
 - rust-lang#120520 (Some cleanups around diagnostic levels.)
 - rust-lang#120540 (add test for try-block-in-match-arm)
 - rust-lang#120547 (`#![feature(inline_const_pat)]` is no longer incomplete)
 - rust-lang#120552 (Correctly check `never_type` feature gating)
 - rust-lang#120555 (put pnkfelix (me) back on the review queue.)
 - rust-lang#120556 (Improve the diagnostics for unused generic parameters)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 4, 2024
…iaskrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#119759 (Add FileCheck annotations to dataflow-const-prop tests)
 - rust-lang#120323 (On E0277 be clearer about implicit `Sized` bounds on type params and assoc types)
 - rust-lang#120473 (Only suggest removal of `as_*` and `to_` conversion methods on E0308)
 - rust-lang#120520 (Some cleanups around diagnostic levels.)
 - rust-lang#120540 (add test for try-block-in-match-arm)
 - rust-lang#120547 (`#![feature(inline_const_pat)]` is no longer incomplete)
 - rust-lang#120552 (Correctly check `never_type` feature gating)
 - rust-lang#120555 (put pnkfelix (me) back on the review queue.)
 - rust-lang#120556 (Improve the diagnostics for unused generic parameters)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 4, 2024
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#119759 (Add FileCheck annotations to dataflow-const-prop tests)
 - rust-lang#120323 (On E0277 be clearer about implicit `Sized` bounds on type params and assoc types)
 - rust-lang#120473 (Only suggest removal of `as_*` and `to_` conversion methods on E0308)
 - rust-lang#120540 (add test for try-block-in-match-arm)
 - rust-lang#120547 (`#![feature(inline_const_pat)]` is no longer incomplete)
 - rust-lang#120552 (Correctly check `never_type` feature gating)
 - rust-lang#120555 (put pnkfelix (me) back on the review queue.)
 - rust-lang#120556 (Improve the diagnostics for unused generic parameters)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 2b25957 into rust-lang:master Feb 5, 2024
11 checks passed
@rustbot rustbot added this to the 1.78.0 milestone Feb 5, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Feb 5, 2024
Rollup merge of rust-lang#119759 - sfzhu93:master, r=cjgillot

Add FileCheck annotations to dataflow-const-prop tests

part of rust-lang#116971.

A few shadowing variable names are changed, so that it is easier to match the variable names in MIR using FileCheck syntax.

Also, there's a FIXME in [enum.rs](https://github.com/rust-lang/rust/pull/119759/files#diff-7621f55327838e489a95ac99ae1e6126b37c57aff582594e6bee9d7e7e56fc58) because the MIR looks suspicious to me. It has been explained in the comments.

r? cjgillot
@sfzhu93 sfzhu93 deleted the master branch February 5, 2024 07:38
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