Skip to content

Commit

Permalink
Rollup merge of rust-lang#63051 - estebank:borrow-ice, r=matthewjasper
Browse files Browse the repository at this point in the history
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
```
  • Loading branch information
Centril committed Jul 28, 2019
2 parents 117ee3b + 01ba0e3 commit a55b8e8
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 13 deletions.
25 changes: 12 additions & 13 deletions src/librustc_mir/borrow_check/conflict_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1140,19 +1140,18 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
bug!("try_report_cannot_return_reference_to_local: not a local")
};
match self.body.local_kind(*local) {
LocalKind::ReturnPointer | LocalKind::Temp => {
(
"temporary value".to_string(),
"temporary value created here".to_string(),
)
}
LocalKind::Arg => {
(
"function parameter".to_string(),
"function parameter borrowed here".to_string(),
)
},
LocalKind::Var => bug!("local variable without a name"),
LocalKind::ReturnPointer | LocalKind::Temp => (
"temporary value".to_string(),
"temporary value created here".to_string(),
),
LocalKind::Arg => (
"function parameter".to_string(),
"function parameter borrowed here".to_string(),
),
LocalKind::Var => (
"local binding".to_string(),
"local binding introduced here".to_string(),
),
}
};

Expand Down
33 changes: 33 additions & 0 deletions src/test/ui/borrowck/return-local-binding-from-desugaring.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// To avoid leaking the names of local bindings from expressions like for loops, #60984
// explicitly ignored them, but an assertion that `LocalKind::Var` *must* have a name would
// trigger an ICE. Before this change, this file's output would be:
// ```
// 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
// ```
// FIXME: ideally `LocalKind` would carry more information to more accurately explain the problem.

use std::collections::HashMap;
use std::hash::Hash;

fn group_by<I, F, T>(xs: &mut I, f: F) -> HashMap<T, Vec<&I::Item>>
where
I: Iterator,
F: Fn(&I::Item) -> T,
T: Eq + Hash,
{
let mut result = HashMap::new();
for ref x in xs {
let key = f(x);
result.entry(key).or_insert(Vec::new()).push(x);
}
result //~ ERROR cannot return value referencing local binding
}

fn main() {}
12 changes: 12 additions & 0 deletions src/test/ui/borrowck/return-local-binding-from-desugaring.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
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

error: aborting due to previous error

For more information about this error, try `rustc --explain E0515`.

0 comments on commit a55b8e8

Please sign in to comment.