rustc_data_structures cleanups
#124831
Merged
rustc_data_structures cleanups
#124831
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Normal `use` items are nicer.
- `use` before `mod` - `pub` before `non-pub` - Alphabetical order within sections
rustbot
added
S-waiting-on-review
|
Some changes occurred to the CTFE / Miri engine cc @rust-lang/miri |
This comment has been minimized.
This comment has been minimized.
RalfJung
reviewed
May 7, 2024
RalfJung
reviewed
May 7, 2024
There was a problem hiding this comment.
Thanks for the PR, @nnethercote! Looks good to me overall.
Let's do a perf run to see if there is a difference due to the case @RalfJung points out.
SparrowLii
reviewed
May 7, 2024
`use` is a nicer way of doing things.
nnethercote
force-pushed
the
rustc_data_structures-cleanups
branch
from
76ceba3
to
475c4fe
Compare
|
@michaelwoerister I have addressed the comments. I don't think a perf run is necessary now that I changed the |
This comment has been minimized.
This comment has been minimized.
nnethercote
force-pushed
the
rustc_data_structures-cleanups
branch
from
475c4fe
to
8b1fec2
Compare
RalfJung
reviewed
May 8, 2024
nnethercote
force-pushed
the
rustc_data_structures-cleanups
branch
from
8b1fec2
to
82476c4
Compare
And move the `repr` line after the `derive` line, where it's harder to overlook. (I overlooked it initially, and didn't understand how this type worked.)
It is optimized for lists with a single element, avoiding the need for an allocation in that case. But `SmallVec<[T; 1]>` also avoids the allocation, and is better in general: more standard, log2 number of allocations if the list exceeds one item, and a much more capable API. This commit removes `TinyList` and converts the two uses to `SmallVec<[T; 1]>`. It also reorders the `use` items in the relevant file so they are in just two sections (`pub` and non-`pub`), ordered alphabetically, instead of many sections. (This is a relevant part of the change because I had to decide where to add a `use` item for `SmallVec`.)
It provides a way to effectively embed a linked list within an `IndexVec` and also iterate over that list. It's written in a very generic way, involving two traits `Links` and `LinkElem`. But the `Links` trait is only impl'd for `IndexVec` and `&IndexVec`, and the whole thing is only used in one module within `rustc_borrowck`. So I think it's over-engineered and hard to read. Plus it has no comments. This commit removes it, and adds a (non-generic) local iterator for the use within `rustc_borrowck`. Much simpler.
It's a macro that just creates an enum with a `from_u32` method. It has two arms. One is unused and the other has a single use. This commit inlines that single use and removes the whole macro. This increases readability because we don't have two different macros interacting (`enum_from_u32` and `language_item_table`).
nnethercote
force-pushed
the
rustc_data_structures-cleanups
branch
from
82476c4
to
58a06b6
Compare
|
@bors r=michaelwoerister |
bors
added
S-waiting-on-bors
|
☀️ Test successful - checks-actions |
|
Finished benchmarking commit (37dc766): comparison URL. Overall result: ❌ regressions - no action needed@rustbot label: -perf-regression Instruction countThis is a highly reliable metric that was used to determine the overall result at the top of this comment.
Max RSS (memory usage)ResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesThis benchmark run did not return any relevant results for this metric. Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 675.709s -> 676.526s (0.12%) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Some improvements I found while looking through this code.
r? @michaelwoerister