-
Notifications
You must be signed in to change notification settings - Fork 13.4k
Custom error when moving arg outside of its closure #47144
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
Changes from 5 commits
8460421
c31c60c
2c5f2df
1a1afd7
1820da5
6f9ecaa
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,94 @@ | ||
// Copyright 2018 The Rust Project Developers. See the COPYRIGHT | ||
// file at the top-level directory of this distribution and at | ||
// http://rust-lang.org/COPYRIGHT. | ||
// | ||
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or | ||
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license | ||
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your | ||
// option. This file may not be copied, modified, or distributed | ||
// except according to those terms. | ||
|
||
//! Error Reporting for Anonymous Region Lifetime Errors | ||
//! where both the regions are anonymous. | ||
|
||
use infer::error_reporting::nice_region_error::NiceRegionError; | ||
use infer::SubregionOrigin; | ||
use ty::RegionKind; | ||
use hir::{Expr, ExprClosure}; | ||
use hir::map::NodeExpr; | ||
use util::common::ErrorReported; | ||
use infer::lexical_region_resolve::RegionResolutionError::SubSupConflict; | ||
|
||
impl<'a, 'gcx, 'tcx> NiceRegionError<'a, 'gcx, 'tcx> { | ||
/// Print the error message for lifetime errors when binding excapes a closure. | ||
/// | ||
/// Consider a case where we have | ||
/// | ||
/// ```no_run | ||
/// fn with_int<F>(f: F) where F: FnOnce(&isize) { | ||
/// let x = 3; | ||
/// f(&x); | ||
/// } | ||
/// fn main() { | ||
/// let mut x = None; | ||
/// with_int(|y| x = Some(y)); | ||
/// } | ||
/// ``` | ||
/// | ||
/// the output will be | ||
/// | ||
/// ```text | ||
/// let mut x = None; | ||
/// ----- borrowed data cannot be stored into here... | ||
/// with_int(|y| x = Some(y)); | ||
/// --- ^ cannot be stored outside of its closure | ||
/// | | ||
/// ...because it cannot outlive this closure | ||
/// ``` | ||
pub(super) fn try_report_outlives_closure(&self) -> Option<ErrorReported> { | ||
if let Some(SubSupConflict(origin, | ||
ref sub_origin, | ||
_, | ||
ref sup_origin, | ||
sup_region)) = self.error { | ||
|
||
// #45983: when trying to assign the contents of an argument to a binding outside of a | ||
// closure, provide a specific message pointing this out. | ||
if let (&SubregionOrigin::BindingTypeIsNotValidAtDecl(ref external_span), | ||
&RegionKind::ReFree(ref free_region)) = (&sub_origin, sup_region) { | ||
let hir = &self.tcx.hir; | ||
if let Some(node_id) = hir.as_local_node_id(free_region.scope) { | ||
match hir.get(node_id) { | ||
NodeExpr(Expr { | ||
node: ExprClosure(_, _, _, closure_span, false), | ||
.. | ||
}) => { | ||
let sup_sp = sup_origin.span(); | ||
let origin_sp = origin.span(); | ||
let mut err = self.tcx.sess.struct_span_err( | ||
sup_sp, | ||
"borrowed data cannot be stored outside of its closure"); | ||
err.span_label(sup_sp, "cannot be stored outside of its closure"); | ||
if sup_sp == origin_sp { | ||
err.span_label(*external_span, | ||
"borrowed data cannot be stored into here..."); | ||
err.span_label(*closure_span, | ||
"...because it cannot outlive this closure"); | ||
} else { | ||
err.span_label(*closure_span, | ||
"borrowed data cannot outlive this closure"); | ||
err.span_label(origin_sp, | ||
"cannot infer an appropriate lifetime"); | ||
} | ||
err.emit(); | ||
return Some(ErrorReported); | ||
} | ||
_ => {} | ||
} | ||
} | ||
} | ||
} | ||
None | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
// Copyright 2018 The Rust Project Developers. See the COPYRIGHT | ||
// file at the top-level directory of this distribution and at | ||
// http://rust-lang.org/COPYRIGHT. | ||
// | ||
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or | ||
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license | ||
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your | ||
// option. This file may not be copied, modified, or distributed | ||
// except according to those terms. | ||
|
||
fn give_any<F: for<'r> FnOnce(&'r ())>(f: F) { | ||
f(&()); | ||
} | ||
|
||
fn main() { | ||
let x = None; | ||
give_any(|y| x = Some(y)); | ||
//~^ ERROR borrowed data cannot be stored outside of its closure | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
error: borrowed data cannot be stored outside of its closure | ||
--> $DIR/issue-45983.rs:17:27 | ||
| | ||
16 | let x = None; | ||
| - borrowed data cannot be stored into here... | ||
17 | give_any(|y| x = Some(y)); | ||
| --- ^ cannot be stored outside of its closure | ||
| | | ||
| ...because it cannot outlive this closure | ||
|
||
error: aborting due to previous error | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can you add a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Moved existing test that shows the behavior to ui: https://github.com/rust-lang/rust/pull/47144/files#diff-e5107c6d2a404ecd34f782e9e9e8b20f |
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
error: borrowed data cannot be stored outside of its closure | ||
--> $DIR/issue-7573.rs:31:27 | ||
| | ||
27 | let mut lines_to_use: Vec<&CrateId> = Vec::new(); | ||
| - cannot infer an appropriate lifetime | ||
28 | //~^ NOTE cannot infer an appropriate lifetime | ||
29 | let push_id = |installed_id: &CrateId| { | ||
| ------------------------ borrowed data cannot outlive this closure | ||
30 | //~^ NOTE borrowed data cannot outlive this closure | ||
31 | lines_to_use.push(installed_id); | ||
| ^^^^^^^^^^^^ cannot be stored outside of its closure | ||
|
||
error: aborting due to previous error | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
error: borrowed data cannot be stored outside of its closure | ||
--> $DIR/regions-escape-bound-fn-2.rs:18:27 | ||
| | ||
17 | let mut x = None; | ||
| ----- borrowed data cannot be stored into here... | ||
18 | with_int(|y| x = Some(y)); | ||
| --- ^ cannot be stored outside of its closure | ||
| | | ||
| ...because it cannot outlive this closure | ||
|
||
error: aborting due to previous error | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
error: borrowed data cannot be stored outside of its closure | ||
--> $DIR/regions-escape-bound-fn.rs:18:27 | ||
| | ||
18 | with_int(|y| x = Some(y)); | ||
| --- -----^- | ||
| | | | | ||
| | | cannot be stored outside of its closure | ||
| | cannot infer an appropriate lifetime | ||
| borrowed data cannot outlive this closure | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @nikomatsakis I'm not entirely happy with the output on this case :-/ This triggers the case I originally neglected and was trying to fix... Should we keep this PR on hold until I have time to look at how to differentiate between these two cases for a more appropriate output? In my mind, this case should completely ignore the lifetime issue for
It should be possible to do so as the current diagnostic does point at
Also, should I create a new error code for each of these cases, one shared between all of them or reuse the existing one? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this output is way better than the original output. The main change I would do is not to print the "cannot infer an appropriate lifetime" label, which seems to add zero value. Saying that the data cannot escape the closure and indicating the closure is pretty awesome. |
||
|
||
error: aborting due to previous error | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
error: borrowed data cannot be stored outside of its closure | ||
--> $DIR/regions-escape-unboxed-closure.rs:16:32 | ||
| | ||
16 | with_int(&mut |y| x = Some(y)); | ||
| --- -----^- | ||
| | | | | ||
| | | cannot be stored outside of its closure | ||
| | cannot infer an appropriate lifetime | ||
| borrowed data cannot outlive this closure | ||
|
||
error: aborting due to previous error | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should just drop this label. I don't really think it conveys useful information to the end-user. The
origin_sp
is not especially meaningful -- it's basically a point where the region is relevant, but not necessarily a particularly good choice of point.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel uneasy about removing the span completely because it's the original error span, and want to point to it in some way. Modified so that it occurs in fewer cases, but will keep it for the #7573 case. I'm planning to revisit that and the type mismatch cases at a later PR.