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

Don't use the HirId to NodeId map in MIR #71197

Merged
merged 3 commits into from
Apr 17, 2020
Merged

Conversation

ljedrz
Copy link
Contributor

@ljedrz ljedrz commented Apr 16, 2020

Another step towards not having to build a HirId to NodeId map other than for doc and RLS purposes.

We are currently sorting unsafe blocks by NodeId in check_unsafety; change it to sorting by Span instead; this passes the tests, but better ideas are welcome.

In addition, simplify the split between the used and unused unsafe blocks for readability and less sorting.

cc #50928

@rust-highfive
Copy link
Collaborator

r? @ecstatic-morse

(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 16, 2020
@ecstatic-morse
Copy link
Contributor

LGTM, I asked a clarifying question above, but that doesn't need to block this PR.

@bors r+

@bors
Copy link
Contributor

bors commented Apr 16, 2020

📌 Commit 672b768 has been approved by ecstatic-morse

@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 Apr 16, 2020
@ecstatic-morse
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Apr 16, 2020

📌 Commit 66575c9 has been approved by ecstatic-morse

bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 16, 2020
Rollup of 5 pull requests

Successful merges:

 - rust-lang#70611 (Add long error explanation for E0708 rust-lang#61137)
 - rust-lang#71197 (Don't use the HirId to NodeId map in MIR)
 - rust-lang#71211 (Update cargo)
 - rust-lang#71219 (Minor fixes to doc comments of 'VecDeque')
 - rust-lang#71221 (Dogfood or_patterns in rustdoc)

Failed merges:

r? @ghost
@bors bors merged commit aa0db0b into rust-lang:master Apr 17, 2020
@ljedrz ljedrz deleted the unsafe_unused branch April 17, 2020 02:29
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Apr 20, 2020
…d_mir, r=ecstatic-morse

MIR: use HirId instead of NodeId to avoid cycles while inlining

I wanted to see if I could limit the number of uses of `NodeId` when `HirId` is available and I saw that some of the MIR `Inliner` code could use `Span` instead of `NodeId`, not unlike in rust-lang#71197.

~If I'm understanding the reason for not calling `optimized_mir` in incremental builds here correctly, this change could also allow us to do so.~

This change could affect performance, so if this approach makes sense, a perf run is probably a good idea.
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.

4 participants