Skip to content

Adding E0623 for structs #43700

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.

Sign up for GitHub

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

Merged
merged 8 commits into from
Aug 25, 2017
Merged

Adding E0623 for structs #43700

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
fe9c68a
Adding E0623 for structs
gaurikholkar Aug 5, 2017
b1cee11
code review fixes
gaurikholkar Aug 17, 2017
8b89f31
code review fixes
gaurikholkar Aug 17, 2017
2ff1734
code review fixes
gaurikholkar Aug 20, 2017
90ab9d9
code review fixes
gaurikholkar Aug 23, 2017
b569094
minor fix
gaurikholkar Aug 23, 2017
cb563a9
adding macro, cleaning up code
gaurikholkar Aug 23, 2017
2cd1318
build fixes
gaurikholkar Aug 23, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
240 changes: 171 additions & 69 deletions src/librustc/infer/error_reporting/anon_anon_conflict.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,65 +27,84 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
// { x.push(y); }.
// The example gives
// fn foo(x: &mut Vec<&u8>, y: &u8) {
// --- --- these references must have the same lifetime
// --- --- these references are declared with different lifetimes...
// x.push(y);
// ^ data from `y` flows into `x` here
// It will later be extended to trait objects and structs.
// ^ ...but data from `y` flows into `x` here
// It has been extended for the case of structs too.
// Consider the example
// struct Ref<'a> { x: &'a u32 }
// fn foo(mut x: Vec<Ref>, y: Ref) {
// --- --- these structs are declared with different lifetimes...
// x.push(y);
// ^ ...but data from `y` flows into `x` here
// }
// It will later be extended to trait objects.
pub fn try_report_anon_anon_conflict(&self, error: &RegionResolutionError<'tcx>) -> bool {

let (span, sub, sup) = match *error {
ConcreteFailure(ref origin, sub, sup) => (origin.span(), sub, sup),
_ => return false, // inapplicable
};

// Determine whether the sub and sup consist of both anonymous (elided) regions.
let (ty1, ty2) = if self.is_suitable_anonymous_region(sup).is_some() &&
self.is_suitable_anonymous_region(sub).is_some() {
if let (Some(anon_reg1), Some(anon_reg2)) =
(self.is_suitable_anonymous_region(sup), self.is_suitable_anonymous_region(sub)) {
let ((_, br1), (_, br2)) = (anon_reg1, anon_reg2);
if self.find_anon_type(sup, &br1).is_some() &&
self.find_anon_type(sub, &br2).is_some() {
(self.find_anon_type(sup, &br1).unwrap(),
self.find_anon_type(sub, &br2).unwrap())
} else {
return false;
}
} else {
return false;
}
} else {
return false; // inapplicable
};
let anon_reg_sup = or_false!(self.is_suitable_anonymous_region(sup));

if let (Some(sup_arg), Some(sub_arg)) =
let anon_reg_sub = or_false!(self.is_suitable_anonymous_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;
let bregion_sub = anon_reg_sub.boundregion;

let ty_sup = or_false!(self.find_anon_type(sup, &bregion_sup));

let ty_sub = or_false!(self.find_anon_type(sub, &bregion_sub));

let (main_label, label1, label2) = if let (Some(sup_arg), Some(sub_arg)) =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe flatten these if let constraints with or_else! too? I hate to drag out this PR anymore though, so happy to do it as follow-up

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll open up a separate PR for that

(self.find_arg_with_anonymous_region(sup, sup),
self.find_arg_with_anonymous_region(sub, sub)) {
let ((anon_arg1, _, _, _), (anon_arg2, _, _, _)) = (sup_arg, sub_arg);

let span_label_var1 = if let Some(simple_name) = anon_arg1.pat.simple_name() {
format!(" from `{}` ", simple_name)
} else {
format!(" ")
};
let (anon_arg_sup, is_first_sup, anon_arg_sub, is_first_sub) =
(sup_arg.arg, sup_arg.is_first, sub_arg.arg, sub_arg.is_first);
if self.is_self_anon(is_first_sup, scope_def_id_sup) ||
self.is_self_anon(is_first_sub, scope_def_id_sub) {
return false;
}

let span_label_var2 = if let Some(simple_name) = anon_arg2.pat.simple_name() {
format!(" into `{}` ", simple_name)
if self.is_return_type_anon(scope_def_id_sup, bregion_sup) ||
self.is_return_type_anon(scope_def_id_sub, bregion_sub) {
return false;
}

if anon_arg_sup == anon_arg_sub {
(format!("this type was declared with multiple lifetimes..."),
format!(" with one lifetime"),
format!(" into the other"))
} else {
format!(" ")
};

struct_span_err!(self.tcx.sess, span, E0623, "lifetime mismatch")
.span_label(ty1.span,
format!("these references are not declared with the same lifetime..."))
.span_label(ty2.span, format!(""))
.span_label(span,
format!("...but data{}flows{}here", span_label_var1, span_label_var2))
.emit();
let span_label_var1 = if let Some(simple_name) = anon_arg_sup.pat.simple_name() {
format!(" from `{}`", simple_name)
} else {
format!("")
};

let span_label_var2 = if let Some(simple_name) = anon_arg_sub.pat.simple_name() {
format!(" into `{}`", simple_name)
} else {
format!("")
};

let span_label =
format!("these two types are declared with different lifetimes...",);

(span_label, span_label_var1, span_label_var2)
}
} else {
return false;
}
};

struct_span_err!(self.tcx.sess, span, E0623, "lifetime mismatch")
.span_label(ty_sup.span, main_label)
.span_label(ty_sub.span, format!(""))
.span_label(span, format!("...but data{} flows{} here", label1, label2))
.emit();
return true;
}

Expand All @@ -94,7 +113,6 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
/// contains the anonymous type.
///
/// # Arguments
///
/// region - the anonymous region corresponding to the anon_anon conflict
/// br - the bound region corresponding to the above region which is of type `BrAnon(_)`
///
Expand All @@ -105,39 +123,56 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
/// ```
/// The function returns the nested type corresponding to the anonymous region
/// for e.g. `&u8` and Vec<`&u8`.
fn find_anon_type(&self, region: Region<'tcx>, br: &ty::BoundRegion) -> Option<&hir::Ty> {
pub fn find_anon_type(&self, region: Region<'tcx>, br: &ty::BoundRegion) -> Option<&hir::Ty> {
if let Some(anon_reg) = self.is_suitable_anonymous_region(region) {
let (def_id, _) = anon_reg;
let def_id = anon_reg.def_id;
if let Some(node_id) = self.tcx.hir.as_local_node_id(def_id) {
let ret_ty = self.tcx.type_of(def_id);
if let ty::TyFnDef(_, _) = ret_ty.sty {
if let hir_map::NodeItem(it) = self.tcx.hir.get(node_id) {
if let hir::ItemFn(ref fndecl, _, _, _, _, _) = it.node {
return fndecl
.inputs
.iter()
.filter_map(|arg| {
let mut nested_visitor = FindNestedTypeVisitor {
infcx: &self,
hir_map: &self.tcx.hir,
bound_region: *br,
found_type: None,
};
nested_visitor.visit_ty(&**arg);
if nested_visitor.found_type.is_some() {
nested_visitor.found_type
} else {
None
}
})
.next();
}
}
let inputs: &[_] =
match self.tcx.hir.get(node_id) {
hir_map::NodeItem(&hir::Item {
node: hir::ItemFn(ref fndecl, ..), ..
}) => &fndecl.inputs,
hir_map::NodeTraitItem(&hir::TraitItem {
node: hir::TraitItemKind::Method(ref fndecl, ..),
..
}) => &fndecl.decl.inputs,
hir_map::NodeImplItem(&hir::ImplItem {
node: hir::ImplItemKind::Method(ref fndecl, ..),
..
}) => &fndecl.decl.inputs,

_ => &[],
};

return inputs
.iter()
.filter_map(|arg| {
self.find_component_for_bound_region(&**arg, br)
})
.next();
}
}
}
None
}

// This method creates a FindNestedTypeVisitor which returns the type corresponding
// to the anonymous region.
fn find_component_for_bound_region(&self,
arg: &'gcx hir::Ty,
br: &ty::BoundRegion)
-> Option<(&'gcx hir::Ty)> {
let mut nested_visitor = FindNestedTypeVisitor {
infcx: &self,
hir_map: &self.tcx.hir,
bound_region: *br,
found_type: None,
};
nested_visitor.visit_ty(arg);
nested_visitor.found_type
}
}

// The FindNestedTypeVisitor captures the corresponding `hir::Ty` of the
Expand Down Expand Up @@ -176,8 +211,8 @@ impl<'a, 'gcx, 'tcx> Visitor<'gcx> for FindNestedTypeVisitor<'a, 'gcx, 'tcx> {
hir::TyRptr(ref lifetime, _) => {
match self.infcx.tcx.named_region_map.defs.get(&lifetime.id) {
// the lifetime of the TyRptr
Some(&rl::Region::LateBoundAnon(debuijn_index, anon_index)) => {
if debuijn_index.depth == 1 && anon_index == br_index {
Some(&rl::Region::LateBoundAnon(debruijn_index, anon_index)) => {
if debruijn_index.depth == 1 && anon_index == br_index {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this can get the deBruijn indexes wrong in cases like fn foo<'a>(data: &for<> Fn(&'a u32)) {}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might be hard to fix, and it will only cause silly error messages (rather than e.g. crashing the compiler, which other debruijn index mistakes did), so I think it's ok if you leave it broken for now.

Copy link
Author

@gaurikholkar-zz gaurikholkar-zz Aug 16, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will be opening up a different PR for the case you mentioned. My guess is, it falls under TyBareFn

self.found_type = Some(arg);
return; // we can stop visiting now
}
Expand All @@ -191,10 +226,77 @@ impl<'a, 'gcx, 'tcx> Visitor<'gcx> for FindNestedTypeVisitor<'a, 'gcx, 'tcx> {
}
}
}
// Checks if it is of type `hir::TyPath` which corresponds to a struct.
hir::TyPath(_) => {
let subvisitor = &mut TyPathVisitor {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

indentation

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did run rustfmt. This is what I got.

infcx: self.infcx,
found_it: false,
bound_region: self.bound_region,
hir_map: self.hir_map,
};
intravisit::walk_ty(subvisitor, arg); // call walk_ty; as visit_ty is empty,
// this will visit only outermost type
if subvisitor.found_it {
self.found_type = Some(arg);
}
}
_ => {}
}
// walk the embedded contents: e.g., if we are visiting `Vec<&Foo>`,
// go on to visit `&Foo`
intravisit::walk_ty(self, arg);
}
}

// The visitor captures the corresponding `hir::Ty` of the anonymous region
// in the case of structs ie. `hir::TyPath`.
// This visitor would be invoked for each lifetime corresponding to a struct,
// and would walk the types like Vec<Ref> in the above example and Ref looking for the HIR
// where that lifetime appears. This allows us to highlight the
// specific part of the type in the error message.
struct TyPathVisitor<'a, 'gcx: 'a + 'tcx, 'tcx: 'a> {
infcx: &'a InferCtxt<'a, 'gcx, 'tcx>,
hir_map: &'a hir::map::Map<'gcx>,
found_it: bool,
bound_region: ty::BoundRegion,
}

impl<'a, 'gcx, 'tcx> Visitor<'gcx> for TyPathVisitor<'a, 'gcx, 'tcx> {
fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'gcx> {
NestedVisitorMap::OnlyBodies(&self.hir_map)
}

fn visit_lifetime(&mut self, lifetime: &hir::Lifetime) {
let br_index = match self.bound_region {
ty::BrAnon(index) => index,
_ => return,
};

match self.infcx.tcx.named_region_map.defs.get(&lifetime.id) {
// the lifetime of the TyPath!
Some(&rl::Region::LateBoundAnon(debruijn_index, anon_index)) => {
if debruijn_index.depth == 1 && anon_index == br_index {
self.found_it = true;
}
}
Some(&rl::Region::Static) |
Some(&rl::Region::EarlyBound(_, _)) |
Some(&rl::Region::LateBound(_, _)) |
Some(&rl::Region::Free(_, _)) |
None => {
debug!("no arg found");
}
}
}

fn visit_ty(&mut self, arg: &'gcx hir::Ty) {
// ignore nested types
//
// If you have a type like `Foo<'a, &Ty>` we
// are only interested in the immediate lifetimes ('a).
//
// Making `visit_ty` empty will ignore the `&Ty` embedded
// inside, it will get reached by the outer visitor.
debug!("`Ty` corresponding to a struct is {:?}", arg);
}
}
4 changes: 3 additions & 1 deletion src/librustc/infer/error_reporting/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,10 @@ use errors::{DiagnosticBuilder, DiagnosticStyledString};
mod note;

mod need_type_info;
mod util;

mod named_anon_conflict;
#[macro_use]
mod util;
mod anon_anon_conflict;

impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
Expand Down
Loading