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

Borrowck error reporting cleanup #60984

Merged
merged 3 commits into from
May 24, 2019

Conversation

matthewjasper
Copy link
Contributor

  • Don't show variables created by desugarings in borrowck errors
  • Move "conflict error" reporting to it's own module, so that error_reporting contains only common error reporting methods.
  • Remove unused ScopeTree parameter.

r? @pnkfelix

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 20, 2019
@pnkfelix
Copy link
Member

r=me once review nits above are addressed.

@matthewjasper matthewjasper force-pushed the borrowck-error-reporting-cleanup branch from c6111c5 to f8e9031 Compare May 21, 2019 18:36
@matthewjasper
Copy link
Contributor Author

@bors r=pnkfelix

@bors
Copy link
Contributor

bors commented May 21, 2019

📌 Commit f8e90318986288934ddea949d6096fd93ed587cc has been approved by pnkfelix

@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 May 21, 2019
@matthewjasper matthewjasper force-pushed the borrowck-error-reporting-cleanup branch from f8e9031 to 8dc945c Compare May 21, 2019 19:38
@matthewjasper
Copy link
Contributor Author

@bors r=pnkfelix

@bors
Copy link
Contributor

bors commented May 21, 2019

📌 Commit 8dc945c has been approved by pnkfelix

@Centril
Copy link
Contributor

Centril commented May 21, 2019

@bors rollup=never

@bors
Copy link
Contributor

bors commented May 24, 2019

⌛ Testing commit 8dc945c with merge 4680580...

bors added a commit that referenced this pull request May 24, 2019
…, r=pnkfelix

Borrowck error reporting cleanup

* Don't show variables created by desugarings in borrowck errors
* Move "conflict error" reporting to it's own module, so that `error_reporting` contains only common error reporting methods.
* Remove unused `ScopeTree` parameter.

r? @pnkfelix
@bors
Copy link
Contributor

bors commented May 24, 2019

☀️ Test successful - checks-travis, status-appveyor
Approved by: pnkfelix
Pushing 4680580 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label May 24, 2019
@bors bors merged commit 8dc945c into rust-lang:master May 24, 2019
@matthewjasper matthewjasper deleted the borrowck-error-reporting-cleanup branch May 24, 2019 06:25
Centril added a commit to Centril/rust that referenced this pull request Jul 28, 2019
Avoid ICE when referencing desugared local binding in borrow error

To avoid leaking the names of local bindings from expressions like for loops, rust-lang#60984 explicitly ignored them, but an assertion that `LocalKind::Var` *must* have a name would trigger an ICE.

Before this change, the binding generated by desugaring the for loop would leak into the diagnostic (rust-lang#63027):
```
error[E0515]: cannot return value referencing local variable `__next`
  --> return-local-binding-from-desugaring.rs:LL:CC
   |
LL |     for ref x in xs {
   |         ----- `__next` is borrowed here
...
LL |     result
   |     ^^^^^^ returns a value referencing data owned by the current function
```

Ideally `LocalKind` would carry more information to more accurately explain the problem, but for now, in order to avoid the ICE (fix rust-lang#63026), we accept `LocalKind::Var` without a name and produce the following output:

```
error[E0515]: cannot return value referencing local binding
  --> $DIR/return-local-binding-from-desugaring.rs:30:5
   |
LL |     for ref x in xs {
   |                  -- local binding introduced here
...
LL |     result
   |     ^^^^^^ returns a value referencing data owned by the current function
```
Centril added a commit to Centril/rust that referenced this pull request Jul 28, 2019
Avoid ICE when referencing desugared local binding in borrow error

To avoid leaking the names of local bindings from expressions like for loops, rust-lang#60984 explicitly ignored them, but an assertion that `LocalKind::Var` *must* have a name would trigger an ICE.

Before this change, the binding generated by desugaring the for loop would leak into the diagnostic (rust-lang#63027):
```
error[E0515]: cannot return value referencing local variable `__next`
  --> return-local-binding-from-desugaring.rs:LL:CC
   |
LL |     for ref x in xs {
   |         ----- `__next` is borrowed here
...
LL |     result
   |     ^^^^^^ returns a value referencing data owned by the current function
```

Ideally `LocalKind` would carry more information to more accurately explain the problem, but for now, in order to avoid the ICE (fix rust-lang#63026), we accept `LocalKind::Var` without a name and produce the following output:

```
error[E0515]: cannot return value referencing local binding
  --> $DIR/return-local-binding-from-desugaring.rs:30:5
   |
LL |     for ref x in xs {
   |                  -- local binding introduced here
...
LL |     result
   |     ^^^^^^ returns a value referencing data owned by the current function
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

5 participants