8

Github Suggest async {} for async || {} by rokob · Pull Request #76580 · rust-la...

 3 years ago
source link: https://github.com/rust-lang/rust/pull/76580
Go to the source link to view the article. You can view the picture content, updated content and better typesetting reading experience. If the link is broken, please click the button below to view the snapshot at that time.
neoserver,ios ssh client

Suggest async {} for async || {} #76580

Merged

Conversation

Contributor

rokob commented on Sep 11, 2020

Fixes #76011

This adds support for adding help diagnostics to the feature gating checks and
then uses it for the async_closure gate to add the extra bit of help
information as described in the issue.

Collaborator

rust-highfive commented on Sep 11, 2020

r? @Mark-Simulacrum

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

Contributor

estebank commented on Sep 12, 2020

edited

Can we add a test? It should live somewhere in src/test/ui/** and after the .rs file is created (and test check comments are added in the right place) you can run ./x.py test src/test/ui --stage 1 --bless to autogenerate the .stderr files.

debug!("gate_feature(feature = {:?}, span = {:?}); has? {}", name, span, has_feature);

if !has_feature && !span.allows_unstable($name) {

feature_err_issue(&visitor.sess.parse_sess, name, span, GateIssue::Language, explain)

.span_help(span, help)

estebank on Sep 12, 2020

Contributor

Because we have a single span, I think this should be a bare help call (without a span) so the output isn't too verbose/redundant. I would not close #76011, as that is asking for a structured suggestion, which we might be able to provide in the future (but this current change is a great mitigation).

Contributor

Author

rokob commented on Sep 23, 2020

I updated the src/test/ui/async-await/feature-async-closure UI test. I also made a change to compiler/rustc_parse/src/parser/expr.rs because otherwise this help would show up in this test src/test/ui/parser/block-no-opening-brace.

I can backout that change if you want and instead have this help show up in that case as well. But looking at that example, it didn't seem right to me that this code:

fn f5() {
    async
        let x = 0;
}

would produce an error about async closures being unstable. You don't know that this is trying to be a closure, it could very well have been an async block which is not unstable. The help message I am adding makes this part even more confusing because it suggests removing the || and replacing it with a { but there is no ||. I think moving the feature gate until after the parsing of the rest of the closure has no semantic impact but makes more sense as you shouldn't assume you have a closure if you don't have |, or ||, or {.

Member

jyn514 commented on Nov 20, 2020

ping @estebank - is there a better reviewer for this?

Member

camelid commented 21 days ago

I think estebank is catching up on reviews now.

Contributor

estebank commented 19 days ago

edited

@bors r+

Noticed the following output

error: expected one of `move`, `|`, or `||`, found keyword `let`
  --> $DIR/block-no-opening-brace.rs:30:9
   |
LL |     async
   |          - expected one of `move`, `|`, or `||`
LL |         let x = 0;
   |         ^^^ unexpected token

should also include { as an expected token, but that's independent of this. Edit: Created #80931.

Contributor

bors commented 19 days ago

pushpin Commit 5b475a4 has been approved by estebank

Contributor

bors commented 19 days ago

hourglass Testing commit 5b475a4 with merge 467f5e9...

Contributor

bors commented 19 days ago

sunny Test successful - checks-actions
Approved by: estebank
Pushing 467f5e9 to master...

bors

merged commit 467f5e9 into rust-lang:master 19 days ago

12 checks passed

rustbot

added this to the 1.51.0 milestone

19 days ago

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

About Joyk


Aggregate valuable and interesting links.
Joyk means Joy of geeK