-
Notifications
You must be signed in to change notification settings - Fork 13k
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
Removed most instances of vec!(..) and replaced them with vec![..]s #37476
Conversation
On 64-bit this reduces the size of `Expr_` from 144 to 64 bytes, and reduces the size of `Expr` from 176 to 96 bytes.
Most of the Rust community agrees that the vec! macro is more clear when called using square brackets [] instead of regular brackets (). Most of these ocurrences are from before macros allowed using different types of brackets.
These are all legacies from before the clearer vec![] was possible to write. They are now all gone. This would be a perfect time to write a lint for ocurrences of vec!().
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Aatch (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
|
Oh dear, looks like I hit some sort of snag in pretty printing. Looks like Should I revert this single file's changes, or is there something else that needs fixing? Edit: If it is something else that needs fixing, it'll probably need doing in another PR, so I'll just revert this for now. |
Sadly it looks like pretty printing still wants vec![]s to be vec!()s :( This is something that should be fixed at another time.
I'd guess that'll be rather hard to fix--I don't believe pretty printing is "aware" of I'm not familiar with that part of the compiler though, so may be wrong. |
Shrink Expr_::ExprInlineAsm. On 64-bit this reduces the size of `Expr_` from 144 to 64 bytes, and reduces the size of `Expr` from 176 to 96 bytes. For the workload in rust-lang#36799 this reduces the RSS for the "lowering ast -> hir" phase and all subsequent phases by 50 MiB, which reduces the peak RSS for that workload by about 1%. Not huge, but it's a very easy improvement. r? @eddyb
☔ The latest upstream changes (presumably #37431) made this pull request unmergeable. Please resolve the merge conflicts. |
Never resolved a merge conflict before. Sure hope I did that right! |
Typically the way merge conflicts are resolved is by rebasing your pull request on top of master, that way you don't have extra merge commits cluttering up the history. |
Fix ICE when attempting to print closure generics Fixes rust-lang#36622. r? @eddyb or @arielb1
save-analysis: change imports to carry a ref id rather than their own… … node id To make jump to def for imports work r? @eddyb
Do not clone Mir unnecessarily r? @eddyb
…xcrichton Replace all uses of SHA-256 with BLAKE2b. Removes the SHA-256 implementation and replaces all uses of it with BLAKE2b, which we already use for debuginfo type guids and incremental compilation hashes. It doesn't make much sense to have two different cryptographic hash implementations in the compiler and Blake has a few advantages over SHA-2 (computationally less expensive, hashes of up to 512 bits).
☔ The latest upstream changes (presumably #37439) made this pull request unmergeable. Please resolve the merge conflicts. |
Most of the Rust community agrees that the vec! macro is more clear when called using square brackets [] instead of regular brackets (). Most of these ocurrences are from before macros allowed using different types of brackets.
I fucked up my git history. :( |
Okay I completely fucked up everything. I'm just going to close this. |
@iirelu happy to help out if you need some; I think this is a good change, and would hate to see |
Make all vec! macros use square brackets: Attempt 2 [The last PR](#37476) ended with tears after a valiant struggle with git. I managed to clean up the completely broken history of that into a brand spanking new PR! Yay! Original: > Everyone hates the old syntax. I hope. Otherwise this PR has some controversy I wasn't expecting. > This would be the perfect time to write a lint recommending vec![..] when you use another style. > Disclaimer: I may have broken something. If I have, I'll fix them when the tests come in. Luckily the chance for a non-syntactical error is pretty low in all this.
Make all vec! macros use square brackets: Attempt 2 [The last PR](rust-lang/rust#37476) ended with tears after a valiant struggle with git. I managed to clean up the completely broken history of that into a brand spanking new PR! Yay! Original: > Everyone hates the old syntax. I hope. Otherwise this PR has some controversy I wasn't expecting. > This would be the perfect time to write a lint recommending vec![..] when you use another style. > Disclaimer: I may have broken something. If I have, I'll fix them when the tests come in. Luckily the chance for a non-syntactical error is pretty low in all this.
Everyone hates the old syntax. I hope. Otherwise this PR has some controversy I wasn't expecting.
This would be the perfect time to write a lint recommending
vec![..]
when you use another style.Disclaimer: I may have broken something. If I have, I'll fix them when the tests come in. Luckily the chance for a non-syntactical error is pretty low in all this.