Skip to content

Commit

Permalink
Auto merge of #53900 - davidtwco:issue-53771, r=nikomatsakis
Browse files Browse the repository at this point in the history
NLL regresses diagnostic for impl-trait/static-return-lifetime-infered.rs

Fixes #53771.

r? @nikomatsakis
cc @pnkfelix @estebank
  • Loading branch information
bors committed Sep 18, 2018
2 parents b80cb47 + 18c1374 commit 36c0ee9
Show file tree
Hide file tree
Showing 11 changed files with 276 additions and 124 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.return_type_impl_trait(anon_reg_sup.def_id).is_some()
{
let sp = var_origin.span();
let return_sp = sub_origin.span();
Expand Down
79 changes: 0 additions & 79 deletions src/librustc/infer/error_reporting/nice_region_error/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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<FreeRegionInfo> {
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.
Expand All @@ -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
Expand All @@ -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
}
}
82 changes: 81 additions & 1 deletion src/librustc/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -872,6 +872,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
Expand Down Expand Up @@ -1571,6 +1583,74 @@ 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<FreeRegionInfo> {
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 return_type_impl_trait(
&self,
scope_def_id: DefId,
) -> Option<Ty<'tcx>> {
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());
if output.is_impl_trait() {
Some(output)
} else {
None
}
}
_ => None
}
}

// 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> {
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down
Loading

0 comments on commit 36c0ee9

Please sign in to comment.