Skip to content
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

More codegen cleanups #112830

Merged
merged 2 commits into from
Jun 21, 2023
Merged

More codegen cleanups #112830

merged 2 commits into from
Jun 21, 2023

Conversation

nnethercote
Copy link
Contributor

Some additional cleanups I found while looking closely at this code, following up from #112827.

r= @oli-obk

@rustbot
Copy link
Collaborator

rustbot commented Jun 20, 2023

r? @jackh726

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

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 20, 2023
Copy link
Contributor

@oli-obk oli-obk left a comment

Choose a reason for hiding this comment

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

r=me with nit

@oli-obk
Copy link
Contributor

oli-obk commented Jun 20, 2023

r? @oli-obk

@rustbot rustbot assigned oli-obk and unassigned jackh726 Jun 20, 2023
@rust-log-analyzer

This comment has been minimized.

They both match on a `WorkItem`. It's simpler to do it all in one place.
There's no need to store it in `Queries`. We can just use a local
variable, because it's always used shortly after it's produced.

The commit also removes the `tcx.analysis()` call in `ongoing_codegen`,
because it's easy to ensure that's done beforehand.

All this makes the dataflow within `run_compiler` easier to follow, at
the cost of making one test slightly more verbose, which I think is a
good tradeoff.
@nnethercote nnethercote force-pushed the more-codegen-cleanups branch from d44cf57 to 1da1348 Compare June 21, 2023 01:30
@nnethercote
Copy link
Contributor Author

The first commit caused the crash, and I don't understand why. I have removed it, I'll try doing it in a separate PR. The other two commits can be merged here.

@bors r=oli-obk rollup

@bors
Copy link
Contributor

bors commented Jun 21, 2023

📌 Commit 1da1348 has been approved by oli-obk

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 Jun 21, 2023
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 21, 2023
Rollup of 6 pull requests

Successful merges:

 - rust-lang#112632 (Implement PartialOrd for `Vec`s over different allocators)
 - rust-lang#112759 (Make closure_saved_names_of_captured_variables a query. )
 - rust-lang#112772 (Add a fully fledged `Clause` type, rename old `Clause` to `ClauseKind`)
 - rust-lang#112790 (Syntactically accept `become` expressions (explicit tail calls experiment))
 - rust-lang#112830 (More codegen cleanups)
 - rust-lang#112844 (Add retag in MIR transform: `Adt` for `Unique` may contain a reference)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 904994e into rust-lang:master Jun 21, 2023
@rustbot rustbot added this to the 1.72.0 milestone Jun 21, 2023
@nnethercote nnethercote deleted the more-codegen-cleanups branch June 21, 2023 22:04
@nnethercote
Copy link
Contributor Author

The first commit caused the crash, and I don't understand why. I have removed it, I'll try doing it in a separate PR.

It was because I changed the type of __rustc_codegen_backend but didn't also change its callers. (This isn't caught by the type checker because of the code structure.) I found a simpler way to do it that doesn't change the type in #112913.

flip1995 pushed a commit to flip1995/rust that referenced this pull request Jun 30, 2023
Rollup of 6 pull requests

Successful merges:

 - rust-lang#112632 (Implement PartialOrd for `Vec`s over different allocators)
 - rust-lang#112759 (Make closure_saved_names_of_captured_variables a query. )
 - rust-lang#112772 (Add a fully fledged `Clause` type, rename old `Clause` to `ClauseKind`)
 - rust-lang#112790 (Syntactically accept `become` expressions (explicit tail calls experiment))
 - rust-lang#112830 (More codegen cleanups)
 - rust-lang#112844 (Add retag in MIR transform: `Adt` for `Unique` may contain a reference)

r? `@ghost`
`@rustbot` modify labels: rollup
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
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
Development

Successfully merging this pull request may close these issues.

6 participants