Skip to content

Change format_snippet to return None when it has failed to format macro call #2775

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

Merged
merged 5 commits into from
Jun 7, 2018

Conversation

topecongiro
Copy link
Contributor

rustfmt manually adds/removes indentation when trying to format macro def's body. This does not work when the macro body contains a macro call that rustfmt cannot format, and causes an issue like #2607 or #2770. This PR fixes it by changing the format_snippet to return None when it has failed to format the macro call in the snippet.

Closes #2607.
Closes #2770.

@nrc
Copy link
Member

nrc commented Jun 7, 2018

There's a few test failures, but r+ with those fixed

@topecongiro
Copy link
Contributor Author

Thank you for you review! I have fixed the test failures.

N.B. rustfmt is now unable to format code block with invalid syntax macro calls, like the one in tests/target/issue-2523.rs.

@nrc nrc merged commit 34067a1 into rust-lang:master Jun 7, 2018
@topecongiro topecongiro deleted the macro-def-with-complex-macro branch June 7, 2018 21:11
where
F: FnMut(&FileName, &mut String, &[(usize, usize)], &FormatReport) -> Result<bool, io::Error>,
{
let mut result = FileMap::new();
// diff mode: check if any files are differing
let mut has_diff = false;
let mut has_macro_rewrite_failure = false;
Copy link
Contributor

@t-botz t-botz Jun 9, 2018

Choose a reason for hiding this comment

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

@topecongiro @nrc
Shouldn't has_macro_rewrite_failure be part of the FormatReport instead?
IMO That should be a new ErrorKind

Copy link
Member

Choose a reason for hiding this comment

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

I figure the two will get merged in some way, so it doesn't really matter what we do right now

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.

Weird formatting of asm! statement within macro definition Rustfmt Unconditionally Indents Code Each Time It's Run
3 participants