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

rustc: use IndexVec<DefIndex, T> instead of Vec<T>. #65825

Merged
merged 1 commit into from Oct 29, 2019

Conversation

@eddyb
Copy link
Member

eddyb commented Oct 25, 2019

Now that DefIndex is a proper index type, we can do that.
There was also an unnecessary Option I removed, I wonder if that has perf implications.

@rust-highfive

This comment has been minimized.

Copy link
Collaborator

rust-highfive commented Oct 25, 2019

r? @zackmdavis

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

@eddyb

This comment has been minimized.

Copy link
Member Author

eddyb commented Oct 25, 2019

@bors try @rust-timer queue

@rust-timer

This comment has been minimized.

Copy link

rust-timer commented Oct 25, 2019

Awaiting bors try build completion

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Oct 25, 2019

⌛️ Trying commit 70eb4bf with merge 7653088...

bors added a commit that referenced this pull request Oct 25, 2019
rustc: use IndexVec<DefIndex, T> instead of Vec<T>.

Now that `DefIndex` is a proper index type, we can do that.
There was also an unnecessary `Option` I removed, I wonder if that has perf implications.
@bors

This comment has been minimized.

Copy link
Contributor

bors commented Oct 25, 2019

☀️ Try build successful - checks-azure
Build commit: 7653088 (76530884546a7aa7bf406b7af7e07c6225197ec0)

@rust-timer

This comment has been minimized.

Copy link

rust-timer commented Oct 25, 2019

Queued 7653088 with parent 23f890f, future comparison URL.

bors added a commit that referenced this pull request Oct 26, 2019
rustc: rewrite the HirId validator to only check HIR map compactness.

*Based on #65825

This should be cheaper than the current validator, and not assume item-like owners, but I did have to add a few more kinds of nodes to the HIR map, so that every node with a `HirId` is in it.

AFAICT, collecting the HIR map in the first place already checks owners, and I didn't want to duplicate any complications to that for #63849 (comment) (making all nodes with a `DefId` into `HirId` owners).

r? @michaelwoerister cc @nikomatsakis
@rust-timer

This comment has been minimized.

Copy link

rust-timer commented Oct 26, 2019

Finished benchmarking try commit 7653088, comparison URL.

bors added a commit that referenced this pull request Oct 26, 2019
rustc: rewrite the HirId validator to only check HIR map compactness.

*Based on #65825

This should be cheaper than the current validator, and not assume item-like owners, but I did have to add a few more kinds of nodes to the HIR map, so that every node with a `HirId` is in it.

AFAICT, collecting the HIR map in the first place already checks owners, and I didn't want to duplicate any complications to that for #63849 (comment) (making all nodes with a `DefId` into `HirId` owners).

I've also cleaned up the errors to be more useful in determining the "neighborhood" of nodes around missing `HirId`s, at a glance (we can probably do even better but there's little point at this time).

r? @michaelwoerister cc @nikomatsakis
@varkor

This comment has been minimized.

Copy link
Member

varkor commented Oct 26, 2019

@bors r+ rollup

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Oct 26, 2019

📌 Commit 70eb4bf has been approved by varkor

bors added a commit that referenced this pull request Oct 26, 2019
Rollup of 8 pull requests

Successful merges:

 - #65743 (rustc_typeck: don't record direct callees in generator_interior.)
 - #65761 (libsyntax: Enhance documentation of the AST module)
 - #65772 (Remove the last remaining READMEs)
 - #65773 (Increase spacing for suggestions in diagnostics)
 - #65791 (Adding doc on keyword continue)
 - #65824 (rustc: make DefPathData (and friends) Copy (now that it uses Symbol).)
 - #65828 (Derive Eq and Hash for SourceInfo again)
 - #65842 (Add more information on rustdoc search)

Failed merges:

 - #65825 (rustc: use IndexVec<DefIndex, T> instead of Vec<T>.)

r? @ghost
@bors

This comment was marked as resolved.

Copy link
Contributor

bors commented Oct 26, 2019

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

@eddyb eddyb force-pushed the eddyb:def-index-vec branch from 70eb4bf to cc575a6 Oct 28, 2019
@eddyb

This comment has been minimized.

Copy link
Member Author

eddyb commented Oct 28, 2019

@bors r=varkor

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Oct 28, 2019

📌 Commit cc575a6 has been approved by varkor

JohnTitor added a commit to JohnTitor/rust that referenced this pull request Oct 28, 2019
rustc: use IndexVec<DefIndex, T> instead of Vec<T>.

Now that `DefIndex` is a proper index type, we can do that.
There was also an unnecessary `Option` I removed, I wonder if that has perf implications.
Centril added a commit to Centril/rust that referenced this pull request Oct 28, 2019
rustc: use IndexVec<DefIndex, T> instead of Vec<T>.

Now that `DefIndex` is a proper index type, we can do that.
There was also an unnecessary `Option` I removed, I wonder if that has perf implications.
bors added a commit that referenced this pull request Oct 28, 2019
Rollup of 9 pull requests

Successful merges:

 - #65563 (Add long error explanation for E0587)
 - #65640 (Use heuristics to recover parsing of missing `;`)
 - #65643 (Correct handling of type flags with `ConstValue::Placeholder`)
 - #65825 (rustc: use IndexVec<DefIndex, T> instead of Vec<T>.)
 - #65858 (suggest `const_in_array_repeat_expression` flag)
 - #65877 (doc: introduce `once` in `iter::chain` document)
 - #65887 (doc: mention `get(_mut)` in Vec)
 - #65891 (self-profiling: Record something more useful for crate metadata generation event.)
 - #65893 (Output previous stable  error messaging when using stable build.)

Failed merges:

r? @ghost
@bors bors merged commit cc575a6 into rust-lang:master Oct 29, 2019
4 checks passed
4 checks passed
pr Build #20191028.25 succeeded
Details
pr (Linux mingw-check) Linux mingw-check succeeded
Details
pr (Linux x86_64-gnu-llvm-6.0) Linux x86_64-gnu-llvm-6.0 succeeded
Details
pr (LinuxTools) LinuxTools succeeded
Details
@eddyb eddyb deleted the eddyb:def-index-vec branch Oct 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.