From 78b21643a85a6a7e17964889122829a89e6da593 Mon Sep 17 00:00:00 2001 From: David Wood Date: Sun, 2 Sep 2018 00:42:51 +0200 Subject: [PATCH 1/6] Moved helper functions from NiceRegionError to TyCtxt. --- .../nice_region_error/different_lifetimes.rs | 4 +- .../nice_region_error/find_anon_type.rs | 2 +- .../nice_region_error/named_anon_conflict.rs | 8 +- .../nice_region_error/static_impl_trait.rs | 4 +- .../error_reporting/nice_region_error/util.rs | 79 ------------------- src/librustc/ty/context.rs | 79 ++++++++++++++++++- src/librustc/ty/mod.rs | 2 +- 7 files changed, 88 insertions(+), 90 deletions(-) diff --git a/src/librustc/infer/error_reporting/nice_region_error/different_lifetimes.rs b/src/librustc/infer/error_reporting/nice_region_error/different_lifetimes.rs index 168285b455f0a..97b1978cb2eb2 100644 --- a/src/librustc/infer/error_reporting/nice_region_error/different_lifetimes.rs +++ b/src/librustc/infer/error_reporting/nice_region_error/different_lifetimes.rs @@ -56,9 +56,9 @@ impl<'a, 'gcx, 'tcx> NiceRegionError<'a, 'gcx, 'tcx> { let (span, sub, sup) = self.get_regions(); // Determine whether the sub and sup consist of both anonymous (elided) regions. - let anon_reg_sup = self.is_suitable_region(sup)?; + let anon_reg_sup = self.tcx.is_suitable_region(sup)?; - let anon_reg_sub = self.is_suitable_region(sub)?; + let anon_reg_sub = self.tcx.is_suitable_region(sub)?; let scope_def_id_sup = anon_reg_sup.def_id; let bregion_sup = anon_reg_sup.boundregion; let scope_def_id_sub = anon_reg_sub.def_id; diff --git a/src/librustc/infer/error_reporting/nice_region_error/find_anon_type.rs b/src/librustc/infer/error_reporting/nice_region_error/find_anon_type.rs index e3faf755672a1..10a2d179dd564 100644 --- a/src/librustc/infer/error_reporting/nice_region_error/find_anon_type.rs +++ b/src/librustc/infer/error_reporting/nice_region_error/find_anon_type.rs @@ -36,7 +36,7 @@ impl<'a, 'gcx, 'tcx> NiceRegionError<'a, 'gcx, 'tcx> { region: Region<'tcx>, br: &ty::BoundRegion, ) -> Option<(&hir::Ty, &hir::FnDecl)> { - if let Some(anon_reg) = self.is_suitable_region(region) { + if let Some(anon_reg) = self.tcx.is_suitable_region(region) { let def_id = anon_reg.def_id; if let Some(node_id) = self.tcx.hir.as_local_node_id(def_id) { let fndecl = match self.tcx.hir.get(node_id) { diff --git a/src/librustc/infer/error_reporting/nice_region_error/named_anon_conflict.rs b/src/librustc/infer/error_reporting/nice_region_error/named_anon_conflict.rs index 4e26a4178b95d..0eb634ea2cc8e 100644 --- a/src/librustc/infer/error_reporting/nice_region_error/named_anon_conflict.rs +++ b/src/librustc/infer/error_reporting/nice_region_error/named_anon_conflict.rs @@ -33,23 +33,23 @@ impl<'a, 'gcx, 'tcx> NiceRegionError<'a, 'gcx, 'tcx> { // version new_ty of its type where the anonymous region is replaced // with the named one.//scope_def_id let (named, anon, anon_arg_info, region_info) = if self.is_named_region(sub) - && self.is_suitable_region(sup).is_some() + && self.tcx.is_suitable_region(sup).is_some() && self.find_arg_with_region(sup, sub).is_some() { ( sub, sup, self.find_arg_with_region(sup, sub).unwrap(), - self.is_suitable_region(sup).unwrap(), + self.tcx.is_suitable_region(sup).unwrap(), ) - } else if self.is_named_region(sup) && self.is_suitable_region(sub).is_some() + } else if self.is_named_region(sup) && self.tcx.is_suitable_region(sub).is_some() && self.find_arg_with_region(sub, sup).is_some() { ( sup, sub, self.find_arg_with_region(sub, sup).unwrap(), - self.is_suitable_region(sub).unwrap(), + self.tcx.is_suitable_region(sub).unwrap(), ) } else { return None; // inapplicable diff --git a/src/librustc/infer/error_reporting/nice_region_error/static_impl_trait.rs b/src/librustc/infer/error_reporting/nice_region_error/static_impl_trait.rs index d25dcd5b045ca..2f7c6adfd2185 100644 --- a/src/librustc/infer/error_reporting/nice_region_error/static_impl_trait.rs +++ b/src/librustc/infer/error_reporting/nice_region_error/static_impl_trait.rs @@ -27,9 +27,9 @@ impl<'a, 'gcx, 'tcx> NiceRegionError<'a, 'gcx, 'tcx> { sup_origin, sup_r, ) => { - let anon_reg_sup = self.is_suitable_region(sup_r)?; + let anon_reg_sup = self.tcx.is_suitable_region(sup_r)?; if sub_r == &RegionKind::ReStatic && - self.is_return_type_impl_trait(anon_reg_sup.def_id) + self.tcx.is_return_type_impl_trait(anon_reg_sup.def_id) { let sp = var_origin.span(); let return_sp = sub_origin.span(); diff --git a/src/librustc/infer/error_reporting/nice_region_error/util.rs b/src/librustc/infer/error_reporting/nice_region_error/util.rs index 30406f1fec55c..afc50fe115166 100644 --- a/src/librustc/infer/error_reporting/nice_region_error/util.rs +++ b/src/librustc/infer/error_reporting/nice_region_error/util.rs @@ -15,7 +15,6 @@ use hir; use infer::error_reporting::nice_region_error::NiceRegionError; use ty::{self, Region, Ty}; use hir::def_id::DefId; -use hir::Node; use syntax_pos::Span; // The struct contains the information about the anonymous region @@ -35,18 +34,6 @@ pub(super) struct AnonymousArgInfo<'tcx> { pub is_first: bool, } -// This struct contains information regarding the -// Refree((FreeRegion) corresponding to lifetime conflict -#[derive(Debug)] -pub(super) struct FreeRegionInfo { - // def id corresponding to FreeRegion - pub def_id: DefId, - // the bound region corresponding to FreeRegion - pub boundregion: ty::BoundRegion, - // checks if bound region is in Impl Item - pub is_impl_item: bool, -} - impl<'a, 'gcx, 'tcx> NiceRegionError<'a, 'gcx, 'tcx> { // This method walks the Type of the function body arguments using // `fold_regions()` function and returns the @@ -122,36 +109,6 @@ impl<'a, 'gcx, 'tcx> NiceRegionError<'a, 'gcx, 'tcx> { } } - // This method returns the DefId and the BoundRegion corresponding to the given region. - pub(super) fn is_suitable_region(&self, region: Region<'tcx>) -> Option { - let (suitable_region_binding_scope, bound_region) = match *region { - ty::ReFree(ref free_region) => (free_region.scope, free_region.bound_region), - ty::ReEarlyBound(ref ebr) => ( - self.tcx.parent_def_id(ebr.def_id).unwrap(), - ty::BoundRegion::BrNamed(ebr.def_id, ebr.name), - ), - _ => return None, // not a free region - }; - - let node_id = self.tcx - .hir - .as_local_node_id(suitable_region_binding_scope) - .unwrap(); - let is_impl_item = match self.tcx.hir.find(node_id) { - Some(Node::Item(..)) | Some(Node::TraitItem(..)) => false, - Some(Node::ImplItem(..)) => { - self.is_bound_region_in_impl_item(suitable_region_binding_scope) - } - _ => return None, - }; - - return Some(FreeRegionInfo { - def_id: suitable_region_binding_scope, - boundregion: bound_region, - is_impl_item: is_impl_item, - }); - } - // Here, we check for the case where the anonymous region // is in the return type. // FIXME(#42703) - Need to handle certain cases here. @@ -176,22 +133,6 @@ impl<'a, 'gcx, 'tcx> NiceRegionError<'a, 'gcx, 'tcx> { None } - pub(super) fn is_return_type_impl_trait( - &self, - scope_def_id: DefId, - ) -> bool { - let ret_ty = self.tcx.type_of(scope_def_id); - match ret_ty.sty { - ty::FnDef(_, _) => { - let sig = ret_ty.fn_sig(self.tcx); - let output = self.tcx.erase_late_bound_regions(&sig.output()); - return output.is_impl_trait(); - } - _ => {} - } - false - } - // Here we check for the case where anonymous region // corresponds to self and if yes, we display E0312. // FIXME(#42700) - Need to format self properly to @@ -203,24 +144,4 @@ impl<'a, 'gcx, 'tcx> NiceRegionError<'a, 'gcx, 'tcx> { .map(|i| i.method_has_self_argument) == Some(true) } - // Here we check if the bound region is in Impl Item. - pub(super) fn is_bound_region_in_impl_item( - &self, - suitable_region_binding_scope: DefId, - ) -> bool { - let container_id = self.tcx - .associated_item(suitable_region_binding_scope) - .container - .id(); - if self.tcx.impl_trait_ref(container_id).is_some() { - // For now, we do not try to target impls of traits. This is - // because this message is going to suggest that the user - // change the fn signature, but they may not be free to do so, - // since the signature must match the trait. - // - // FIXME(#42706) -- in some cases, we could do better here. - return true; - } - false - } } diff --git a/src/librustc/ty/context.rs b/src/librustc/ty/context.rs index 83d6b715e95bc..c1ae7bba50a29 100644 --- a/src/librustc/ty/context.rs +++ b/src/librustc/ty/context.rs @@ -17,7 +17,7 @@ use session::Session; use session::config::{BorrowckMode, OutputFilenames}; use session::config::CrateType; use middle; -use hir::{TraitCandidate, HirId, ItemLocalId}; +use hir::{TraitCandidate, HirId, ItemLocalId, Node}; use hir::def::{Def, Export}; use hir::def_id::{CrateNum, DefId, DefIndex, LOCAL_CRATE}; use hir::map as hir_map; @@ -870,6 +870,18 @@ impl<'tcx> CommonTypes<'tcx> { } } +// This struct contains information regarding the `ReFree(FreeRegion)` corresponding to a lifetime +// conflict. +#[derive(Debug)] +pub struct FreeRegionInfo { + // def id corresponding to FreeRegion + pub def_id: DefId, + // the bound region corresponding to FreeRegion + pub boundregion: ty::BoundRegion, + // checks if bound region is in Impl Item + pub is_impl_item: bool, +} + /// The central data structure of the compiler. It stores references /// to the various **arenas** and also houses the results of the /// various **compiler queries** that have been performed. See the @@ -1545,6 +1557,71 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> { } }) } + + // This method returns the DefId and the BoundRegion corresponding to the given region. + pub fn is_suitable_region(&self, region: Region<'tcx>) -> Option { + let (suitable_region_binding_scope, bound_region) = match *region { + ty::ReFree(ref free_region) => (free_region.scope, free_region.bound_region), + ty::ReEarlyBound(ref ebr) => ( + self.parent_def_id(ebr.def_id).unwrap(), + ty::BoundRegion::BrNamed(ebr.def_id, ebr.name), + ), + _ => return None, // not a free region + }; + + let node_id = self.hir + .as_local_node_id(suitable_region_binding_scope) + .unwrap(); + let is_impl_item = match self.hir.find(node_id) { + Some(Node::Item(..)) | Some(Node::TraitItem(..)) => false, + Some(Node::ImplItem(..)) => { + self.is_bound_region_in_impl_item(suitable_region_binding_scope) + } + _ => return None, + }; + + return Some(FreeRegionInfo { + def_id: suitable_region_binding_scope, + boundregion: bound_region, + is_impl_item: is_impl_item, + }); + } + + pub fn is_return_type_impl_trait( + &self, + scope_def_id: DefId, + ) -> bool { + let ret_ty = self.type_of(scope_def_id); + match ret_ty.sty { + ty::FnDef(_, _) => { + let sig = ret_ty.fn_sig(*self); + let output = self.erase_late_bound_regions(&sig.output()); + return output.is_impl_trait(); + } + _ => {} + } + false + } + + // Here we check if the bound region is in Impl Item. + pub fn is_bound_region_in_impl_item( + &self, + suitable_region_binding_scope: DefId, + ) -> bool { + let container_id = self.associated_item(suitable_region_binding_scope) + .container + .id(); + if self.impl_trait_ref(container_id).is_some() { + // For now, we do not try to target impls of traits. This is + // because this message is going to suggest that the user + // change the fn signature, but they may not be free to do so, + // since the signature must match the trait. + // + // FIXME(#42706) -- in some cases, we could do better here. + return true; + } + false + } } impl<'a, 'tcx> TyCtxt<'a, 'tcx, 'tcx> { diff --git a/src/librustc/ty/mod.rs b/src/librustc/ty/mod.rs index a48aabc3cd7c7..3732e7d8fd2fb 100644 --- a/src/librustc/ty/mod.rs +++ b/src/librustc/ty/mod.rs @@ -80,7 +80,7 @@ pub use self::sty::TyKind::*; pub use self::binding::BindingMode; pub use self::binding::BindingMode::*; -pub use self::context::{TyCtxt, GlobalArenas, AllArenas, tls, keep_local}; +pub use self::context::{TyCtxt, FreeRegionInfo, GlobalArenas, AllArenas, tls, keep_local}; pub use self::context::{Lift, TypeckTables}; pub use self::instance::{Instance, InstanceDef}; From 6bf131f3f4c978b93e709533d8d7ff4cb22e4769 Mon Sep 17 00:00:00 2001 From: David Wood Date: Sun, 2 Sep 2018 11:10:51 +0200 Subject: [PATCH 2/6] Added help message for impl trait static constraint. --- .../nice_region_error/static_impl_trait.rs | 2 +- src/librustc/ty/context.rs | 13 +++-- .../nll/region_infer/error_reporting/mod.rs | 57 ++++++++++++++++--- ...t_outlive_least_region_or_bound.nll.stderr | 8 +++ .../static-return-lifetime-infered.nll.stderr | 8 +++ 5 files changed, 73 insertions(+), 15 deletions(-) diff --git a/src/librustc/infer/error_reporting/nice_region_error/static_impl_trait.rs b/src/librustc/infer/error_reporting/nice_region_error/static_impl_trait.rs index 2f7c6adfd2185..29ba23b58bc0c 100644 --- a/src/librustc/infer/error_reporting/nice_region_error/static_impl_trait.rs +++ b/src/librustc/infer/error_reporting/nice_region_error/static_impl_trait.rs @@ -29,7 +29,7 @@ impl<'a, 'gcx, 'tcx> NiceRegionError<'a, 'gcx, 'tcx> { ) => { let anon_reg_sup = self.tcx.is_suitable_region(sup_r)?; if sub_r == &RegionKind::ReStatic && - self.tcx.is_return_type_impl_trait(anon_reg_sup.def_id) + self.tcx.return_type_impl_trait(anon_reg_sup.def_id).is_some() { let sp = var_origin.span(); let return_sp = sub_origin.span(); diff --git a/src/librustc/ty/context.rs b/src/librustc/ty/context.rs index c1ae7bba50a29..ae840f93c20e4 100644 --- a/src/librustc/ty/context.rs +++ b/src/librustc/ty/context.rs @@ -1587,20 +1587,23 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> { }); } - pub fn is_return_type_impl_trait( + pub fn return_type_impl_trait( &self, scope_def_id: DefId, - ) -> bool { + ) -> Option { let ret_ty = self.type_of(scope_def_id); match ret_ty.sty { ty::FnDef(_, _) => { let sig = ret_ty.fn_sig(*self); let output = self.erase_late_bound_regions(&sig.output()); - return output.is_impl_trait(); + if output.is_impl_trait() { + Some(output) + } else { + None + } } - _ => {} + _ => None } - false } // Here we check if the bound region is in Impl Item. diff --git a/src/librustc_mir/borrow_check/nll/region_infer/error_reporting/mod.rs b/src/librustc_mir/borrow_check/nll/region_infer/error_reporting/mod.rs index 0b9b9b33b3f15..2d8f782f748ec 100644 --- a/src/librustc_mir/borrow_check/nll/region_infer/error_reporting/mod.rs +++ b/src/librustc_mir/borrow_check/nll/region_infer/error_reporting/mod.rs @@ -15,7 +15,7 @@ use rustc::hir::def_id::DefId; use rustc::infer::error_reporting::nice_region_error::NiceRegionError; use rustc::infer::InferCtxt; use rustc::mir::{self, Location, Mir, Place, Rvalue, StatementKind, TerminatorKind}; -use rustc::ty::{TyCtxt, RegionVid}; +use rustc::ty::{TyCtxt, Ty, TyS, TyKind, Region, RegionKind, RegionVid}; use rustc_data_structures::indexed_vec::IndexVec; use rustc_errors::Diagnostic; use std::collections::VecDeque; @@ -344,7 +344,9 @@ impl<'tcx> RegionInferenceContext<'tcx> { ); // Check if we can use one of the "nice region errors". - if let (Some(f), Some(o)) = (self.to_error_region(fr), self.to_error_region(outlived_fr)) { + let fr_region = self.to_error_region(fr); + let outlived_fr_region = self.to_error_region(outlived_fr); + if let (Some(f), Some(o)) = (fr_region, outlived_fr_region) { let tables = infcx.tcx.typeck_tables_of(mir_def_id); let nice = NiceRegionError::new_from_span(infcx.tcx, span, o, f, Some(tables)); if let Some(_error_reported) = nice.try_report_from_nll() { @@ -356,17 +358,18 @@ impl<'tcx> RegionInferenceContext<'tcx> { self.universal_regions.is_local_free_region(fr), self.universal_regions.is_local_free_region(outlived_fr), ); - debug!("report_error: fr_is_local={:?} outlived_fr_is_local={:?} category={:?}", - fr_is_local, outlived_fr_is_local, category); + debug!("report_error: fr_is_local={:?} outlived_fr_is_local={:?} fr_region={:?} \ + outlived_fr_region={:?} category={:?}", + fr_is_local, outlived_fr_is_local, fr_region, outlived_fr_region, category); match (category, fr_is_local, outlived_fr_is_local) { (ConstraintCategory::Assignment, true, false) | (ConstraintCategory::CallArgument, true, false) => - self.report_escaping_data_error(mir, infcx, mir_def_id, fr, outlived_fr, - category, span, errors_buffer), + self.report_escaping_data_error(mir, infcx, mir_def_id, fr, fr_region, outlived_fr, + outlived_fr_region, category, span, errors_buffer), _ => - self.report_general_error(mir, infcx, mir_def_id, fr, fr_is_local, - outlived_fr, outlived_fr_is_local, + self.report_general_error(mir, infcx, mir_def_id, fr, fr_is_local, fr_region, + outlived_fr, outlived_fr_is_local, outlived_fr_region, category, span, errors_buffer), }; } @@ -377,7 +380,9 @@ impl<'tcx> RegionInferenceContext<'tcx> { infcx: &InferCtxt<'_, '_, 'tcx>, mir_def_id: DefId, fr: RegionVid, + fr_region: Option>, outlived_fr: RegionVid, + outlived_fr_region: Option>, category: ConstraintCategory, span: Span, errors_buffer: &mut Vec, @@ -390,7 +395,8 @@ impl<'tcx> RegionInferenceContext<'tcx> { if fr_name_and_span.is_none() && outlived_fr_name_and_span.is_none() { return self.report_general_error(mir, infcx, mir_def_id, - fr, true, outlived_fr, false, + fr, true, fr_region, + outlived_fr, false, outlived_fr_region, category, span, errors_buffer); } @@ -430,8 +436,10 @@ impl<'tcx> RegionInferenceContext<'tcx> { mir_def_id: DefId, fr: RegionVid, fr_is_local: bool, + fr_region: Option>, outlived_fr: RegionVid, outlived_fr_is_local: bool, + outlived_fr_region: Option>, category: ConstraintCategory, span: Span, errors_buffer: &mut Vec, @@ -465,6 +473,26 @@ impl<'tcx> RegionInferenceContext<'tcx> { }, } + if let (Some(f), Some(RegionKind::ReStatic)) = (fr_region, outlived_fr_region) { + if let Some(TyS { + sty: TyKind::Anon(did, _), + .. + }) = self.return_type_impl_trait(infcx, f) { + let span = infcx.tcx.def_span(*did); + if let Ok(snippet) = infcx.tcx.sess.source_map().span_to_snippet(span) { + diag.span_suggestion( + span, + &format!( + "you can add a constraint to the return type to make it last \ + less than `'static` and match {}", + fr_name, + ), + format!("{} + {}", snippet, fr_name), + ); + } + } + } + diag.buffer(errors_buffer); } @@ -490,4 +518,15 @@ impl<'tcx> RegionInferenceContext<'tcx> { let (_, span, _) = self.best_blame_constraint(mir, tcx, fr1, |r| r == fr2); span } + + fn return_type_impl_trait<'cx>( + &self, + infcx: &'cx InferCtxt<'_, '_, 'tcx>, + outlived_fr_region: Region<'tcx>, + ) -> Option> { + infcx.tcx.is_suitable_region(outlived_fr_region) + .map(|r| r.def_id) + .map(|id| infcx.tcx.return_type_impl_trait(id)) + .unwrap_or(None) + } } diff --git a/src/test/ui/impl-trait/must_outlive_least_region_or_bound.nll.stderr b/src/test/ui/impl-trait/must_outlive_least_region_or_bound.nll.stderr index 1e3e997b111ac..e372bd33aa2d5 100644 --- a/src/test/ui/impl-trait/must_outlive_least_region_or_bound.nll.stderr +++ b/src/test/ui/impl-trait/must_outlive_least_region_or_bound.nll.stderr @@ -11,12 +11,20 @@ error: unsatisfied lifetime constraints | LL | fn explicit<'a>(x: &'a i32) -> impl Copy { x } | -- lifetime `'a` defined here ^ returning this value requires that `'a` must outlive `'static` +help: you can add a constraint to the return type to make it last less than `'static` and match 'a + | +LL | fn explicit<'a>(x: &'a i32) -> impl Copy + 'a { x } + | ^^^^^^^^^^^^^^ error: unsatisfied lifetime constraints --> $DIR/must_outlive_least_region_or_bound.rs:22:69 | LL | fn with_bound<'a>(x: &'a i32) -> impl LifetimeTrait<'a> + 'static { x } | -- lifetime `'a` defined here ^ returning this value requires that `'a` must outlive `'static` +help: you can add a constraint to the return type to make it last less than `'static` and match 'a + | +LL | fn with_bound<'a>(x: &'a i32) -> impl LifetimeTrait<'a> + 'static + 'a { x } + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: unsatisfied lifetime constraints --> $DIR/must_outlive_least_region_or_bound.rs:29:5 diff --git a/src/test/ui/impl-trait/static-return-lifetime-infered.nll.stderr b/src/test/ui/impl-trait/static-return-lifetime-infered.nll.stderr index d75de81fc1c7f..d6b19acb86b26 100644 --- a/src/test/ui/impl-trait/static-return-lifetime-infered.nll.stderr +++ b/src/test/ui/impl-trait/static-return-lifetime-infered.nll.stderr @@ -5,6 +5,10 @@ LL | fn iter_values_anon(&self) -> impl Iterator { | - let's call the lifetime of this reference `'1` LL | self.x.iter().map(|a| a.0) | ^^^^^^ cast requires that `'1` must outlive `'static` +help: you can add a constraint to the return type to make it last less than `'static` and match '1 + | +LL | fn iter_values_anon(&self) -> impl Iterator + '1 { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: unsatisfied lifetime constraints --> $DIR/static-return-lifetime-infered.rs:21:9 @@ -13,6 +17,10 @@ LL | fn iter_values<'a>(&'a self) -> impl Iterator { | -- lifetime `'a` defined here LL | self.x.iter().map(|a| a.0) | ^^^^^^ cast requires that `'a` must outlive `'static` +help: you can add a constraint to the return type to make it last less than `'static` and match 'a + | +LL | fn iter_values<'a>(&'a self) -> impl Iterator + 'a { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: aborting due to 2 previous errors From 65e253966693a1c222badf977dc133df028f2351 Mon Sep 17 00:00:00 2001 From: David Wood Date: Mon, 3 Sep 2018 22:50:56 +0200 Subject: [PATCH 3/6] Don't suggest adding a synthesized region name. --- .../nll/region_infer/error_reporting/mod.rs | 32 ++++++++-- .../error_reporting/region_name.rs | 61 +++++++++++++------ ...t_outlive_least_region_or_bound.nll.stderr | 4 +- 3 files changed, 72 insertions(+), 25 deletions(-) diff --git a/src/librustc_mir/borrow_check/nll/region_infer/error_reporting/mod.rs b/src/librustc_mir/borrow_check/nll/region_infer/error_reporting/mod.rs index 2d8f782f748ec..bf3b5e2a17c6e 100644 --- a/src/librustc_mir/borrow_check/nll/region_infer/error_reporting/mod.rs +++ b/src/librustc_mir/borrow_check/nll/region_infer/error_reporting/mod.rs @@ -17,14 +17,17 @@ use rustc::infer::InferCtxt; use rustc::mir::{self, Location, Mir, Place, Rvalue, StatementKind, TerminatorKind}; use rustc::ty::{TyCtxt, Ty, TyS, TyKind, Region, RegionKind, RegionVid}; use rustc_data_structures::indexed_vec::IndexVec; -use rustc_errors::Diagnostic; +use rustc_errors::{Diagnostic, DiagnosticBuilder}; use std::collections::VecDeque; use std::fmt; +use syntax::symbol::keywords; use syntax_pos::Span; mod region_name; mod var_name; +use self::region_name::RegionName; + /// Constraints that are considered interesting can be categorized to /// determine why they are interesting. Order of variants indicates /// sort order of the category, thereby influencing diagnostic output. @@ -473,27 +476,44 @@ impl<'tcx> RegionInferenceContext<'tcx> { }, } + self.add_static_impl_trait_suggestion( + infcx, &mut diag, fr_name, fr_region, outlived_fr_region + ); + + diag.buffer(errors_buffer); + } + + fn add_static_impl_trait_suggestion( + &self, + infcx: &InferCtxt<'_, '_, 'tcx>, + diag: &mut DiagnosticBuilder<'_>, + fr_name: RegionName, + fr_region: Option>, + outlived_fr_region: Option>, + ) { if let (Some(f), Some(RegionKind::ReStatic)) = (fr_region, outlived_fr_region) { if let Some(TyS { sty: TyKind::Anon(did, _), .. }) = self.return_type_impl_trait(infcx, f) { + let static_str = keywords::StaticLifetime.name(); let span = infcx.tcx.def_span(*did); if let Ok(snippet) = infcx.tcx.sess.source_map().span_to_snippet(span) { diag.span_suggestion( span, &format!( "you can add a constraint to the return type to make it last \ - less than `'static` and match {}", - fr_name, + less than `{}` and match `{}`", + static_str, fr_name, ), - format!("{} + {}", snippet, fr_name), + match fr_name { + RegionName::Named(name) => format!("{} + {}", snippet, name), + RegionName::Synthesized(_) => format!("{} + '_", snippet), + }, ); } } } - - diag.buffer(errors_buffer); } // Finds some region R such that `fr1: R` and `R` is live at diff --git a/src/librustc_mir/borrow_check/nll/region_infer/error_reporting/region_name.rs b/src/librustc_mir/borrow_check/nll/region_infer/error_reporting/region_name.rs index 5ae123bdc18d1..136f660fe8daa 100644 --- a/src/librustc_mir/borrow_check/nll/region_infer/error_reporting/region_name.rs +++ b/src/librustc_mir/borrow_check/nll/region_infer/error_reporting/region_name.rs @@ -8,6 +8,7 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +use std::fmt::{self, Display}; use borrow_check::nll::region_infer::RegionInferenceContext; use borrow_check::nll::universal_regions::DefiningTy; use borrow_check::nll::ToRegionVid; @@ -23,6 +24,24 @@ use syntax::ast::{Name, DUMMY_NODE_ID}; use syntax::symbol::keywords; use syntax_pos::symbol::InternedString; +/// Name of a region used in error reporting. Variants denote the source of the region name - +/// whether it was synthesized for the error message and therefore should not be used in +/// suggestions; or whether it was found from the region. +#[derive(Debug)] +pub(crate) enum RegionName { + Named(InternedString), + Synthesized(InternedString), +} + +impl Display for RegionName { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match self { + RegionName::Named(name) | RegionName::Synthesized(name) => + write!(f, "{}", name), + } + } +} + impl<'tcx> RegionInferenceContext<'tcx> { /// Maps from an internal MIR region vid to something that we can /// report to the user. In some cases, the region vids will map @@ -57,7 +76,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { fr: RegionVid, counter: &mut usize, diag: &mut DiagnosticBuilder, - ) -> InternedString { + ) -> RegionName { debug!("give_region_a_name(fr={:?}, counter={})", fr, counter); assert!(self.universal_regions.is_universal_region(fr)); @@ -95,7 +114,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { fr: RegionVid, counter: &mut usize, diag: &mut DiagnosticBuilder<'_>, - ) -> Option { + ) -> Option { let error_region = self.to_error_region(fr)?; debug!("give_region_a_name: error_region = {:?}", error_region); @@ -103,19 +122,21 @@ impl<'tcx> RegionInferenceContext<'tcx> { ty::ReEarlyBound(ebr) => { if ebr.has_name() { self.highlight_named_span(tcx, error_region, &ebr.name, diag); - Some(ebr.name) + Some(RegionName::Named(ebr.name)) } else { None } } - ty::ReStatic => Some(keywords::StaticLifetime.name().as_interned_str()), + ty::ReStatic => Some(RegionName::Named( + keywords::StaticLifetime.name().as_interned_str() + )), ty::ReFree(free_region) => match free_region.bound_region { ty::BoundRegion::BrNamed(_, name) => { self.highlight_named_span(tcx, error_region, &name, diag); - Some(name) - } + Some(RegionName::Named(name)) + }, ty::BoundRegion::BrEnv => { let mir_node_id = tcx.hir.as_local_node_id(mir_def_id).expect("non-local mir"); @@ -132,7 +153,10 @@ impl<'tcx> RegionInferenceContext<'tcx> { let region_name = self.synthesize_region_name(counter); diag.span_label( args_span, - format!("lifetime `{}` represents this closure's body", region_name), + format!( + "lifetime `{}` represents this closure's body", + region_name + ), ); let closure_kind_ty = substs.closure_kind_ty(def_id, tcx); @@ -227,7 +251,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { fr: RegionVid, counter: &mut usize, diag: &mut DiagnosticBuilder<'_>, - ) -> Option { + ) -> Option { let implicit_inputs = self.universal_regions.defining_ty.implicit_inputs(); let argument_index = self.get_argument_index_for_region(infcx.tcx, fr)?; @@ -259,7 +283,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { argument_index: usize, counter: &mut usize, diag: &mut DiagnosticBuilder<'_>, - ) -> Option { + ) -> Option { let mir_node_id = infcx.tcx.hir.as_local_node_id(mir_def_id)?; let fn_decl = infcx.tcx.hir.fn_decl(mir_node_id)?; let argument_hir_ty: &hir::Ty = &fn_decl.inputs[argument_index]; @@ -306,7 +330,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { argument_ty: Ty<'tcx>, counter: &mut usize, diag: &mut DiagnosticBuilder<'_>, - ) -> Option { + ) -> Option { let type_name = with_highlight_region(needle_fr, *counter, || { infcx.extract_type_name(&argument_ty) }); @@ -361,7 +385,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { argument_hir_ty: &hir::Ty, counter: &mut usize, diag: &mut DiagnosticBuilder<'_>, - ) -> Option { + ) -> Option { let search_stack: &mut Vec<(Ty<'tcx>, &hir::Ty)> = &mut Vec::new(); search_stack.push((argument_ty, argument_hir_ty)); @@ -457,7 +481,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { counter: &mut usize, diag: &mut DiagnosticBuilder<'_>, search_stack: &mut Vec<(Ty<'tcx>, &'hir hir::Ty)>, - ) -> Option { + ) -> Option { // Did the user give explicit arguments? (e.g., `Foo<..>`) let args = last_segment.args.as_ref()?; let lifetime = self.try_match_adt_and_generic_args(substs, needle_fr, args, search_stack)?; @@ -467,7 +491,10 @@ impl<'tcx> RegionInferenceContext<'tcx> { | hir::LifetimeName::Underscore => { let region_name = self.synthesize_region_name(counter); let ampersand_span = lifetime.span; - diag.span_label(ampersand_span, format!("let's call this `{}`", region_name)); + diag.span_label( + ampersand_span, + format!("let's call this `{}`", region_name) + ); return Some(region_name); } @@ -544,7 +571,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { fr: RegionVid, counter: &mut usize, diag: &mut DiagnosticBuilder<'_>, - ) -> Option { + ) -> Option { let upvar_index = self.get_upvar_index_for_region(tcx, fr)?; let (upvar_name, upvar_span) = self.get_upvar_name_and_span_for_region(tcx, mir, upvar_index); @@ -573,7 +600,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { fr: RegionVid, counter: &mut usize, diag: &mut DiagnosticBuilder<'_>, - ) -> Option { + ) -> Option { let tcx = infcx.tcx; let return_ty = self.universal_regions.unnormalized_output_ty; @@ -622,10 +649,10 @@ impl<'tcx> RegionInferenceContext<'tcx> { /// Create a synthetic region named `'1`, incrementing the /// counter. - fn synthesize_region_name(&self, counter: &mut usize) -> InternedString { + fn synthesize_region_name(&self, counter: &mut usize) -> RegionName { let c = *counter; *counter += 1; - Name::intern(&format!("'{:?}", c)).as_interned_str() + RegionName::Synthesized(Name::intern(&format!("'{:?}", c)).as_interned_str()) } } diff --git a/src/test/ui/impl-trait/must_outlive_least_region_or_bound.nll.stderr b/src/test/ui/impl-trait/must_outlive_least_region_or_bound.nll.stderr index e372bd33aa2d5..e95fc7663710e 100644 --- a/src/test/ui/impl-trait/must_outlive_least_region_or_bound.nll.stderr +++ b/src/test/ui/impl-trait/must_outlive_least_region_or_bound.nll.stderr @@ -11,7 +11,7 @@ error: unsatisfied lifetime constraints | LL | fn explicit<'a>(x: &'a i32) -> impl Copy { x } | -- lifetime `'a` defined here ^ returning this value requires that `'a` must outlive `'static` -help: you can add a constraint to the return type to make it last less than `'static` and match 'a +help: you can add a constraint to the return type to make it last less than `'static` and match `'a` | LL | fn explicit<'a>(x: &'a i32) -> impl Copy + 'a { x } | ^^^^^^^^^^^^^^ @@ -21,7 +21,7 @@ error: unsatisfied lifetime constraints | LL | fn with_bound<'a>(x: &'a i32) -> impl LifetimeTrait<'a> + 'static { x } | -- lifetime `'a` defined here ^ returning this value requires that `'a` must outlive `'static` -help: you can add a constraint to the return type to make it last less than `'static` and match 'a +help: you can add a constraint to the return type to make it last less than `'static` and match `'a` | LL | fn with_bound<'a>(x: &'a i32) -> impl LifetimeTrait<'a> + 'static + 'a { x } | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ From 7a89e93519ce0880fe10c37c2ca8f63686fc210b Mon Sep 17 00:00:00 2001 From: David Wood Date: Tue, 4 Sep 2018 18:56:14 +0200 Subject: [PATCH 4/6] Emit appropriate suggestion when there's already 'static bound on the return type. --- src/librustc/ty/context.rs | 2 +- .../nll/region_infer/error_reporting/mod.rs | 90 ++++++++++++------- .../error_reporting/region_name.rs | 57 ++++++++---- ...t_outlive_least_region_or_bound.nll.stderr | 6 +- 4 files changed, 104 insertions(+), 51 deletions(-) diff --git a/src/librustc/ty/context.rs b/src/librustc/ty/context.rs index ae840f93c20e4..03c99180d5e50 100644 --- a/src/librustc/ty/context.rs +++ b/src/librustc/ty/context.rs @@ -1590,7 +1590,7 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> { pub fn return_type_impl_trait( &self, scope_def_id: DefId, - ) -> Option { + ) -> Option> { let ret_ty = self.type_of(scope_def_id); match ret_ty.sty { ty::FnDef(_, _) => { diff --git a/src/librustc_mir/borrow_check/nll/region_infer/error_reporting/mod.rs b/src/librustc_mir/borrow_check/nll/region_infer/error_reporting/mod.rs index bf3b5e2a17c6e..8690212804e21 100644 --- a/src/librustc_mir/borrow_check/nll/region_infer/error_reporting/mod.rs +++ b/src/librustc_mir/borrow_check/nll/region_infer/error_reporting/mod.rs @@ -15,7 +15,7 @@ use rustc::hir::def_id::DefId; use rustc::infer::error_reporting::nice_region_error::NiceRegionError; use rustc::infer::InferCtxt; use rustc::mir::{self, Location, Mir, Place, Rvalue, StatementKind, TerminatorKind}; -use rustc::ty::{TyCtxt, Ty, TyS, TyKind, Region, RegionKind, RegionVid}; +use rustc::ty::{self, TyCtxt, Region, RegionKind, RegionVid}; use rustc_data_structures::indexed_vec::IndexVec; use rustc_errors::{Diagnostic, DiagnosticBuilder}; use std::collections::VecDeque; @@ -491,26 +491,67 @@ impl<'tcx> RegionInferenceContext<'tcx> { fr_region: Option>, outlived_fr_region: Option>, ) { - if let (Some(f), Some(RegionKind::ReStatic)) = (fr_region, outlived_fr_region) { - if let Some(TyS { - sty: TyKind::Anon(did, _), + if let (Some(f), Some(ty::RegionKind::ReStatic)) = (fr_region, outlived_fr_region) { + if let Some(ty::TyS { + sty: ty::TyKind::Anon(did, substs), .. - }) = self.return_type_impl_trait(infcx, f) { + }) = infcx.tcx.is_suitable_region(f) + .map(|r| r.def_id) + .map(|id| infcx.tcx.return_type_impl_trait(id)) + .unwrap_or(None) + { + let has_static_predicate = { + let predicates_of = infcx.tcx.predicates_of(*did); + let bounds = predicates_of.instantiate(infcx.tcx, substs); + + let mut found = false; + for predicate in bounds.predicates { + if let ty::Predicate::TypeOutlives(binder) = predicate { + if let ty::OutlivesPredicate( + _, + RegionKind::ReStatic + ) = binder.skip_binder() { + found = true; + break; + } + } + } + + found + }; + + debug!("add_static_impl_trait_suggestion: has_static_predicate={:?}", + has_static_predicate); let static_str = keywords::StaticLifetime.name(); - let span = infcx.tcx.def_span(*did); - if let Ok(snippet) = infcx.tcx.sess.source_map().span_to_snippet(span) { - diag.span_suggestion( - span, - &format!( - "you can add a constraint to the return type to make it last \ - less than `{}` and match `{}`", - static_str, fr_name, - ), - match fr_name { - RegionName::Named(name) => format!("{} + {}", snippet, name), - RegionName::Synthesized(_) => format!("{} + '_", snippet), - }, - ); + if has_static_predicate { + let span = self.get_span_of_named_region(infcx.tcx, f, &fr_name); + if let Ok(snippet) = infcx.tcx.sess.source_map().span_to_snippet(span) { + diag.span_suggestion( + span, + &format!( + "you can add a constraint to the definition of `{}` to require it \ + outlive `{}`", + fr_name, static_str, + ), + format!("{}: {}", snippet, static_str), + ); + } + } else { + let span = infcx.tcx.def_span(*did); + if let Ok(snippet) = infcx.tcx.sess.source_map().span_to_snippet(span) { + diag.span_suggestion( + span, + &format!( + "you can add a constraint to the return type to make it last \ + less than `{}` and match `{}`", + static_str, fr_name, + ), + match fr_name { + RegionName::Named(name) => format!("{} + {}", snippet, name), + RegionName::Synthesized(_) => format!("{} + '_", snippet), + }, + ); + } } } } @@ -538,15 +579,4 @@ impl<'tcx> RegionInferenceContext<'tcx> { let (_, span, _) = self.best_blame_constraint(mir, tcx, fr1, |r| r == fr2); span } - - fn return_type_impl_trait<'cx>( - &self, - infcx: &'cx InferCtxt<'_, '_, 'tcx>, - outlived_fr_region: Region<'tcx>, - ) -> Option> { - infcx.tcx.is_suitable_region(outlived_fr_region) - .map(|r| r.def_id) - .map(|id| infcx.tcx.return_type_impl_trait(id)) - .unwrap_or(None) - } } diff --git a/src/librustc_mir/borrow_check/nll/region_infer/error_reporting/region_name.rs b/src/librustc_mir/borrow_check/nll/region_infer/error_reporting/region_name.rs index 136f660fe8daa..857f1dae7ab51 100644 --- a/src/librustc_mir/borrow_check/nll/region_infer/error_reporting/region_name.rs +++ b/src/librustc_mir/borrow_check/nll/region_infer/error_reporting/region_name.rs @@ -22,6 +22,7 @@ use rustc::util::ppaux::with_highlight_region; use rustc_errors::DiagnosticBuilder; use syntax::ast::{Name, DUMMY_NODE_ID}; use syntax::symbol::keywords; +use syntax_pos::Span; use syntax_pos::symbol::InternedString; /// Name of a region used in error reporting. Variants denote the source of the region name - @@ -33,6 +34,14 @@ pub(crate) enum RegionName { Synthesized(InternedString), } +impl RegionName { + fn as_interned_string(&self) -> &InternedString { + match self { + RegionName::Named(name) | RegionName::Synthesized(name) => name, + } + } +} + impl Display for RegionName { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match self { @@ -121,8 +130,9 @@ impl<'tcx> RegionInferenceContext<'tcx> { match error_region { ty::ReEarlyBound(ebr) => { if ebr.has_name() { - self.highlight_named_span(tcx, error_region, &ebr.name, diag); - Some(RegionName::Named(ebr.name)) + let name = RegionName::Named(ebr.name); + self.highlight_named_span(tcx, error_region, &name, diag); + Some(name) } else { None } @@ -134,8 +144,9 @@ impl<'tcx> RegionInferenceContext<'tcx> { ty::ReFree(free_region) => match free_region.bound_region { ty::BoundRegion::BrNamed(_, name) => { + let name = RegionName::Named(name); self.highlight_named_span(tcx, error_region, &name, diag); - Some(RegionName::Named(name)) + Some(name) }, ty::BoundRegion::BrEnv => { @@ -198,6 +209,26 @@ impl<'tcx> RegionInferenceContext<'tcx> { } } + /// Get the span of a named region. + pub(super) fn get_span_of_named_region( + &self, + tcx: TyCtxt<'_, '_, 'tcx>, + error_region: &RegionKind, + name: &RegionName, + ) -> Span { + let scope = error_region.free_region_binding_scope(tcx); + let node = tcx.hir.as_local_node_id(scope).unwrap_or(DUMMY_NODE_ID); + + let span = tcx.sess.source_map().def_span(tcx.hir.span(node)); + if let Some(param) = tcx.hir.get_generics(scope).and_then(|generics| { + generics.get_named(name.as_interned_string()) + }) { + param.span + } else { + span + } + } + /// Highlight a named span to provide context for error messages that /// mention that span, for example: /// @@ -216,23 +247,15 @@ impl<'tcx> RegionInferenceContext<'tcx> { &self, tcx: TyCtxt<'_, '_, 'tcx>, error_region: &RegionKind, - name: &InternedString, + name: &RegionName, diag: &mut DiagnosticBuilder<'_>, ) { - let cm = tcx.sess.source_map(); - - let scope = error_region.free_region_binding_scope(tcx); - let node = tcx.hir.as_local_node_id(scope).unwrap_or(DUMMY_NODE_ID); - - let mut sp = cm.def_span(tcx.hir.span(node)); - if let Some(param) = tcx.hir - .get_generics(scope) - .and_then(|generics| generics.get_named(name)) - { - sp = param.span; - } + let span = self.get_span_of_named_region(tcx, error_region, name); - diag.span_label(sp, format!("lifetime `{}` defined here", name)); + diag.span_label( + span, + format!("lifetime `{}` defined here", name), + ); } /// Find an argument that contains `fr` and label it with a fully diff --git a/src/test/ui/impl-trait/must_outlive_least_region_or_bound.nll.stderr b/src/test/ui/impl-trait/must_outlive_least_region_or_bound.nll.stderr index e95fc7663710e..1e527140127a5 100644 --- a/src/test/ui/impl-trait/must_outlive_least_region_or_bound.nll.stderr +++ b/src/test/ui/impl-trait/must_outlive_least_region_or_bound.nll.stderr @@ -21,10 +21,10 @@ error: unsatisfied lifetime constraints | LL | fn with_bound<'a>(x: &'a i32) -> impl LifetimeTrait<'a> + 'static { x } | -- lifetime `'a` defined here ^ returning this value requires that `'a` must outlive `'static` -help: you can add a constraint to the return type to make it last less than `'static` and match `'a` +help: you can add a constraint to the definition of `'a` to require it outlive `'static` | -LL | fn with_bound<'a>(x: &'a i32) -> impl LifetimeTrait<'a> + 'static + 'a { x } - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | fn with_bound<'a: 'static>(x: &'a i32) -> impl LifetimeTrait<'a> + 'static { x } + | ^^^^^^^^^^^ error: unsatisfied lifetime constraints --> $DIR/must_outlive_least_region_or_bound.rs:29:5 From 37e18b1c5131b14e4e74fc3194e34608ab4dee04 Mon Sep 17 00:00:00 2001 From: David Wood Date: Wed, 5 Sep 2018 23:26:50 +0200 Subject: [PATCH 5/6] Updated suggestion/help messages. --- .../nll/region_infer/error_reporting/mod.rs | 80 +++++++++---------- ...t_outlive_least_region_or_bound.nll.stderr | 6 +- .../static-return-lifetime-infered.nll.stderr | 6 +- 3 files changed, 44 insertions(+), 48 deletions(-) diff --git a/src/librustc_mir/borrow_check/nll/region_infer/error_reporting/mod.rs b/src/librustc_mir/borrow_check/nll/region_infer/error_reporting/mod.rs index 8690212804e21..4db9507ac77d0 100644 --- a/src/librustc_mir/borrow_check/nll/region_infer/error_reporting/mod.rs +++ b/src/librustc_mir/borrow_check/nll/region_infer/error_reporting/mod.rs @@ -15,7 +15,7 @@ use rustc::hir::def_id::DefId; use rustc::infer::error_reporting::nice_region_error::NiceRegionError; use rustc::infer::InferCtxt; use rustc::mir::{self, Location, Mir, Place, Rvalue, StatementKind, TerminatorKind}; -use rustc::ty::{self, TyCtxt, Region, RegionKind, RegionVid}; +use rustc::ty::{self, TyCtxt, RegionVid}; use rustc_data_structures::indexed_vec::IndexVec; use rustc_errors::{Diagnostic, DiagnosticBuilder}; use std::collections::VecDeque; @@ -347,9 +347,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { ); // Check if we can use one of the "nice region errors". - let fr_region = self.to_error_region(fr); - let outlived_fr_region = self.to_error_region(outlived_fr); - if let (Some(f), Some(o)) = (fr_region, outlived_fr_region) { + if let (Some(f), Some(o)) = (self.to_error_region(fr), self.to_error_region(outlived_fr)) { let tables = infcx.tcx.typeck_tables_of(mir_def_id); let nice = NiceRegionError::new_from_span(infcx.tcx, span, o, f, Some(tables)); if let Some(_error_reported) = nice.try_report_from_nll() { @@ -362,17 +360,16 @@ impl<'tcx> RegionInferenceContext<'tcx> { self.universal_regions.is_local_free_region(outlived_fr), ); - debug!("report_error: fr_is_local={:?} outlived_fr_is_local={:?} fr_region={:?} \ - outlived_fr_region={:?} category={:?}", - fr_is_local, outlived_fr_is_local, fr_region, outlived_fr_region, category); + debug!("report_error: fr_is_local={:?} outlived_fr_is_local={:?} category={:?}", + fr_is_local, outlived_fr_is_local, category); match (category, fr_is_local, outlived_fr_is_local) { (ConstraintCategory::Assignment, true, false) | (ConstraintCategory::CallArgument, true, false) => - self.report_escaping_data_error(mir, infcx, mir_def_id, fr, fr_region, outlived_fr, - outlived_fr_region, category, span, errors_buffer), + self.report_escaping_data_error(mir, infcx, mir_def_id, fr, outlived_fr, + category, span, errors_buffer), _ => - self.report_general_error(mir, infcx, mir_def_id, fr, fr_is_local, fr_region, - outlived_fr, outlived_fr_is_local, outlived_fr_region, + self.report_general_error(mir, infcx, mir_def_id, fr, fr_is_local, + outlived_fr, outlived_fr_is_local, category, span, errors_buffer), }; } @@ -383,9 +380,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { infcx: &InferCtxt<'_, '_, 'tcx>, mir_def_id: DefId, fr: RegionVid, - fr_region: Option>, outlived_fr: RegionVid, - outlived_fr_region: Option>, category: ConstraintCategory, span: Span, errors_buffer: &mut Vec, @@ -398,8 +393,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { if fr_name_and_span.is_none() && outlived_fr_name_and_span.is_none() { return self.report_general_error(mir, infcx, mir_def_id, - fr, true, fr_region, - outlived_fr, false, outlived_fr_region, + fr, true, outlived_fr, false, category, span, errors_buffer); } @@ -439,10 +433,8 @@ impl<'tcx> RegionInferenceContext<'tcx> { mir_def_id: DefId, fr: RegionVid, fr_is_local: bool, - fr_region: Option>, outlived_fr: RegionVid, outlived_fr_is_local: bool, - outlived_fr_region: Option>, category: ConstraintCategory, span: Span, errors_buffer: &mut Vec, @@ -477,7 +469,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { } self.add_static_impl_trait_suggestion( - infcx, &mut diag, fr_name, fr_region, outlived_fr_region + infcx, &mut diag, fr, fr_name, outlived_fr, ); diag.buffer(errors_buffer); @@ -487,11 +479,15 @@ impl<'tcx> RegionInferenceContext<'tcx> { &self, infcx: &InferCtxt<'_, '_, 'tcx>, diag: &mut DiagnosticBuilder<'_>, + fr: RegionVid, + // We need to pass `fr_name` - computing it again will label it twice. fr_name: RegionName, - fr_region: Option>, - outlived_fr_region: Option>, + outlived_fr: RegionVid, ) { - if let (Some(f), Some(ty::RegionKind::ReStatic)) = (fr_region, outlived_fr_region) { + if let ( + Some(f), + Some(ty::RegionKind::ReStatic) + ) = (self.to_error_region(fr), self.to_error_region(outlived_fr)) { if let Some(ty::TyS { sty: ty::TyKind::Anon(did, substs), .. @@ -500,6 +496,10 @@ impl<'tcx> RegionInferenceContext<'tcx> { .map(|id| infcx.tcx.return_type_impl_trait(id)) .unwrap_or(None) { + // Check whether or not the impl trait return type is intended to capture + // data with the static lifetime. + // + // eg. check for `impl Trait + 'static` instead of `impl Trait`. let has_static_predicate = { let predicates_of = infcx.tcx.predicates_of(*did); let bounds = predicates_of.instantiate(infcx.tcx, substs); @@ -509,7 +509,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { if let ty::Predicate::TypeOutlives(binder) = predicate { if let ty::OutlivesPredicate( _, - RegionKind::ReStatic + ty::RegionKind::ReStatic ) = binder.skip_binder() { found = true; break; @@ -523,33 +523,31 @@ impl<'tcx> RegionInferenceContext<'tcx> { debug!("add_static_impl_trait_suggestion: has_static_predicate={:?}", has_static_predicate); let static_str = keywords::StaticLifetime.name(); + // If there is a static predicate, then the only sensible suggestion is to replace + // fr with `'static`. if has_static_predicate { - let span = self.get_span_of_named_region(infcx.tcx, f, &fr_name); - if let Ok(snippet) = infcx.tcx.sess.source_map().span_to_snippet(span) { - diag.span_suggestion( - span, - &format!( - "you can add a constraint to the definition of `{}` to require it \ - outlive `{}`", - fr_name, static_str, - ), - format!("{}: {}", snippet, static_str), - ); - } + diag.help( + &format!( + "consider replacing `{}` with `{}`", + fr_name, static_str, + ), + ); } else { + // Otherwise, we should suggest adding a constraint on the return type. let span = infcx.tcx.def_span(*did); if let Ok(snippet) = infcx.tcx.sess.source_map().span_to_snippet(span) { + let suggestable_fr_name = match fr_name { + RegionName::Named(name) => format!("{}", name), + RegionName::Synthesized(_) => "'_".to_string(), + }; diag.span_suggestion( span, &format!( - "you can add a constraint to the return type to make it last \ - less than `{}` and match `{}`", - static_str, fr_name, + "to allow this impl Trait to capture borrowed data with lifetime \ + `{}`, add `{}` as a constraint", + fr_name, suggestable_fr_name, ), - match fr_name { - RegionName::Named(name) => format!("{} + {}", snippet, name), - RegionName::Synthesized(_) => format!("{} + '_", snippet), - }, + format!("{} + {}", snippet, suggestable_fr_name), ); } } diff --git a/src/test/ui/impl-trait/must_outlive_least_region_or_bound.nll.stderr b/src/test/ui/impl-trait/must_outlive_least_region_or_bound.nll.stderr index 1e527140127a5..3019e3aa1f732 100644 --- a/src/test/ui/impl-trait/must_outlive_least_region_or_bound.nll.stderr +++ b/src/test/ui/impl-trait/must_outlive_least_region_or_bound.nll.stderr @@ -11,7 +11,7 @@ error: unsatisfied lifetime constraints | LL | fn explicit<'a>(x: &'a i32) -> impl Copy { x } | -- lifetime `'a` defined here ^ returning this value requires that `'a` must outlive `'static` -help: you can add a constraint to the return type to make it last less than `'static` and match `'a` +help: to allow this impl Trait to capture borrowed data with lifetime `'a`, add `'a` as a constraint | LL | fn explicit<'a>(x: &'a i32) -> impl Copy + 'a { x } | ^^^^^^^^^^^^^^ @@ -21,10 +21,8 @@ error: unsatisfied lifetime constraints | LL | fn with_bound<'a>(x: &'a i32) -> impl LifetimeTrait<'a> + 'static { x } | -- lifetime `'a` defined here ^ returning this value requires that `'a` must outlive `'static` -help: you can add a constraint to the definition of `'a` to require it outlive `'static` | -LL | fn with_bound<'a: 'static>(x: &'a i32) -> impl LifetimeTrait<'a> + 'static { x } - | ^^^^^^^^^^^ + = help: consider replacing `'a` with `'static` error: unsatisfied lifetime constraints --> $DIR/must_outlive_least_region_or_bound.rs:29:5 diff --git a/src/test/ui/impl-trait/static-return-lifetime-infered.nll.stderr b/src/test/ui/impl-trait/static-return-lifetime-infered.nll.stderr index d6b19acb86b26..4bc8876c232e3 100644 --- a/src/test/ui/impl-trait/static-return-lifetime-infered.nll.stderr +++ b/src/test/ui/impl-trait/static-return-lifetime-infered.nll.stderr @@ -5,9 +5,9 @@ LL | fn iter_values_anon(&self) -> impl Iterator { | - let's call the lifetime of this reference `'1` LL | self.x.iter().map(|a| a.0) | ^^^^^^ cast requires that `'1` must outlive `'static` -help: you can add a constraint to the return type to make it last less than `'static` and match '1 +help: to allow this impl Trait to capture borrowed data with lifetime `'1`, add `'_` as a constraint | -LL | fn iter_values_anon(&self) -> impl Iterator + '1 { +LL | fn iter_values_anon(&self) -> impl Iterator + '_ { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: unsatisfied lifetime constraints @@ -17,7 +17,7 @@ LL | fn iter_values<'a>(&'a self) -> impl Iterator { | -- lifetime `'a` defined here LL | self.x.iter().map(|a| a.0) | ^^^^^^ cast requires that `'a` must outlive `'static` -help: you can add a constraint to the return type to make it last less than `'static` and match 'a +help: to allow this impl Trait to capture borrowed data with lifetime `'a`, add `'a` as a constraint | LL | fn iter_values<'a>(&'a self) -> impl Iterator + 'a { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ From 18c1374bf86bf061a7f5f73fcebea5abc2a80922 Mon Sep 17 00:00:00 2001 From: David Wood Date: Thu, 13 Sep 2018 12:07:23 +0200 Subject: [PATCH 6/6] Update from TyKind::Anon to TyKind::Opaque --- .../borrow_check/nll/region_infer/error_reporting/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc_mir/borrow_check/nll/region_infer/error_reporting/mod.rs b/src/librustc_mir/borrow_check/nll/region_infer/error_reporting/mod.rs index 4db9507ac77d0..dfed41cb1defd 100644 --- a/src/librustc_mir/borrow_check/nll/region_infer/error_reporting/mod.rs +++ b/src/librustc_mir/borrow_check/nll/region_infer/error_reporting/mod.rs @@ -489,7 +489,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { Some(ty::RegionKind::ReStatic) ) = (self.to_error_region(fr), self.to_error_region(outlived_fr)) { if let Some(ty::TyS { - sty: ty::TyKind::Anon(did, substs), + sty: ty::TyKind::Opaque(did, substs), .. }) = infcx.tcx.is_suitable_region(f) .map(|r| r.def_id)