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

Latest update of stable fails to format |foo = bar| (in a macro) #3583

Closed
thomcc opened this issue May 24, 2019 · 6 comments · Fixed by #3589
Closed

Latest update of stable fails to format |foo = bar| (in a macro) #3583

thomcc opened this issue May 24, 2019 · 6 comments · Fixed by #3589
Labels
a-macros bug Panic, non-idempotency, invalid code, etc.

Comments

@thomcc
Copy link
Member

thomcc commented May 24, 2019

This file fails to format with the latest stable rustfmt (1.2.0, --version output at end). The macro below seems to be the cause https://github.com/mozilla/application-services/blob/eb72c9aa1e5ecfcf79c1444eb98910727c699f61/components/places/benches/search.rs#L66-L74 (We managed to fix that issue by changing the = to a :)

It also fails to report any errors beyond exiting with code 101, which appears to be issue #3008.

$ rustfmt --version
rustfmt 1.2.0-stable (09940a70 2019-03-27)
@scampi scampi added the bug Panic, non-idempotency, invalid code, etc. label May 25, 2019
@rchaser53
Copy link
Contributor

rchaser53 commented May 25, 2019

This is the rustfmt failed CI link in your repository.
It seems that the other trouble(maybe kinda network problem) so it's not a rustfmt bug.

I tried to reproduce your CI error in my fork repository. But rustfmt run correctly.
This is a CI link in my repository.
It's also failed, but it's a correct behavior.
cargo fmt -- --check founds the diff like below.

cargo_fmt_check_diff

@thomcc
Copy link
Member Author

thomcc commented May 25, 2019

That's not the right link to a CI failure from this, here's one: https://circleci.com/gh/mozilla/application-services/15051

You can also repro by checking out commit eb72c9aa1e5ecfcf79c1444eb98910727c699f61 and using rustfmt.

@rchaser53
Copy link
Contributor

rchaser53 commented May 25, 2019

I succeed to reproduce the problem, thank you.

@topecongiro
Copy link
Contributor

A minimal working example:

foo!(|0 =);

rustfmt somehow aborts while parsing the argument of foo!. Since rustfmt catches unwinding panics from the parser, I suspect that in some cases the parser aborts the whole process. Not sure how this can be fixed from the rustfmt side.

@thomcc
Copy link
Member Author

thomcc commented May 27, 2019

I'm not sure the rust parser aborting makes a lot of sense, since the code in the case I'm reporting compiles (and runs) fine.

@topecongiro
Copy link
Contributor

@thomcc Yeah, you are right. I took another look at the code and found that panics raised while parsing macro arguments are not caught.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a-macros bug Panic, non-idempotency, invalid code, etc.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants