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 span of while (let) expressions after lowering #71494

Merged
merged 2 commits into from
Apr 25, 2020

Conversation

flip1995
Copy link
Member

Credit goes to @alex-700 who found this while trying to fix a suggestion in Clippy.

While if, try, for and await expressions get the span of the original expression when desugared, while loops got the span of the scrutinee, which lead to weird code, when building the suggestion, that randomly worked: https://github.com/rust-lang/rust-clippy/pull/5511/files#diff-df4e9d2bf840a5f2e3b580bef73da3bcR106-R108

I'm wondering, if DesugaringKind should get a variant WhileLoop and instead of using the span of the ast::ExprKind::While expr directly, a new span with self.mark_span_with_reason should be used, like it is done with for loops.

There was some fallout, but I think that is acceptable. If not, I need some help to find out where this can be fixed.

@rust-highfive
Copy link
Collaborator

r? @petrochenkov

(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 Apr 23, 2020
@petrochenkov
Copy link
Contributor

petrochenkov commented Apr 24, 2020

Seems ok as is.
I'm note sure some of those mark_span_with_reason are even necessary now when they no longer generate anything unstable.
@bors r+

@bors
Copy link
Contributor

bors commented Apr 24, 2020

📌 Commit 898cbf2 has been approved by petrochenkov

@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 Apr 24, 2020
@bors
Copy link
Contributor

bors commented Apr 24, 2020

💡 This pull request was already approved, no need to approve it again.

@bors
Copy link
Contributor

bors commented Apr 24, 2020

📌 Commit 898cbf2 has been approved by petrochenkov

bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 25, 2020
Rollup of 5 pull requests

Successful merges:

 - rust-lang#71364 (Ignore -Zprofile when building compiler_builtins)
 - rust-lang#71494 (Fix span of while (let) expressions after lowering)
 - rust-lang#71517 ( Quick and dirty fix of the unused_braces lint)
 - rust-lang#71523 (Take a single root node in range_search)
 - rust-lang#71533 (Revert PR 70566 for const validation fix)

Failed merges:

r? @ghost
@bors bors merged commit 6ded356 into rust-lang:master Apr 25, 2020
@flip1995 flip1995 deleted the while_let_span branch April 25, 2020 17:02
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants