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

Provide diagnostic suggestion in ExprUseVisitor Delegate #78662

Merged
merged 2 commits into from
Nov 5, 2020

Conversation

null-sleep
Copy link
Contributor

@null-sleep null-sleep commented Nov 2, 2020

The Delegate trait currently use PlaceWithHirId which is composed of Hir Place and the
corresponding expression id.

Even though this is an accurate way of expressing how a Place is used,
it can cause confusion during diagnostics.

Eg:

let arr : [String; 5];

let [a, ...]     =   arr;
 ^^^ E1 ^^^      =  ^^E2^^

Here arr is moved because of the binding created E1. However, when we
point to E1 in diagnostics with the message arr was moved, it can be
confusing. Rather we would like to report E2 to the user.

Closes: rust-lang/project-rfc-2229#20

r? @ghost

The [Delegate
trait](https://github.com/rust-lang/rust/blob/981346fc07dd5ef414c5b1b21999f7604cece006/compiler/rustc_typeck/src/expr_use_visitor.rs#L28-L38)
currently use `PlaceWithHirId` which is composed of Hir `Place` and the
corresponding expression id.

Even though this is an accurate way of expressing how a Place is used,
it can cause confusion during diagnostics.

Eg:

```
let arr : [String; 5];

let [a, ...]     =   arr;
 ^^^ E1 ^^^      =  ^^E2^^
 ```

 Here `arr` is moved because of the binding created E1. However, when we
 point to E1 in diagnostics with the message `arr` was moved, it can be
 confusing.  Rather we would like to report E2 to the user.

 Closes: rust-lang/project-rfc-2229#20
@null-sleep
Copy link
Contributor Author

Making sure clippy tests pass :)

@null-sleep
Copy link
Contributor Author

r? @nikomatsakis

Copy link
Contributor

@nikomatsakis nikomatsakis left a comment

Choose a reason for hiding this comment

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

Looks good, I left a few wording nits.

compiler/rustc_typeck/src/expr_use_visitor.rs Outdated Show resolved Hide resolved
compiler/rustc_typeck/src/expr_use_visitor.rs Outdated Show resolved Hide resolved
compiler/rustc_typeck/src/expr_use_visitor.rs Outdated Show resolved Hide resolved
@camelid camelid added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. A-diagnostics Area: Messages for errors, warnings, and lints labels Nov 3, 2020
@null-sleep
Copy link
Contributor Author

@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 Nov 4, 2020
@nikomatsakis
Copy link
Contributor

@bors t+

@nikomatsakis
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Nov 4, 2020

📌 Commit c9d9359 has been approved by nikomatsakis

@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 Nov 4, 2020
@bors
Copy link
Contributor

bors commented Nov 4, 2020

⌛ Testing commit c9d9359 with merge 0fb0025...

@bors
Copy link
Contributor

bors commented Nov 5, 2020

☀️ Test successful - checks-actions
Approved by: nikomatsakis
Pushing 0fb0025 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Nov 5, 2020
@bors bors merged commit 0fb0025 into rust-lang:master Nov 5, 2020
@rustbot rustbot added this to the 1.49.0 milestone Nov 5, 2020
flip1995 pushed a commit to flip1995/rust that referenced this pull request Nov 5, 2020
…r=nikomatsakis

Provide diagnostic suggestion in ExprUseVisitor Delegate

The [Delegate trait](https://github.com/rust-lang/rust/blob/981346fc07dd5ef414c5b1b21999f7604cece006/compiler/rustc_typeck/src/expr_use_visitor.rs#L28-L38) currently use `PlaceWithHirId` which is composed of Hir `Place` and the
corresponding expression id.

Even though this is an accurate way of expressing how a Place is used,
it can cause confusion during diagnostics.

Eg:

```
let arr : [String; 5];

let [a, ...]     =   arr;
 ^^^ E1 ^^^      =  ^^E2^^
 ```

 Here `arr` is moved because of the binding created E1. However, when we
 point to E1 in diagnostics with the message `arr` was moved, it can be
 confusing.  Rather we would like to report E2 to the user.

 Closes: rust-lang/project-rfc-2229#20

r? `@ghost`
@arora-aman arora-aman deleted the add_expr_id_to_delegate branch November 17, 2020 07:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Provide diagnostic suggestion in ExprUseVisitor Delegate
6 participants