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

rustc_driver cleanup #83610

Merged
merged 14 commits into from
May 12, 2021
Merged

rustc_driver cleanup #83610

merged 14 commits into from
May 12, 2021

Conversation

bjorn3
Copy link
Member

@bjorn3 bjorn3 commented Mar 28, 2021

Best reviewed one commit at a time.

@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 labels Mar 28, 2021
@rust-highfive
Copy link
Collaborator

r? @matthewjasper

(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 Mar 28, 2021
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@bors

This comment has been minimized.

Sign in to view
@bjorn3
Copy link
Member Author

bjorn3 commented Mar 30, 2021

Rebased and added two extra commits.

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@bjorn3
Copy link
Member Author

bjorn3 commented Mar 30, 2021

ff2af40cd7d4d3373bfdc247f2ad8847213aea51 should be functionally identical. Can it be a spurious error?

@jayaddison jayaddison mentioned this pull request Apr 14, 2021
Merged
@crlf0710 crlf0710 added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 17, 2021
cjgillot
cjgillot reviewed May 2, 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.

I left a few comments.
The CI will re-run with the additional commits. We will see then if the failure is spurious.

compiler/rustc_driver/src/lib.rs Outdated Show resolved Hide resolved
compiler/rustc_driver/src/lib.rs Outdated Show resolved Hide resolved
compiler/rustc_driver/src/lib.rs Outdated Show resolved Hide resolved
compiler/rustc_middle/src/ty/context.rs Outdated Show resolved Hide resolved
bjorn3 added 2 commits May 2, 2021 18:00
@bjorn3
Let load_query_result_cache take a &DefPathTable
0447f91 
This allows removing a confusing mem::replace in create_global_ctxt
@bjorn3
Don't arena allocate Definitions
4a6cfc6
@bjorn3 bjorn3 force-pushed the driver_cleanup branch from bb61699 to 95a1cf7 Compare May 2, 2021 16:02
@bjorn3 bjorn3 force-pushed the driver_cleanup branch from 95a1cf7 to b71cd56 Compare May 2, 2021 16:12
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@bjorn3
Copy link
Member Author

bjorn3 commented May 3, 2021

CI passed.

@cjgillot
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented May 11, 2021

📌 Commit 163b480 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 May 11, 2021
@bors
Copy link
Contributor

bors commented May 12, 2021

⌛ Testing commit 163b480 with merge ac923d9...

@bors
Copy link
Contributor

bors commented May 12, 2021

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

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label May 12, 2021
@bors bors merged commit ac923d9 into rust-lang:master May 12, 2021
@rustbot rustbot added this to the 1.54.0 milestone May 12, 2021
This was referenced May 12, 2021
Merged
Merged
Merged
Merged
Merged
@bjorn3 bjorn3 deleted the driver_cleanup branch May 12, 2021 12:02
@pnkfelix
Copy link
Member

Performance triage indicates that this PR introduced a 1.7% regression when fully compiling coercions-debug.

I don't think any performance hit was expected. However, it also seems like this may be just noise.

bjorn3
bjorn3 commented May 18, 2021
View reviewed changes
compiler/rustc_middle/src/ty/context.rs
@@ -967,7 +967,7 @@ pub struct GlobalCtxt<'tcx> {
export_map: ExportMap<LocalDefId>,

pub(crate) untracked_crate: &'tcx hir::Crate<'tcx>,
pub(crate) definitions: &'tcx Definitions,
pub(crate) definitions: Definitions,
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the only change I can think of that could caused the regression if it isn't just noise.

bjorn3 pushed a commit to bjorn3/rust that referenced this pull request May 27, 2021
@bors
Auto merge of rust-lang#83610 - bjorn3:driver_cleanup, r=cjgillot
809722d 
rustc_driver cleanup

Best reviewed one commit at a time.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cjgillot cjgillot cjgillot left review comments

Assignees

@matthewjasper matthewjasper

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.
Projects
None yet
Milestone
1.54.0
Development

Successfully merging this pull request may close these issues.
9 participants
@bjorn3 @rust-highfive @rust-log-analyzer @bors @cjgillot @pnkfelix @crlf0710 @matthewjasper @rustbot