Closed
Conversation
Some places use the local `tcx` variable, some use `self.tcx()`. This commit removes the local variable so that all places use `self.tcx()`, for consistency.
This wins 6% on `unicode_normalization`, by avoiding a call to `lub_concrete_regions()` and a `Region` equality test.
The function only has one call site, so we don't need a tag argument.
And make it more uniform with other macros. By merging placeholders for future derives' outputs into the derive container's output fragment early.
It means an allocation is required to create an empty `TokenStream`, but all other operations are simpler and marginally faster due to not having to check for `None`. Overall it simplifies the code for a negligible performance effect. The commit also removes `TokenStream::empty` by implementing `Default`, which is now possible.
This avoids some unnecessary creation of empty token streams.
…=eddyb Compute the layout of uninhabited structs fixes rust-lang#64506 r? @eddyb
…hewjasper expand: Simplify expansion of derives And make it more uniform with other macros. This is done by merging placeholders for future derives' outputs into the derive container's output fragment early (addressing FIXMEs from rust-lang#63667). Also, macros with names starting with `_` are no longer reported as unused, in accordance with the usual behavior of `unused` lints. r? @matthewjasper or @mark-i-m
…-expansion, r=nikomatsakis Optimize `LexicalResolve::expansion`. A win for `unicode_normalization`. r? @nikomatsakis
…eam, r=petrochenkov Remove `Option` from `TokenStream` A code simplification. r? @petrochenkov
Contributor
Author
|
@bors r+ p=4 rollup=never |
Collaborator
|
📌 Commit ed9d45b has been approved by |
Collaborator
|
⌛ Testing commit ed9d45b with merge 23ad56b1c77991fd2274c4fa626c22c9452ba59c... |
Collaborator
|
💔 Test failed - checks-azure |
Contributor
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
Contributor
Author
|
I suspect the cause of failure here was #65252. |
This file contains hidden or 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
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.
Successful merges:
LexicalResolve::expansion. #65260 (OptimizeLexicalResolve::expansion.)OptionfromTokenStream#65261 (RemoveOptionfromTokenStream)Failed merges:
r? @ghost