Skip to content

Don't use a generator for BoxedResolver #85885

New issue

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

#

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

Already on GitHub? # to your account

Jump to bottom
Merged
merged 17 commits into from
Jun 11, 2021
Merged

Don't use a generator for BoxedResolver #85885

merged 17 commits into from
Jun 11, 2021

Conversation

bjorn3
Copy link
Member

@bjorn3 bjorn3 commented Jun 1, 2021

The generator is non-trivial and requires unsafe code anyway. Using regular unsafe code without a generator is much easier to follow.

Based on #85810 as it touches rustc_interface too.

@bjorn3 bjorn3 added C-cleanup Category: PRs that clean code up or issues documenting cleanup. A-driver Area: rustc_driver that ties everything together into the `rustc` compiler T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 1, 2021
@rust-highfive
Copy link
Contributor

r? @varkor

(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 Jun 1, 2021
@bjorn3 bjorn3 force-pushed the remove_box_region branch from 17b655b to 2868764 Compare June 1, 2021 09:31
@rust-log-analyzer

This comment has been minimized.

# to view
@cjgillot cjgillot self-assigned this Jun 2, 2021
@bors
Copy link
Collaborator

bors commented Jun 4, 2021

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

@bjorn3 bjorn3 force-pushed the remove_box_region branch from d02d165 to c63feb0 Compare June 4, 2021 11:21
bjorn3 added 14 commits June 8, 2021 19:24
@bjorn3
Simplify box_region macros
8f397bc
@bjorn3
Inline box_region macro calls
86b3ebe
@bjorn3
Inline the rest of box_region
99e112d
@bjorn3
Inline PinnedGenerator
1b7ec34
@bjorn3
Use more accurate lifetimes
bddf151
@bjorn3
Replace Pin::new with .as_mut()
ecc68e7
@bjorn3
Store boxed metadata loader in CrateLoader
db4d8e2
@bjorn3
Let several methods take &Resolver instead of a BoxedResolver wrapper
d376f03
@bjorn3
Inline two more methods
9d9ccec
@bjorn3
Don't return a BoxedResolver on errors
36bdfdc
@bjorn3
Don't use a generator for BoxedResolver
86c2d1a 
The generator is non-trivial and requires unsafe code anyway. Using
regular unsafe code without a generator is much easier to follow.
@bjorn3
Use a submodule as safety boundary for BoxedResolver
5e14820
@bjorn3
Revert "Let several methods take &Resolver instead of a BoxedResolver…
cf1f92a 
… wrapper"

This reverts commit 5343ec338f72a61e2f51f9d90117092c8e8a725a.
@bjorn3
Don't require LintStore to live for 'a in configure_and_expand_inner
2bf839e
@bjorn3 bjorn3 force-pushed the remove_box_region branch from c63feb0 to 2bf839e Compare June 8, 2021 17:24
cjgillot
cjgillot approved these changes Jun 8, 2021
View reviewed changes
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.

Overall LGTM, and a nice simplification.

@varkor
Copy link
Member

varkor commented Jun 8, 2021

r? @cjgillot

@cjgillot
Copy link
Contributor

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jun 10, 2021
@bors
Copy link
Collaborator

bors commented Jun 10, 2021

⌛ Trying commit 4301d1e with merge ec4ab094f160363a91a8d43faf18b30b2b17ab44...

@bors
Copy link
Collaborator

bors commented Jun 10, 2021

☀️ Try build successful - checks-actions
Build commit: ec4ab094f160363a91a8d43faf18b30b2b17ab44 (ec4ab094f160363a91a8d43faf18b30b2b17ab44)

@rust-timer
Copy link
Collaborator

Queued ec4ab094f160363a91a8d43faf18b30b2b17ab44 with parent 1f949e9, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking try commit (ec4ab094f160363a91a8d43faf18b30b2b17ab44): comparison url.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying rollup- to bors.

Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jun 10, 2021
@cjgillot
Copy link
Contributor

@bors r+

@bors
Copy link
Collaborator

bors commented Jun 11, 2021

📌 Commit 4301d1e has been approved by cjgillot

@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 Jun 11, 2021
@bors
Copy link
Collaborator

bors commented Jun 11, 2021

⌛ Testing commit 4301d1e with merge 0a8629b...

@bors
Copy link
Collaborator

bors commented Jun 11, 2021

☀️ Test successful - checks-actions
Approved by: cjgillot
Pushing 0a8629b to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jun 11, 2021
@bors bors merged commit 0a8629b into rust-lang:master Jun 11, 2021
@rustbot rustbot added this to the 1.54.0 milestone Jun 11, 2021
@bjorn3 bjorn3 deleted the remove_box_region branch June 11, 2021 19:10
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@cjgillot cjgillot cjgillot approved these changes

Assignees

@cjgillot cjgillot

Labels
A-driver Area: rustc_driver that ties everything together into the `rustc` compiler C-cleanup Category: PRs that clean code up or issues documenting cleanup. 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. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.54.0
Development

Successfully merging this pull request may close these issues.
8 participants
@bjorn3 @rust-highfive @rust-log-analyzer @bors @varkor @cjgillot @rust-timer @rustbot