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

Preserve semicolon after macro call inside foreign mod #5282

Merged
merged 2 commits into from
Mar 30, 2022

Conversation

dtolnay
Copy link
Member

@dtolnay dtolnay commented Mar 29, 2022

Fixes #5281.

Before:

$ echo 'extern { x!(-); x!(x); }' | cargo run --bin rustfmt
extern "C" {
    x!(-)
    x!(x);
}

After:

$ echo 'extern { x!(-); x!(x); }' | cargo run --bin rustfmt
extern "C" {
    x!(-);
    x!(x);
}

@ytmimi
Copy link
Contributor

ytmimi commented Mar 30, 2022

Thanks for the PR! Changes look good to me. Just so I'm understanding this correctly, since we fail to parse the macro args (because we can't parse - or #) we end up calling return_macro_parse_failure_fallback, which was dropping the trailing ;. The fix seems pretty reasonable to me.

@calebcartwright
Copy link
Member

Just so I'm understanding this correctly, since we fail to parse the macro args (because we can't parse - or #) we end up calling return_macro_parse_failure_fallback, which was dropping the trailing ;

Essentially yes, though I'd probably make one distinction in that the trailing semi isn't part of the maccall.

We still can't/don't do much with calls whose args can't be directly parsed as some valid Rust construct and the fallback behavior is to just leave the call itself however the author wrote it and that's working "correctly". However, there was a change upstream in rustc not too terribly long ago that removed the trailing semi from the various spans including that of the maccall.

We added a couple workarounds that account for that and recover the trailing semi, along with handling more absurd scenarios (like comments between the closing delim and semi), but those workarounds apparently aren't utilized on the call path for a maccall in a foreign item context.

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.

Thank you for the report and fix.

TBH I'm not too terribly keen on having yet another one of these semicolon workarounds, however, content to move forward with this anyway given the impact of the issue.

Mostly comments for our future selves, but at some point down the road I think we should probably reuse behavior leveraged in other contexts to look between the end of the mac call's span and the semi at the end of the containing foreign item's span

@calebcartwright calebcartwright merged commit 5ff7b63 into rust-lang:master Mar 30, 2022
@dtolnay dtolnay deleted the externmac branch March 30, 2022 03:51
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 creates invalid syntax on extern block containing macro call
3 participants