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

fix: break inside async closure has incorrect span for enclosing closure #125078

Merged
merged 1 commit into from
Jun 19, 2024

Conversation

linyihai
Copy link
Contributor

Fixes #124496

@rustbot
Copy link
Collaborator

rustbot commented May 13, 2024

r? @davidtwco

rustbot has assigned @davidtwco.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels May 13, 2024
@@ -91,6 +91,10 @@ impl<'a, 'hir> Visitor<'hir> for CheckLoopVisitor<'a, 'hir> {
}) => {
let cx = match kind {
hir::ClosureKind::Coroutine(hir::CoroutineKind::Desugared(kind, source)) => {
let fn_decl_span = match self.cx {
Copy link
Member

Choose a reason for hiding this comment

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

This seems like a bit too specific of a case here. I think we should just fix the fn_decl_span for the hir::Closure from an async closure.

This comment was marked as off-topic.

Copy link
Member

@compiler-errors compiler-errors left a comment

Choose a reason for hiding this comment

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

Pls look into fixing this in a more general way

@linyihai

This comment was marked as resolved.

@linyihai
Copy link
Contributor Author

@rustbot ready

Copy link
Member

@compiler-errors compiler-errors left a comment

Choose a reason for hiding this comment

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

Sorry, I wasn't clear when I meant that this should be made more general. I think this needs to be fixed when the hir::Closure is constructed, somewhere in rustc_ast_lowering.

See https://doc.rust-lang.org/nightly/nightly-rustc/src/rustc_ast_lowering/expr.rs.html#1056-1122 or somewhere nearby.

@rustbot rustbot 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 May 20, 2024
@rust-log-analyzer

This comment has been minimized.

@linyihai
Copy link
Contributor Author

@rustbot ready

@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 May 27, 2024
@rust-log-analyzer

This comment has been minimized.

@linyihai
Copy link
Contributor Author

linyihai commented Jun 5, 2024

I think it's ready to review. Would you @compiler-errors leave some feadback? I will be very appreciate for it

@compiler-errors
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Jun 18, 2024

📌 Commit 05b7b46 has been approved by compiler-errors

It is now in the queue for this repository.

@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 Jun 18, 2024
jieyouxu added a commit to jieyouxu/rust that referenced this pull request Jun 19, 2024

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
…errors

fix: break inside async closure has incorrect span for enclosing closure

Fixes rust-lang#124496
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 19, 2024
Rollup of 10 pull requests

Successful merges:

 - rust-lang#124135 (delegation: Implement glob delegation)
 - rust-lang#125078 (fix: break inside async closure has incorrect span for enclosing closure)
 - rust-lang#125293 (Place tail expression behind terminating scope)
 - rust-lang#126422 (Suggest using a standalone doctest for non-local impl defs)
 - rust-lang#126493 (safe transmute: support non-ZST, variantful, uninhabited enums)
 - rust-lang#126504 (Sync fuchsia test runner with clang test runner)
 - rust-lang#126558 (hir_typeck: be more conservative in making "note caller chooses ty param" note)
 - rust-lang#126586 (Add `@badboy` and `@BlackHoleFox` as Mac Catalyst maintainers)
 - rust-lang#126615 (Add `rustc-ice*` to `.gitignore`)
 - rust-lang#126632 (Replace `move||` with `move ||`)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit c9a9d5c into rust-lang:master Jun 19, 2024
6 checks passed
@rustbot rustbot added this to the 1.81.0 milestone Jun 19, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jun 19, 2024

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Rollup merge of rust-lang#125078 - linyihai:issue-124496, r=compiler-errors

fix: break inside async closure has incorrect span for enclosing closure

Fixes rust-lang#124496
jhpratt added a commit to jhpratt/rust that referenced this pull request Jun 28, 2024

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
…s, r=oli-obk

Tighten `fn_decl_span` for async blocks

Tightens the span of `async {}` blocks in diagnostics, and subsequently async closures and async fns, by actually setting the `fn_decl_span` correctly. This is kinda a follow-up on rust-lang#125078, but it fixes the problem in a more general way.

I think the diagnostics are significantly improved, since we no longer have a bunch of overlapping spans. I'll point out one caveat where I think the diagnostic may get a bit more confusing, but where I don't think it matters.

r? `@estebank` or `@oli-obk` or someone else on wg-diag or compiler i dont really care lol
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jun 28, 2024

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
…s, r=oli-obk

Tighten `fn_decl_span` for async blocks

Tightens the span of `async {}` blocks in diagnostics, and subsequently async closures and async fns, by actually setting the `fn_decl_span` correctly. This is kinda a follow-up on rust-lang#125078, but it fixes the problem in a more general way.

I think the diagnostics are significantly improved, since we no longer have a bunch of overlapping spans. I'll point out one caveat where I think the diagnostic may get a bit more confusing, but where I don't think it matters.

r? ``@estebank`` or ``@oli-obk`` or someone else on wg-diag or compiler i dont really care lol
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jun 28, 2024

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
…s, r=oli-obk

Tighten `fn_decl_span` for async blocks

Tightens the span of `async {}` blocks in diagnostics, and subsequently async closures and async fns, by actually setting the `fn_decl_span` correctly. This is kinda a follow-up on rust-lang#125078, but it fixes the problem in a more general way.

I think the diagnostics are significantly improved, since we no longer have a bunch of overlapping spans. I'll point out one caveat where I think the diagnostic may get a bit more confusing, but where I don't think it matters.

r? ```@estebank``` or ```@oli-obk``` or someone else on wg-diag or compiler i dont really care lol
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jun 28, 2024

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
…s, r=oli-obk

Tighten `fn_decl_span` for async blocks

Tightens the span of `async {}` blocks in diagnostics, and subsequently async closures and async fns, by actually setting the `fn_decl_span` correctly. This is kinda a follow-up on rust-lang#125078, but it fixes the problem in a more general way.

I think the diagnostics are significantly improved, since we no longer have a bunch of overlapping spans. I'll point out one caveat where I think the diagnostic may get a bit more confusing, but where I don't think it matters.

r? ````@estebank```` or ````@oli-obk```` or someone else on wg-diag or compiler i dont really care lol
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jun 28, 2024

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Rollup merge of rust-lang#127058 - compiler-errors:tighten-async-spans, r=oli-obk

Tighten `fn_decl_span` for async blocks

Tightens the span of `async {}` blocks in diagnostics, and subsequently async closures and async fns, by actually setting the `fn_decl_span` correctly. This is kinda a follow-up on rust-lang#125078, but it fixes the problem in a more general way.

I think the diagnostics are significantly improved, since we no longer have a bunch of overlapping spans. I'll point out one caveat where I think the diagnostic may get a bit more confusing, but where I don't think it matters.

r? ````@estebank```` or ````@oli-obk```` or someone else on wg-diag or compiler i dont really care lol
flip1995 pushed a commit to flip1995/rust that referenced this pull request Jul 11, 2024
…s, r=oli-obk

Tighten `fn_decl_span` for async blocks

Tightens the span of `async {}` blocks in diagnostics, and subsequently async closures and async fns, by actually setting the `fn_decl_span` correctly. This is kinda a follow-up on rust-lang#125078, but it fixes the problem in a more general way.

I think the diagnostics are significantly improved, since we no longer have a bunch of overlapping spans. I'll point out one caveat where I think the diagnostic may get a bit more confusing, but where I don't think it matters.

r? ````@estebank```` or ````@oli-obk```` or someone else on wg-diag or compiler i dont really care lol
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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
Development

Successfully merging this pull request may close these issues.

break inside async closure has incorrect span for enclosing closure
6 participants