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.

Sign up for GitHub

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

Jump to bottom

Gracefully handle mistyping -> as => in function return type #77035

Merged
merged 3 commits into from
Dec 19, 2020
Merged

Gracefully handle mistyping -> as => in function return type #77035

merged 3 commits into from
Dec 19, 2020

Conversation

mibac138
Copy link
Contributor

Fixes #77019

@rust-highfive
Copy link
Collaborator

r? @davidtwco

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 21, 2020
@jyn514 jyn514 added A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Sep 22, 2020
@tesuji
Copy link
Contributor

tesuji commented Sep 22, 2020 •
edited
Loading

Might want a crater check if we want make sure it introduces no regressions.

@jyn514
Copy link
Member

jyn514 commented Sep 22, 2020

@lzutao what does crater have to do with this? All this does is improve a diagnostic for code that was already broken.

@tesuji

This comment has been minimized.

Sign in to view
@jyn514
Copy link
Member

jyn514 commented Sep 22, 2020

If every pull request went through crater, we'd never get anything merged, it would take weeks for the smallest change.

@tesuji

This comment has been minimized.

Sign in to view
davidtwco
davidtwco requested changes Sep 22, 2020
View reviewed changes
Copy link
Member

@davidtwco davidtwco left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Some changes are required but this is definitely an improvement. Please resolve all of the review comments that have been left so far.

You'll also need to run ./x.py fmt to run rustfmt and pass CI.

compiler/rustc_parse/src/parser/ty.rs Show resolved Hide resolved
@bugaevc

This comment has been minimized.

Sign in to view
@LeSeulArtichaut LeSeulArtichaut mentioned this pull request Sep 23, 2020
Closed
@LeSeulArtichaut

This comment has been minimized.

Sign in to view
@JohnCSimon JohnCSimon added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 12, 2020
@JohnCSimon

This comment has been minimized.

Sign in to view
@mibac138 mibac138 force-pushed the fn-fat-arrow-return branch from 66d758e to c297a2e Compare October 15, 2020 21:21
@bors

This comment has been minimized.

Sign in to view
@crlf0710

This comment has been minimized.

Sign in to view
@JohnCSimon

This comment has been minimized.

Sign in to view
@mibac138 mibac138 force-pushed the fn-fat-arrow-return branch from c297a2e to cf94bc6 Compare December 3, 2020 13:48
@mibac138 mibac138 force-pushed the fn-fat-arrow-return branch from cf94bc6 to 3548be9 Compare December 3, 2020 17:19
@mibac138
Copy link
Contributor Author

mibac138 commented Dec 3, 2020

Fixed the build and rebased.
@rustbot modify labels: +S-waiting-on-review -S-waiting-on-author

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 3, 2020
@davidtwco
Copy link
Member

@craterbot check

@craterbot
Copy link
Collaborator

👌 Experiment pr-77035 created and queued.
🤖 Automatically detected try build 55b3f428b4fac6790b1e8492e0b91ff2d80aec7c
⚠️ Try build based on commit 39b841d, but latest commit is 3548be9. Did you forget to make a new try build?
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot craterbot added S-waiting-on-crater Status: Waiting on a crater run to be completed. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 10, 2020
@craterbot
Copy link
Collaborator

🚧 Experiment pr-77035 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot
Copy link
Collaborator

🎉 Experiment pr-77035 is completed!
📊 4 regressed and 11 fixed (134885 total)
📰 Open the full report.

⚠️ If you notice any spurious failure please add them to the blacklist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot craterbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-crater Status: Waiting on a crater run to be completed. labels Dec 17, 2020
@davidtwco
Copy link
Member

@mibac138 Great, no regressions, once the review comments are resolved then we can land this!

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@mibac138 mibac138 force-pushed the fn-fat-arrow-return branch from f475fe5 to e916641 Compare December 17, 2020 13:13
@mibac138
Copy link
Contributor Author

@davidtwco Glad to hear! I have now addressed your review comments, so I think this should be good to go now?

@davidtwco
Copy link
Member

Thanks!

@bors r+

@bors
Copy link
Contributor

bors commented Dec 18, 2020

📌 Commit e916641 has been approved by davidtwco

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 18, 2020
@bors
Copy link
Contributor

bors commented Dec 19, 2020

⌛ Testing commit e916641 with merge d1741e5...

@bors
Copy link
Contributor

bors commented Dec 19, 2020

☀️ Test successful - checks-actions
Approved by: davidtwco
Pushing d1741e5 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Dec 19, 2020
@bors bors merged commit d1741e5 into rust-lang:master Dec 19, 2020
@rustbot rustbot added this to the 1.50.0 milestone Dec 19, 2020
This was referenced Dec 19, 2020
Merged
Merged
Closed
pickfire
pickfire reviewed Dec 30, 2020
View reviewed changes
src/test/ui/fn/fn-recover-return-sign.fixed
fn a() -> usize { 0 }
//~^ ERROR return types are denoted using `->`

fn b()-> usize { 0 }
Copy link
Contributor

Choose a reason for hiding this comment

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

How come rustfix does not add a space here?

Copy link
Member

Choose a reason for hiding this comment

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

Rustfmt doesn't run on the test suite (and shouldn't, because sometimes the tests are for the parser, and the parser should be whitespace-independent).

Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't this the output of rustfix? Shouldn't rustfix code output be sightly formatted (at least not as good as rustfmt)?

Copy link
Member

Choose a reason for hiding this comment

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

Oh! I missed that this is a .fixed file. This happens because the original doesn't have a space before the ::

fn b(): usize { 0 }
//~^ ERROR return types are denoted using `->`

Ideally the suggestion would add its own space, yeah.

@mibac138 mibac138 deleted the fn-fat-arrow-return branch January 9, 2021 01:19
@matthiaskrgr matthiaskrgr mentioned this pull request Jun 1, 2024
Closed
@fmease fmease mentioned this pull request Jun 1, 2024
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 1, 2024
@bors
Auto merge of rust-lang#125863 - fmease:rej-CVarArgs-in-parse_ty_for_…
f67a1ac 
…where_clause, r=compiler-errors

Reject `CVarArgs` in `parse_ty_for_where_clause`

Fixes rust-lang#125847. This regressed in rust-lang#77035 where the `parse_ty` inside `parse_ty_where_predicate` was replaced with the at the time new `parse_ty_for_where_clause` which incorrectly stated it would permit CVarArgs (maybe a copy/paste error).

r? parser
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jyn514 jyn514 jyn514 left review comments

@tesuji tesuji tesuji left review comments

@pickfire pickfire pickfire left review comments

@davidtwco davidtwco davidtwco requested changes

Assignees

@davidtwco davidtwco

Labels
A-diagnostics Area: Messages for errors, warnings, and lints merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.50.0
Development

Successfully merging this pull request may close these issues.

Give a better error message for => in a return type