-
Notifications
You must be signed in to change notification settings - Fork 13.2k
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
Use ?
Kleene operator again
#56357
Use ?
Kleene operator again
#56357
Conversation
r? @davidtwco (rust_highfive has picked a reviewer for you, use r? to override) |
I am getting some test failures locally... are we not using rust 2018 within the compiler? |
This comment has been minimized.
This comment has been minimized.
I'm not sure about this, not something I've ever run into. Could you fix the tidy issue so that we can see if the error you're seeing shows up on Travis? |
The compiler is certainly not using 2018 edition. |
I still think |
That ship has sailed already...
Interesting... Is that just a matter of running rustfmt and updating x.py? Are there plans to ever do that?
Sure, when I get to a computer |
@davidtwco I pushed a tidy fix, but if the compiler is not using rust 2018, then this PR cannot be merged... |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
@mark-i-m I'm not too sure. I don't know if this has come up before. One way I could think of doing it, though it is admittedly quick hacky, would be to modify the below code so that the 2018 branch is used if rust/src/libsyntax/ext/tt/quoted.rs Lines 437 to 448 in d48ab69
I feel like that's really awful though. Hopefully someone else knows of a better way than modifying the check to identify the compiler in some way. |
@davidtwco I could do that, but what I would really like to do is just use rust 2018. Do you know if there is any blocker to just passing --edition 2018 through x.py and running rustfmt? |
@mark-i-m I have no idea. There may be good reasons why that hasn't happened, but maybe not. 🤷♂️ |
@davidtwco I played around with this a bit, and it looks like the may challenge is just that rustfix needs to be run on the compiler, which is not something |
@mark-i-m Perhaps that it might be best if this lands after Rust 2018 is out in that case. The compiler could switch to 2018, rustfix and rustfmt could be run. |
@davidtwco This is the diff just to make |
In #51587 I removed a bunch of uses of the
?
Kleene operator. Now?
is stable, so I am adding them back 🎉Unfortunately, the macros had all changed slightly since that PR. In particular, a few of them have extra arms, which I also tried to apply
?
to. I think I got it right, but I would appreciate an extra careful review...