Skip to content

Suggest let for assignment, and some code refactor - #101908

Merged
bors merged 1 commit into
rust-lang:masterfrom
chenyukang:fix-101880
Oct 23, 2022
Merged

Suggest let for assignment, and some code refactor#101908
bors merged 1 commit into
rust-lang:masterfrom
chenyukang:fix-101880

Conversation

@chenyukang

@chenyukangchenyukang commented Sep 16, 2022

Copy link
Copy Markdown
Member

Fixes#101880

@rustbotrustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Sep 16, 2022
@rust-highfive

rust-highfive commented Sep 16, 2022

Copy link
Copy Markdown
Contributor

r? @oli-obk

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfiverust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 16, 2022
Rageking8
Rageking8 reviewed Sep 16, 2022
View reviewed changes
Comment threadsrc/test/ui/suggestions/suggest-let-for-assignment.stderr Outdated
@chenyukang
chenyukang marked this pull request as draft September 16, 2022 16:56
@rust-log-analyzer

This comment has been minimized.

Sign in to view
estebank
estebank reviewed Sep 16, 2022
View reviewed changes
Comment threadcompiler/rustc_resolve/src/late/diagnostics.rs Outdated
estebank
estebank reviewed Sep 16, 2022
View reviewed changes
Comment threadcompiler/rustc_resolve/src/late/diagnostics.rs Outdated
estebank
estebank reviewed Sep 16, 2022
View reviewed changes
Comment threadcompiler/rustc_resolve/src/late/diagnostics.rs Outdated
estebank
estebank reviewed Sep 16, 2022
View reviewed changes
Comment threadsrc/test/ui/suggestions/suggest-let-for-assignment.stderr Outdated
estebank
estebank reviewed Sep 16, 2022
View reviewed changes
Comment threadcompiler/rustc_resolve/src/late/diagnostics.rs Outdated
@chenyukang

chenyukang commented Sep 17, 2022•
edited
Loading

Copy link
Copy Markdown
MemberAuthor

By the way, smart_resolve_report_errors is tooo long and seems hard to maintain 😂
Should we refactor this part? @estebank

chenyukang
chenyukang commented Sep 17, 2022
View reviewed changes
Comment threadsrc/test/ui/suggestions/suggest-let-for-assignment.stderr Outdated
@chenyukang
chenyukang marked this pull request as ready for review September 17, 2022 06:21
chenyukang
chenyukang commented Sep 17, 2022
View reviewed changes
Comment threadcompiler/rustc_resolve/src/late/diagnostics.rs Outdated
@estebank

estebank commented Sep 19, 2022

Copy link
Copy Markdown
Contributor

By the way, smart_resolve_report_errors is tooo long and seems hard to maintain 😂 Should we refactor this part?

We should, but better to do it as part of its own PR.

@chenyukangchenyukang mentioned this pull request Sep 21, 2022
Merged
@bors

bors commented Sep 29, 2022

Copy link
Copy Markdown
Collaborator

☔ The latest upstream changes (presumably #102450) made this pull request unmergeable. Please resolve the merge conflicts.

@chenyukang
chenyukangforce-pushed the fix-101880 branch 4 times, most recently from 99a33b3 to 7d201c9CompareSeptember 29, 2022 15:29
@chenyukang

chenyukang commented Oct 14, 2022

Copy link
Copy Markdown
MemberAuthor

r? @estebank

@bors

bors commented Oct 20, 2022

Copy link
Copy Markdown
Collaborator

☔ The latest upstream changes (presumably #103185) made this pull request unmergeable. Please resolve the merge conflicts.

estebank
estebank approved these changes Oct 21, 2022
View reviewed changes

@estebankestebank left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

r=me after rebasing (ping me and I'll approve)

Comment threadcompiler/rustc_resolve/src/late/diagnostics.rs Outdated
Comment on lines 1715 to 1716

@estebankestebankOct 21, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This can now be written as follows

Suggested change
let has_eq = matches!(sm.span_to_snippet(eq_span),Ok(ref snippet)if snippet == "=");
if has_eq {
ifletOk(ref snippet) = sm.span_to_snippet(eq_span) && snippet == "="{

Comment threadcompiler/rustc_resolve/src/late/diagnostics.rs Outdated
Comment on lines 1747 to 1742

@estebankestebankOct 21, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this check to protect against macros or something else?

@estebankestebankOct 21, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ah, yes. Saw the dbg!(binding) case in the test. Hacky, but makes sense!

@chenyukangchenyukangOct 21, 2022

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

emm, I will add a comment for this.

@rust-log-analyzer

This comment has been minimized.

Sign in to view
chenyukang
chenyukang commented Oct 21, 2022
View reviewed changes
Comment threadcompiler/rustc_resolve/src/late/diagnostics.rs
break;
}
}
let closing_span = sm.span_look_ahead(span, Some("}"), Some(50));

@chenyukangchenyukangOct 21, 2022

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Since the next_point bug is fixed, this is changed from 100 to 50 😂

@chenyukang

chenyukang commented Oct 21, 2022

Copy link
Copy Markdown
MemberAuthor

r? @estebank

@estebank

estebank commented Oct 21, 2022

Copy link
Copy Markdown
Contributor

@bors r+

@bors

bors commented Oct 21, 2022

Copy link
Copy Markdown
Collaborator

📌 Commit eb68e27 has been approved by estebank

It is now in the queue for this repository.

@borsbors 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 Oct 21, 2022
notriddle added a commit to notriddle/rust that referenced this pull request Oct 21, 2022
@notriddle
Rollup merge ofrust-lang#101908- chenyukang:fix-101880, r=estebank
b22480f
Suggest let for assignment, and some code refactor
Fixesrust-lang#101880
@notriddlenotriddle mentioned this pull request Oct 21, 2022
Closed
@Dylan-DPCDylan-DPC mentioned this pull request Oct 23, 2022
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 23, 2022
@bors
Auto merge ofrust-lang#103431- Dylan-DPC:rollup-oozfo89, r=Dylan-DPC
9be2f35
Rollup of 6 pull requests
Successful merges:
- rust-lang#101293 (Recover when unclosed char literal is parsed as a lifetime in some positions)
- rust-lang#101908 (Suggest let for assignment, and some code refactor)
- rust-lang#103192 (rustdoc: Eliminate uses of `EarlyDocLinkResolver::all_traits`)
- rust-lang#103226 (Check `needs_infer` before `needs_drop` during HIR generator analysis)
- rust-lang#103249 (resolve: Revert "Set effective visibilities for imports more precisely")
- rust-lang#103305 (Move some tests to more reasonable places)
Failed merges:
r? `@ghost`
`@rustbot` modify labels: rollup
@bors
bors merged commit 518d5eb into rust-lang:masterOct 23, 2022
@rustbotrustbot added this to the 1.66.0 milestone Oct 23, 2022
@chenyukangchenyukang mentioned this pull request Oct 24, 2022
Closed
@Kaligule

Kaligule commented Oct 28, 2022

Copy link
Copy Markdown

Thank you @chenyukang

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

Reviewers

@estebankestebankestebank approved these changes
+1 more reviewer
@Rageking8Rageking8Rageking8 left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@estebankestebank

Labels

S-waiting-on-borsStatus: Waiting on bors to run and complete tests. Bors will change the label on completion.T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Milestone

1.66.0

Development

Successfully merging this pull request may close these issues.

Better error messages: Tell programmer about forgotten let

9 participants

@chenyukang@rust-highfive@rust-log-analyzer@estebank@bors@Kaligule@Rageking8@oli-obk@rustbot