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 match arm scopes and other scope fixes #60174

Merged
merged 11 commits into from
May 23, 2019

Conversation

matthewjasper
Copy link
Contributor

  • Add drop and lint scopes for match arms.
  • Lint attributes are now respected on match arms.
  • Make sure we emit a StorageDead if we diverge when initializing a temporary.
  • Adjust MIR pretty printing of scopes for locals.
  • Don't generate duplicate lint scopes for let statements.
  • Add some previously missing fake borrows for matches.

closes #46525

cc @rust-lang/compiler

@rust-highfive
Copy link
Collaborator

r? @eddyb

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 22, 2019
@matthewjasper
Copy link
Contributor Author

@bors try

This probably should be cratered because it breaks code uses #[deny(...)] on a match arm and then does the thing that was denied. This seems unlikely.
This also probably needs a perf run.

@bors
Copy link
Contributor

bors commented Apr 22, 2019

⌛ Trying commit 2feb6385c62f996092c3883f26918aa9af810fe4 with merge 49a98abd081547c1b12687ed2ca0de70f8d72029...

src/librustc/cfg/construct.rs Show resolved Hide resolved
src/librustc_mir/build/expr/as_temp.rs Outdated Show resolved Hide resolved
src/librustc_mir/build/expr/as_temp.rs Outdated Show resolved Hide resolved
src/librustc_mir/build/scope.rs Outdated Show resolved Hide resolved
src/librustc_mir/dataflow/impls/mod.rs Outdated Show resolved Hide resolved
@Centril
Copy link
Contributor

Centril commented Apr 22, 2019

  • Add drop and lint scopes for match arms.
  • Lint attributes are now respected on match arms.

Seems right, but cc @rust-lang/lang

@matthewjasper matthewjasper force-pushed the add-match-arm-scopes branch 2 times, most recently from 5a387e2 to a8a9153 Compare April 22, 2019 18:38
@eddyb
Copy link
Member

eddyb commented Apr 23, 2019

@matthewjasper I just rebased #56278, can you check if your PR breaks anything I want to enforce in that PR?
Also, feel free to review it, if you have the time.

@matthewjasper
Copy link
Contributor Author

@eddyb It looks like there shouldn't be any problems there.

@bors
Copy link
Contributor

bors commented Apr 24, 2019

☔ The latest upstream changes (presumably #60224) made this pull request unmergeable. Please resolve the merge conflicts.

@matthewjasper
Copy link
Contributor Author

@bors try

@bors
Copy link
Contributor

bors commented Apr 24, 2019

⌛ Trying commit a510e30ca8310916760e246847b0be13db4ce844 with merge 74092351c05ae61047b2be02024415546595e974...

@bors
Copy link
Contributor

bors commented Apr 24, 2019

☀️ Try build successful - checks-travis
Build commit: 74092351c05ae61047b2be02024415546595e974

@estebank
Copy link
Contributor

@craterbot run start=master#e305df1846a6d985315917ae0c81b74af8b4e641 end=try#74092351c05ae61047b2be02024415546595e974 mode=check-only

@craterbot
Copy link
Collaborator

👌 Experiment pr-60174 created and queued.
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot craterbot added S-waiting-on-crater Status: Waiting on a crater run to be completed. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 24, 2019
@craterbot
Copy link
Collaborator

🚧 Experiment pr-60174 is now running on agent aws-3-tmp.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@estebank
Copy link
Contributor

@rust-timer build 74092351c05ae61047b2be02024415546595e974

@rust-timer
Copy link
Collaborator

Success: Queued 74092351c05ae61047b2be02024415546595e974 with parent e305df1, comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking try commit 74092351c05ae61047b2be02024415546595e974

@craterbot
Copy link
Collaborator

🎉 Experiment pr-60174 is completed!
📊 0 regressed and 0 fixed (60951 total)
📰 Open the full report.

⚠️ If you notice any spurious failure please add them to the blacklist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot craterbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-crater Status: Waiting on a crater run to be completed. labels Apr 26, 2019
matthewjasper and others added 9 commits May 21, 2019 19:37
* Don't generate an extra lint scope for each `let` statement.
* Place match guards inside the visiblility scope of the bindings for
  their arm.
Also give arms the correct lint scope in MIR.
I was incorrectly under the impression that this would only lead to
duplicates. See `mir-opt/match-arm-scope.rs` (upcomming commit) for a
case where we didn't emit a fake borrow of `items.1`.
This ensures that we will correctly generate a storage-dead if the
initializing expression diverges.
Co-Authored-By: matthewjasper <mjjasper1@gmail.com>
@matthewjasper matthewjasper force-pushed the add-match-arm-scopes branch from 5b5255d to 0835048 Compare May 21, 2019 19:35
@matthewjasper
Copy link
Contributor Author

@bors r=pnkfelix

@bors
Copy link
Contributor

bors commented May 22, 2019

📌 Commit 0835048 has been approved by pnkfelix

@bors
Copy link
Contributor

bors commented May 22, 2019

⌛ Testing commit 0835048 with merge 79e50518a01ed5b42b0beb321f95c39311ae54b5...

@pietroalbini
Copy link
Member

@bors retry

Yielding priority to beta.

@bors
Copy link
Contributor

bors commented May 22, 2019

⌛ Testing commit 0835048 with merge 11f75e1080e60a8130dfecf39aa89935921892cf...

@pietroalbini
Copy link
Member

@bors retry

@bors
Copy link
Contributor

bors commented May 22, 2019

⌛ Testing commit 0835048 with merge 6f77e237130fe0e7b2c73aa930f778aa9d4081cd...

@pietroalbini
Copy link
Member

@bors retry

@rust-highfive
Copy link
Collaborator

Your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
Cloning into 'rust-lang/rust'...
travis_time:end:2d17cc3a:start=1558532963031480524,finish=1558532969010004454,duration=5978523930
$ cd rust-lang/rust
$ git checkout -qf 6f77e237130fe0e7b2c73aa930f778aa9d4081cd
fatal: reference is not a tree: 6f77e237130fe0e7b2c73aa930f778aa9d4081cd
The command "git checkout -qf 6f77e237130fe0e7b2c73aa930f778aa9d4081cd" failed and exited with 128 during .
Your build has been stopped.

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@bors
Copy link
Contributor

bors commented May 23, 2019

⌛ Testing commit 0835048 with merge 85334c5...

bors added a commit that referenced this pull request May 23, 2019
Add match arm scopes and other scope fixes

* Add drop and lint scopes for match arms.
* Lint attributes are now respected on match arms.
* Make sure we emit a StorageDead if we diverge when initializing a temporary.
* Adjust MIR pretty printing of scopes for locals.
* Don't generate duplicate lint scopes for `let statements`.
* Add some previously missing fake borrows for matches.

closes #46525

cc @rust-lang/compiler
@bors
Copy link
Contributor

bors commented May 23, 2019

☀️ Test successful - checks-travis, status-appveyor
Approved by: pnkfelix
Pushing 85334c5 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label May 23, 2019
@bors bors merged commit 0835048 into rust-lang:master May 23, 2019
@matthewjasper matthewjasper deleted the add-match-arm-scopes branch May 24, 2019 22:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. 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.

match arm bindings have weird lifetimes
10 participants