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

Improve formatting of empty macro_rules! definitions #5883

Merged
merged 1 commit into from
Aug 13, 2023

Conversation

ytmimi
Copy link
Contributor

@ytmimi ytmimi commented Aug 11, 2023

Fixes #5882

@calebcartwright
Copy link
Member

Haven't looked at code and understand the respective issues differ in defs vs calls, but any overlap/correlation between this and ##5879?

@ytmimi
Copy link
Contributor Author

ytmimi commented Aug 12, 2023

From what I can tell the two issues don't seem to be related.

#5882 is an odd case where a macro_rules! definition doesn't have any arms. The code in rewrite_macro_def assumes that there will be arms and always adds an indented newline for the first arm even if it's not there. This PR looks to fix the left behind trailing whitespace issue.

The approach in this PR is to treat the empty macro body as a dummy block and take advantage of comment recovery code defined in the rewrite_block code path.

#5879 on the other hand is highlighting an issue with how rustfmt rewrites vec!{} calls. rustfmt makes the assumption that vec! will always be written with [ ] delimiters, and that leads to a panic! when that assumption isn't ture.

Copy link
Member

@calebcartwright calebcartwright left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm thanks!

(also noticed that this fixes an issue where if the def was solely comments/whitespace then rustfmt would simply erase the comments, might be worth checking to see if there's any open issues to that effect which we could also close as part of this)

@calebcartwright calebcartwright added release-notes Needs an associated changelog entry and removed pr-not-reviewed labels Aug 13, 2023
@calebcartwright calebcartwright merged commit 0d4c143 into rust-lang:master Aug 13, 2023
27 checks passed
@ytmimi ytmimi deleted the issue_5882 branch August 13, 2023 23:23
@ytmimi
Copy link
Contributor Author

ytmimi commented Aug 16, 2023

Doing a search for macro_rules! in the issue tracker didn't turn up any issues with empty macro_rules! macros.

@ytmimi ytmimi removed the release-notes Needs an associated changelog entry label Oct 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

rustfmt adds trailing spaces to macro rules
3 participants